From 8b489d1667c67741cbc1111b0a52d10996aa857d Mon Sep 17 00:00:00 2001 From: "Aaron S." <94858815+stocaaro@users.noreply.github.com> Date: Mon, 27 Jan 2025 14:51:39 -0600 Subject: [PATCH 1/5] fix(api-graphql): Data messages should maintain the keep alive status (#14164) --- .../AWSAppSyncRealTimeProvider.test.ts | 72 +++++++++++++--- .../Providers/AWSWebSocketProvider/index.ts | 82 +++++++++++++------ .../api-graphql/src/Providers/constants.ts | 5 ++ 3 files changed, 122 insertions(+), 37 deletions(-) diff --git a/packages/api-graphql/__tests__/AWSAppSyncRealTimeProvider.test.ts b/packages/api-graphql/__tests__/AWSAppSyncRealTimeProvider.test.ts index 041b9624898..e4947386178 100644 --- a/packages/api-graphql/__tests__/AWSAppSyncRealTimeProvider.test.ts +++ b/packages/api-graphql/__tests__/AWSAppSyncRealTimeProvider.test.ts @@ -147,6 +147,10 @@ describe('AWSAppSyncRealTimeProvider', () => { Object.defineProperty(constants, 'RECONNECT_DELAY', { value: 100, }); + // Reduce the keep alive heartbeat to 10ms + Object.defineProperty(constants, 'DEFAULT_KEEP_ALIVE_HEARTBEAT_TIMEOUT', { + value: 10, + }); }); afterEach(async () => { @@ -765,7 +769,7 @@ describe('AWSAppSyncRealTimeProvider', () => { // Resolve the message delivery actions await replaceConstant( 'DEFAULT_KEEP_ALIVE_ALERT_TIMEOUT', - 5, + 10, async () => { await fakeWebSocketInterface?.readyForUse; await fakeWebSocketInterface?.triggerOpen(); @@ -776,17 +780,17 @@ describe('AWSAppSyncRealTimeProvider', () => { await fakeWebSocketInterface?.startAckMessage(); await fakeWebSocketInterface?.keepAlive(); - }, - ); - await fakeWebSocketInterface?.waitUntilConnectionStateIn([ - CS.Connected, - ]); + await fakeWebSocketInterface?.waitUntilConnectionStateIn([ + CS.Connected, + ]); - // Wait until the socket is automatically disconnected - await fakeWebSocketInterface?.waitUntilConnectionStateIn([ - CS.ConnectionDisrupted, - ]); + // Wait until the socket is automatically disconnected + await fakeWebSocketInterface?.waitUntilConnectionStateIn([ + CS.ConnectionDisrupted, + ]); + }, + ); expect(fakeWebSocketInterface?.observedConnectionStates).toContain( CS.ConnectedPendingKeepAlive, @@ -798,6 +802,54 @@ describe('AWSAppSyncRealTimeProvider', () => { ); }); + test('subscription observer ka is cleared if data is received', async () => { + expect.assertions(1); + + const observer = provider.subscribe({ + appSyncGraphqlEndpoint: 'ws://localhost:8080', + }); + + observer.subscribe({ error: () => {} }); + // Resolve the message delivery actions + await replaceConstant( + 'DEFAULT_KEEP_ALIVE_ALERT_TIMEOUT', + 5, + async () => { + await fakeWebSocketInterface?.readyForUse; + await fakeWebSocketInterface?.triggerOpen(); + await fakeWebSocketInterface?.handShakeMessage({ + connectionTimeoutMs: 100, + }); + + await fakeWebSocketInterface?.startAckMessage(); + + await fakeWebSocketInterface?.keepAlive(); + + await fakeWebSocketInterface?.waitUntilConnectionStateIn([ + CS.ConnectedPendingKeepAlive, + ]); + }, + ); + + // Send message + await fakeWebSocketInterface?.sendDataMessage({ + type: MESSAGE_TYPES.DATA, + payload: { data: {} }, + }); + + await fakeWebSocketInterface?.waitUntilConnectionStateIn([ + CS.Connected, + ]); + + expect(fakeWebSocketInterface?.observedConnectionStates).toEqual([ + CS.Disconnected, + CS.Connecting, + CS.Connected, + CS.ConnectedPendingKeepAlive, + CS.Connected, + ]); + }); + test('subscription connection disruption triggers automatic reconnection', async () => { expect.assertions(1); diff --git a/packages/api-graphql/src/Providers/AWSWebSocketProvider/index.ts b/packages/api-graphql/src/Providers/AWSWebSocketProvider/index.ts index 5d710e478bc..f62f0518f1a 100644 --- a/packages/api-graphql/src/Providers/AWSWebSocketProvider/index.ts +++ b/packages/api-graphql/src/Providers/AWSWebSocketProvider/index.ts @@ -23,7 +23,7 @@ import { CONNECTION_INIT_TIMEOUT, CONNECTION_STATE_CHANGE, DEFAULT_KEEP_ALIVE_ALERT_TIMEOUT, - DEFAULT_KEEP_ALIVE_TIMEOUT, + DEFAULT_KEEP_ALIVE_HEARTBEAT_TIMEOUT, MAX_DELAY_MS, MESSAGE_TYPES, NON_RETRYABLE_CODES, @@ -83,9 +83,8 @@ export abstract class AWSWebSocketProvider { protected awsRealTimeSocket?: WebSocket; private socketStatus: SOCKET_STATUS = SOCKET_STATUS.CLOSED; - private keepAliveTimeoutId?: ReturnType; - private keepAliveTimeout = DEFAULT_KEEP_ALIVE_TIMEOUT; - private keepAliveAlertTimeoutId?: ReturnType; + private keepAliveTimestamp: number = Date.now(); + private keepAliveHeartbeatIntervalId?: ReturnType; private promiseArray: { res(): void; rej(reason?: any): void }[] = []; private connectionState: ConnectionState | undefined; private readonly connectionStateMonitor = new ConnectionStateMonitor(); @@ -119,6 +118,7 @@ export abstract class AWSWebSocketProvider { return new Promise((resolve, reject) => { if (this.awsRealTimeSocket) { this.awsRealTimeSocket.onclose = (_: CloseEvent) => { + this._closeSocket(); this.subscriptionObserverMap = new Map(); this.awsRealTimeSocket = undefined; resolve(); @@ -171,7 +171,7 @@ export abstract class AWSWebSocketProvider { this.logger.debug( `${CONTROL_MSG.REALTIME_SUBSCRIPTION_INIT_ERROR}: ${err}`, ); - this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + this._closeSocket(); }) .finally(() => { subscriptionStartInProgress = false; @@ -435,7 +435,7 @@ export abstract class AWSWebSocketProvider { this.logger.debug({ err }); const message = String(err.message ?? ''); // Resolving to give the state observer time to propogate the update - this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + this._closeSocket(); // Capture the error only when the network didn't cause disruption if ( @@ -544,12 +544,7 @@ export abstract class AWSWebSocketProvider { setTimeout(this._closeSocketIfRequired.bind(this), 1000); } else { this.logger.debug('closing WebSocket...'); - if (this.keepAliveTimeoutId) { - clearTimeout(this.keepAliveTimeoutId); - } - if (this.keepAliveAlertTimeoutId) { - clearTimeout(this.keepAliveAlertTimeoutId); - } + const tempSocket = this.awsRealTimeSocket; // Cleaning callbacks to avoid race condition, socket still exists tempSocket.onclose = null; @@ -557,7 +552,7 @@ export abstract class AWSWebSocketProvider { tempSocket.close(1000); this.awsRealTimeSocket = undefined; this.socketStatus = SOCKET_STATUS.CLOSED; - this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + this._closeSocket(); } } @@ -577,13 +572,40 @@ export abstract class AWSWebSocketProvider { errorType: string; }; + private maintainKeepAlive() { + this.keepAliveTimestamp = Date.now(); + } + + private keepAliveHeartbeat(connectionTimeoutMs: number) { + const currentTime = Date.now(); + + // Check for missed KA message + if ( + currentTime - this.keepAliveTimestamp > + DEFAULT_KEEP_ALIVE_ALERT_TIMEOUT + ) { + this.connectionStateMonitor.record(CONNECTION_CHANGE.KEEP_ALIVE_MISSED); + } else { + this.connectionStateMonitor.record(CONNECTION_CHANGE.KEEP_ALIVE); + } + + // Recognize we are disconnected if we haven't seen messages in the keep alive timeout period + if (currentTime - this.keepAliveTimestamp > connectionTimeoutMs) { + this._errorDisconnect(CONTROL_MSG.TIMEOUT_DISCONNECT); + } + } + private _handleIncomingSubscriptionMessage(message: MessageEvent) { if (typeof message.data !== 'string') { return; } const [isData, data] = this._handleSubscriptionData(message); - if (isData) return; + if (isData) { + this.maintainKeepAlive(); + + return; + } const { type, id, payload } = data; @@ -632,16 +654,7 @@ export abstract class AWSWebSocketProvider { } if (type === MESSAGE_TYPES.GQL_CONNECTION_KEEP_ALIVE) { - if (this.keepAliveTimeoutId) clearTimeout(this.keepAliveTimeoutId); - if (this.keepAliveAlertTimeoutId) - clearTimeout(this.keepAliveAlertTimeoutId); - this.keepAliveTimeoutId = setTimeout(() => { - this._errorDisconnect(CONTROL_MSG.TIMEOUT_DISCONNECT); - }, this.keepAliveTimeout); - this.keepAliveAlertTimeoutId = setTimeout(() => { - this.connectionStateMonitor.record(CONNECTION_CHANGE.KEEP_ALIVE_MISSED); - }, DEFAULT_KEEP_ALIVE_ALERT_TIMEOUT); - this.connectionStateMonitor.record(CONNECTION_CHANGE.KEEP_ALIVE); + this.maintainKeepAlive(); return; } @@ -686,13 +699,21 @@ export abstract class AWSWebSocketProvider { this.logger.debug(`Disconnect error: ${msg}`); if (this.awsRealTimeSocket) { - this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + this._closeSocket(); this.awsRealTimeSocket.close(); } this.socketStatus = SOCKET_STATUS.CLOSED; } + private _closeSocket() { + if (this.keepAliveHeartbeatIntervalId) { + clearInterval(this.keepAliveHeartbeatIntervalId); + this.keepAliveHeartbeatIntervalId = undefined; + } + this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + } + private _timeoutStartSubscriptionAck(subscriptionId: string) { const subscriptionObserver = this.subscriptionObserverMap.get(subscriptionId); @@ -708,7 +729,7 @@ export abstract class AWSWebSocketProvider { subscriptionState: SUBSCRIPTION_STATUS.FAILED, }); - this.connectionStateMonitor.record(CONNECTION_CHANGE.CLOSED); + this._closeSocket(); this.logger.debug( 'timeoutStartSubscription', JSON.stringify({ query, variables }), @@ -820,6 +841,7 @@ export abstract class AWSWebSocketProvider { this.logger.debug(`WebSocket connection error`); }; newSocket.onclose = () => { + this._closeSocket(); reject(new Error('Connection handshake error')); }; newSocket.onopen = () => { @@ -849,6 +871,7 @@ export abstract class AWSWebSocketProvider { this.awsRealTimeSocket.onclose = event => { this.logger.debug(`WebSocket closed ${event.reason}`); + this._closeSocket(); reject(new Error(JSON.stringify(event))); }; @@ -912,7 +935,11 @@ export abstract class AWSWebSocketProvider { return; } - this.keepAliveTimeout = connectionTimeoutMs; + // Set up a keep alive heartbeat for this connection + this.keepAliveHeartbeatIntervalId = setInterval(() => { + this.keepAliveHeartbeat(connectionTimeoutMs); + }, DEFAULT_KEEP_ALIVE_HEARTBEAT_TIMEOUT); + this.awsRealTimeSocket.onmessage = this._handleIncomingSubscriptionMessage.bind(this); @@ -923,6 +950,7 @@ export abstract class AWSWebSocketProvider { this.awsRealTimeSocket.onclose = event => { this.logger.debug(`WebSocket closed ${event.reason}`); + this._closeSocket(); this._errorDisconnect(CONTROL_MSG.CONNECTION_CLOSED); }; } diff --git a/packages/api-graphql/src/Providers/constants.ts b/packages/api-graphql/src/Providers/constants.ts index 5e82f672081..5aef1a60130 100644 --- a/packages/api-graphql/src/Providers/constants.ts +++ b/packages/api-graphql/src/Providers/constants.ts @@ -128,6 +128,11 @@ export const START_ACK_TIMEOUT = 15000; */ export const DEFAULT_KEEP_ALIVE_TIMEOUT = 5 * 60 * 1000; +/** + * Default Time in milleseconds between monitoring checks of keep alive status + */ +export const DEFAULT_KEEP_ALIVE_HEARTBEAT_TIMEOUT = 5 * 1000; + /** * Default Time in milleseconds to alert for missed GQL_CONNECTION_KEEP_ALIVE message */ From 22ca811743f6729d3a00dd71726ff6b5afb44b53 Mon Sep 17 00:00:00 2001 From: Pranav Malewadkar Date: Tue, 28 Jan 2025 08:37:14 -0800 Subject: [PATCH 2/5] fix(deps): fix more implicit deps and add linting (#14137) Co-authored-by: ashika112 <155593080+ashika112@users.noreply.github.com> Co-authored-by: ashika112 --- eslint.config.mjs | 13 ++++++++++++- packages/api-rest/package.json | 1 + packages/api/package.json | 6 ++++++ packages/core/package.json | 1 + packages/datastore/package.json | 1 + packages/geo/package.json | 1 + packages/notifications/package.json | 1 + packages/rtn-push-notification/package.json | 7 +++++++ .../providers/s3/apis/internal/testUtils.ts | 1 - 9 files changed, 30 insertions(+), 2 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 4d4f9c7e3ac..4e079dc7d75 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -24,7 +24,7 @@ const compat = new FlatCompat({ const customClientDtsFiles = customClientDtsBundlerConfig.entries .map(clientBundlerConfig => clientBundlerConfig.outFile) .filter(outFile => outFile?.length > 0) - .map(outFile => outFile.replace(__dirname + path.sep, '')) // Convert absolute path to relative path + .map(outFile => outFile.replace(__dirname + path.sep, '')); // Convert absolute path to relative path export default [ { @@ -294,4 +294,15 @@ export default [ 'jsdoc/no-undefined-types': 1, }, }, + { + ignores: [ + '**/**.{native,android,ios}.**', + '**/__tests__/**', + '**/packages/adapter-nextjs/**', + '**/packages/react-native/example/**', + ], + rules: { + 'import/no-extraneous-dependencies': 'error', + }, + }, ]; diff --git a/packages/api-rest/package.json b/packages/api-rest/package.json index 782b5b082f3..0a3a8d7d460 100644 --- a/packages/api-rest/package.json +++ b/packages/api-rest/package.json @@ -89,6 +89,7 @@ "devDependencies": { "@aws-amplify/core": "6.9.2", "@aws-amplify/react-native": "1.1.6", + "@aws-sdk/types": "3.387.0", "typescript": "5.0.2" }, "size-limit": [ diff --git a/packages/api/package.json b/packages/api/package.json index cececda6512..4527f853c15 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -68,6 +68,7 @@ }, "homepage": "https://aws-amplify.github.io/", "devDependencies": { + "@aws-amplify/core": "6.9.2", "jest-fetch-mock": "3.0.3", "typescript": "5.0.2" }, @@ -82,6 +83,11 @@ "dependencies": { "@aws-amplify/api-graphql": "4.7.2", "@aws-amplify/api-rest": "4.0.67", + "@aws-amplify/data-schema": "^1.7.0", + "rxjs": "^7.8.1", "tslib": "^2.5.0" + }, + "peerDependencies": { + "@aws-amplify/core": "^6.1.0" } } diff --git a/packages/core/package.json b/packages/core/package.json index 7e8c0d13a5d..0732012fc50 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -54,6 +54,7 @@ "@aws-sdk/types": "3.398.0", "@smithy/util-hex-encoding": "2.0.0", "@types/uuid": "^9.0.0", + "cookie": "^0.7.0", "js-cookie": "^3.0.5", "rxjs": "^7.8.1", "tslib": "^2.5.0", diff --git a/packages/datastore/package.json b/packages/datastore/package.json index c0c03841550..f6b8878e800 100644 --- a/packages/datastore/package.json +++ b/packages/datastore/package.json @@ -45,6 +45,7 @@ ], "dependencies": { "@aws-amplify/api": "6.2.2", + "@aws-amplify/api-graphql": "4.7.2", "buffer": "4.9.2", "idb": "5.0.6", "immer": "9.0.6", diff --git a/packages/geo/package.json b/packages/geo/package.json index 5d73b70b4eb..4fab3548d38 100644 --- a/packages/geo/package.json +++ b/packages/geo/package.json @@ -69,6 +69,7 @@ "dependencies": { "@aws-sdk/client-location": "3.621.0", "@turf/boolean-clockwise": "6.5.0", + "@aws-sdk/types": "3.398.0", "camelcase-keys": "6.2.2", "tslib": "^2.5.0" }, diff --git a/packages/notifications/package.json b/packages/notifications/package.json index 72891784a8b..53da78f306c 100644 --- a/packages/notifications/package.json +++ b/packages/notifications/package.json @@ -91,6 +91,7 @@ "push-notifications" ], "dependencies": { + "@aws-sdk/types": "3.398.0", "lodash": "^4.17.21", "tslib": "^2.5.0" }, diff --git a/packages/rtn-push-notification/package.json b/packages/rtn-push-notification/package.json index 7a24a35a039..b20ef6e0aef 100644 --- a/packages/rtn-push-notification/package.json +++ b/packages/rtn-push-notification/package.json @@ -23,10 +23,17 @@ "lint:fix": "eslint '**/*.{ts,tsx}' --fix", "ts-coverage": "typescript-coverage-report -p ./tsconfig.build.json -t 99" }, + "dependencies": { + "lodash": "^4.17.21" + }, "devDependencies": { "@types/react-native": "0.70.0", + "react-native": "0.71.0", "typescript": "5.0.2" }, + "peerDependencies": { + "react-native": ">=0.70" + }, "repository": { "type": "git", "url": "https://github.com/aws-amplify/amplify-js.git" diff --git a/packages/storage/__tests__/providers/s3/apis/internal/testUtils.ts b/packages/storage/__tests__/providers/s3/apis/internal/testUtils.ts index 75f5dd823b2..3c7ee88f42f 100644 --- a/packages/storage/__tests__/providers/s3/apis/internal/testUtils.ts +++ b/packages/storage/__tests__/providers/s3/apis/internal/testUtils.ts @@ -1,5 +1,4 @@ import { AWSCredentials } from '@aws-amplify/core/internals/utils'; -import { expect } from '@jest/globals'; import { type MatcherFunction } from 'expect'; const toBeLastCalledWithConfigAndInput: MatcherFunction< From 619f47c4296a5f9d160e0fbae4155d0ab7254170 Mon Sep 17 00:00:00 2001 From: Ashwin Kumar Date: Tue, 28 Jan 2025 14:01:30 -0800 Subject: [PATCH 3/5] chore(e2e): remove legacy interactions e2e tests (#14163) Co-authored-by: Ashwin Kumar --- .github/integ-config/integ-all.yml | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/.github/integ-config/integ-all.yml b/.github/integ-config/integ-all.yml index 8dce2c9a9ea..6f467fcd84a 100644 --- a/.github/integ-config/integ-all.yml +++ b/.github/integ-config/integ-all.yml @@ -758,13 +758,6 @@ tests: # amplifyjs_dir: true # INTERACTIONS - # - test_name: integ_react_interactions_react_interactions - # desc: 'React Interactions' - # framework: react - # category: interactions - # sample_name: [chatbot-component] - # spec: chatbot-component - # browser: *minimal_browser_list - test_name: integ_react_interactions_chatbot_v1 desc: 'Chatbot V1' framework: react @@ -779,27 +772,6 @@ tests: sample_name: [lex-test-component] spec: chatbot-v2 browser: *minimal_browser_list - # - test_name: integ_angular_interactions - # desc: 'Angular Interactions' - # framework: angular - # category: interactions - # sample_name: [chatbot-component] - # spec: chatbot-component - # browser: *minimal_browser_list - # - test_name: integ_vue_interactions_vue_2_interactions - # desc: 'Vue 2 Interactions' - # framework: vue - # category: interactions - # sample_name: [chatbot-component] - # spec: chatbot-component - # browser: [chrome] - # - test_name: integ_vue_interactionsvue_3_interactions - # desc: 'Vue 3 Interactions' - # framework: vue - # category: interactions - # sample_name: [chatbot-component-vue3] - # spec: chatbot-component - # browser: [chrome] # PREDICTIONS - test_name: integ_react_predictions From e888e7c0ce264871def577a01fd804919f7b79d5 Mon Sep 17 00:00:00 2001 From: Ashwin Kumar Date: Tue, 28 Jan 2025 14:43:24 -0800 Subject: [PATCH 4/5] chore(ci): remove storage-browser integ test auth0 deps (#14080) Co-authored-by: Ashwin Kumar --- .github/workflows/callable-e2e-test.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/callable-e2e-test.yml b/.github/workflows/callable-e2e-test.yml index 9eb0eca702e..0a5002fa211 100644 --- a/.github/workflows/callable-e2e-test.yml +++ b/.github/workflows/callable-e2e-test.yml @@ -49,10 +49,6 @@ env: CYPRESS_GOOGLE_CLIENTID: ${{ secrets.CYPRESS_GOOGLE_CLIENTID }} CYPRESS_GOOGLE_CLIENT_SECRET: ${{ secrets.CYPRESS_GOOGLE_CLIENT_SECRET }} CYPRESS_GOOGLE_REFRESH_TOKEN: ${{ secrets.CYPRESS_GOOGLE_REFRESH_TOKEN }} - CYPRESS_AUTH0_CLIENTID: ${{ secrets.CYPRESS_AUTH0_CLIENTID }} - CYPRESS_AUTH0_SECRET: ${{ secrets.CYPRESS_AUTH0_SECRET }} - CYPRESS_AUTH0_AUDIENCE: ${{ secrets.CYPRESS_AUTH0_AUDIENCE }} - CYPRESS_AUTH0_DOMAIN: ${{ secrets.CYPRESS_AUTH0_DOMAIN }} jobs: e2e-test: From ba025e52522ab5a0b292fc0c0c6b47c8e4c53cf9 Mon Sep 17 00:00:00 2001 From: Chris F <5827964+cshfang@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:32:36 -0800 Subject: [PATCH 5/5] chore(storage): Improve error messaging for MP upload missing ETag (#14170) * chore(storage): Improve error messaging for MP upload missing ETag * Bump size limit --- packages/aws-amplify/package.json | 2 +- .../s3Data/completeMultipartUpload.test.ts | 15 ++++++++++++++- .../client/s3data/completeMultipartUpload.ts | 17 +++++++++++++---- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 159c7da99d5..c399a5309e6 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -515,7 +515,7 @@ "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "22.95 kB" + "limit": "23.00 kB" } ] } diff --git a/packages/storage/__tests__/providers/s3/utils/client/s3Data/completeMultipartUpload.test.ts b/packages/storage/__tests__/providers/s3/utils/client/s3Data/completeMultipartUpload.test.ts index 5036a9de6fb..16e65d11b21 100644 --- a/packages/storage/__tests__/providers/s3/utils/client/s3Data/completeMultipartUpload.test.ts +++ b/packages/storage/__tests__/providers/s3/utils/client/s3Data/completeMultipartUpload.test.ts @@ -83,7 +83,6 @@ describe('completeMultipartUploadSerializer', () => { ], }, }); - console.log(output); expect(output).toEqual({ $metadata: expect.objectContaining(expectedMetadata), }); @@ -140,4 +139,18 @@ describe('completeMultipartUploadSerializer', () => { }), ).rejects.toThrow(integrityError); }); + + it('should fail with specific error messaging when ETag is missing from response', () => { + mockS3TransferHandler.mockResolvedValue( + mockBinaryResponse(completeMultipartUploadSuccessResponse), + ); + expect( + completeMultipartUpload(defaultConfig, { + Bucket: 'bucket', + Key: 'key', + UploadId: 'uploadId', + MultipartUpload: { Parts: [{ PartNumber: 1 }] }, + }), + ).rejects.toThrow('ETag missing'); + }); }); diff --git a/packages/storage/src/providers/s3/utils/client/s3data/completeMultipartUpload.ts b/packages/storage/src/providers/s3/utils/client/s3data/completeMultipartUpload.ts index e7c4c516157..6ce44756b76 100644 --- a/packages/storage/src/providers/s3/utils/client/s3data/completeMultipartUpload.ts +++ b/packages/storage/src/providers/s3/utils/client/s3data/completeMultipartUpload.ts @@ -36,7 +36,11 @@ import type { } from './types'; const INVALID_PARAMETER_ERROR_MSG = - 'Invalid parameter for ComplteMultipartUpload API'; + 'Invalid parameter for CompleteMultipartUpload API'; + +const MISSING_ETAG_ERROR_MSG = 'ETag missing from multipart upload'; +const MISSING_ETAG_ERROR_SUGGESTION = + 'Please ensure S3 bucket CORS configuration includes ETag as part of its `ExposeHeaders` element'; export type CompleteMultipartUploadInput = Pick< CompleteMultipartUploadCommandInput, @@ -95,7 +99,7 @@ const serializeCompletedMultipartUpload = ( input: CompletedMultipartUpload, ): string => { if (!input.Parts?.length) { - throw new Error(`${INVALID_PARAMETER_ERROR_MSG}: ${input}`); + throw new Error(`${INVALID_PARAMETER_ERROR_MSG}: ${JSON.stringify(input)}`); } return `${input.Parts.map( @@ -104,8 +108,13 @@ const serializeCompletedMultipartUpload = ( }; const serializeCompletedPartList = (input: CompletedPart): string => { - if (!input.ETag || input.PartNumber == null) { - throw new Error(`${INVALID_PARAMETER_ERROR_MSG}: ${input}`); + if (input.PartNumber == null) { + throw new Error(`${INVALID_PARAMETER_ERROR_MSG}: ${JSON.stringify(input)}`); + } + if (!input.ETag) { + throw new Error( + `${MISSING_ETAG_ERROR_MSG}: ${JSON.stringify(input)}. ${MISSING_ETAG_ERROR_SUGGESTION}`, + ); } const eTag = `${input.ETag}`;