From da62f578b482cfa2e0fd183f924297e340c669d8 Mon Sep 17 00:00:00 2001 From: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Date: Fri, 25 Oct 2024 10:19:40 -0700 Subject: [PATCH] release(required): network error has not been properly detected (#13935) * fix(core): retry is not applied on network error * fix(auth): wrongly clear tokens on network error when refresh tokens * chore(aws-amplify): bump list bundle size limits --- .../tokenProvider/tokenOrchestrator.test.ts | 22 +++++++++- .../tokenProvider/TokenOrchestrator.ts | 3 +- packages/aws-amplify/package.json | 2 +- .../retry/defaultRetryDecider.test.ts | 40 +++++++++++++++++++ .../middleware/retry/defaultRetryDecider.ts | 7 +++- 5 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 packages/core/__tests__/clients/middleware/retry/defaultRetryDecider.test.ts diff --git a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts index c0853b51f23..d514772695a 100644 --- a/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts +++ b/packages/auth/__tests__/providers/cognito/tokenProvider/tokenOrchestrator.test.ts @@ -1,11 +1,15 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +import { + AmplifyError, + AmplifyErrorCode, +} from '@aws-amplify/core/internals/utils'; + import { tokenOrchestrator } from '../../../../src/providers/cognito/tokenProvider'; import { CognitoAuthTokens } from '../../../../src/providers/cognito/tokenProvider/types'; import { oAuthStore } from '../../../../src/providers/cognito/utils/oauth/oAuthStore'; -jest.mock('@aws-amplify/core/internals/utils'); jest.mock('@aws-amplify/core', () => ({ ...jest.requireActual('@aws-amplify/core'), Hub: { @@ -90,4 +94,20 @@ describe('tokenOrchestrator', () => { expect(newTokens?.signInDetails).toEqual(testSignInDetails); }); }); + + describe('handleErrors method', () => { + it('does not call clearTokens() if the error is a network error thrown from fetch handler', () => { + const clearTokensSpy = jest.spyOn(tokenOrchestrator, 'clearTokens'); + const error = new AmplifyError({ + name: AmplifyErrorCode.NetworkError, + message: 'Network Error', + }); + + expect(() => { + (tokenOrchestrator as any).handleErrors(error); + }).toThrow(error); + + expect(clearTokensSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts index 121875013e2..3f8027d2596 100644 --- a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts +++ b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts @@ -9,6 +9,7 @@ import { } from '@aws-amplify/core'; import { AMPLIFY_SYMBOL, + AmplifyErrorCode, assertTokenProviderConfig, isBrowser, isTokenExpired, @@ -169,7 +170,7 @@ export class TokenOrchestrator implements AuthTokenOrchestrator { private handleErrors(err: unknown) { assertServiceError(err); - if (err.message !== 'Network error') { + if (err.name !== AmplifyErrorCode.NetworkError) { // TODO(v6): Check errors on client this.clearTokens(); } diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index e858c991914..b09f0cfe4d0 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -485,7 +485,7 @@ "name": "[Storage] list (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ list }", - "limit": "15.41 kB" + "limit": "15.50 kB" }, { "name": "[Storage] remove (S3)", diff --git a/packages/core/__tests__/clients/middleware/retry/defaultRetryDecider.test.ts b/packages/core/__tests__/clients/middleware/retry/defaultRetryDecider.test.ts new file mode 100644 index 00000000000..160c4fdbe74 --- /dev/null +++ b/packages/core/__tests__/clients/middleware/retry/defaultRetryDecider.test.ts @@ -0,0 +1,40 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { HttpResponse } from '../../../../src/clients'; +import { getRetryDecider } from '../../../../src/clients/middleware/retry/defaultRetryDecider'; +import { AmplifyError } from '../../../../src/errors'; +import { AmplifyErrorCode } from '../../../../src/types'; + +describe('getRetryDecider', () => { + const mockErrorParser = jest.fn(); + + describe('created retryDecider', () => { + const mockNetworkErrorThrownFromFetch = new AmplifyError({ + name: AmplifyErrorCode.NetworkError, + message: 'Network Error', + }); + const mockNetworkErrorThrownFromXHRInStorage = new Error('Network Error'); + mockNetworkErrorThrownFromXHRInStorage.name = 'ERR_NETWORK'; + mockNetworkErrorThrownFromXHRInStorage.message = 'Network Error'; + + test.each([ + [ + 'a network error from the fetch handler', + true, + mockNetworkErrorThrownFromFetch, + ], + [ + 'a network error from the XHR handler defined in Storage', + true, + mockNetworkErrorThrownFromXHRInStorage, + ], + ])('when receives %p returns %p', (_, expected, error) => { + const mockResponse = {} as unknown as HttpResponse; + mockErrorParser.mockReturnValueOnce(error); + const retryDecider = getRetryDecider(mockErrorParser); + + expect(retryDecider(mockResponse, error)).resolves.toBe(expected); + }); + }); +}); diff --git a/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts b/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts index 874cc74314e..a990fbbdc3c 100644 --- a/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts +++ b/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts @@ -1,6 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 +import { AmplifyErrorCode } from '../../../types'; import { ErrorParser, HttpResponse } from '../../types'; import { isClockSkewError } from './isClockSkewError'; @@ -55,7 +56,11 @@ const isThrottlingError = (statusCode?: number, errorCode?: string) => (!!errorCode && THROTTLING_ERROR_CODES.includes(errorCode)); const isConnectionError = (error?: unknown) => - (error as Error)?.name === 'Network error'; + [ + AmplifyErrorCode.NetworkError, + // TODO(vNext): unify the error code `ERR_NETWORK` used by the Storage XHR handler + 'ERR_NETWORK', + ].includes((error as Error)?.name); const isServerSideError = (statusCode?: number, errorCode?: string) => (!!statusCode && [500, 502, 503, 504].includes(statusCode)) ||