From 50ecafa478d5e43a9db37144b3b7d0a51e4c2e30 Mon Sep 17 00:00:00 2001 From: Hui Zhao <10602282+HuiSF@users.noreply.github.com> Date: Wed, 27 Mar 2024 13:40:37 -0700 Subject: [PATCH] fix(api-graphql): update error handling (#13177) chore(api-graphql): improve error handling - Use GraphApiError to create errors to be thrown * error message field remained the same as before * added recoverySuggestion field for each error case - Created createGraphQLResultWithError utility for rewrapping error into GraphQLResult format --- .../api-graphql/__tests__/GraphQLAPI.test.ts | 117 +++++++++++++----- .../src/internals/InternalGraphQLAPI.ts | 106 ++++++++-------- .../isGraphQLResponseWithErrors.ts | 15 +++ .../api-graphql/src/utils/errors/constants.ts | 56 +++++++++ .../errors/createGraphQLResultWithError.ts | 15 +++ .../api-graphql/src/utils/errors/index.ts | 1 + .../src/utils/errors/repackageAuthError.ts | 18 ++- packages/api-graphql/tsconfig.json | 1 - .../api-rest/src/apis/common/internalPost.ts | 28 +++++ packages/api-rest/src/internals/index.ts | 21 +--- .../src/utils/createCancellableOperation.ts | 2 + packages/aws-amplify/package.json | 2 +- 12 files changed, 262 insertions(+), 120 deletions(-) create mode 100644 packages/api-graphql/src/internals/utils/runtimeTypeGuards/isGraphQLResponseWithErrors.ts create mode 100644 packages/api-graphql/src/utils/errors/constants.ts create mode 100644 packages/api-graphql/src/utils/errors/createGraphQLResultWithError.ts diff --git a/packages/api-graphql/__tests__/GraphQLAPI.test.ts b/packages/api-graphql/__tests__/GraphQLAPI.test.ts index 29599a9cb81..e3936363270 100644 --- a/packages/api-graphql/__tests__/GraphQLAPI.test.ts +++ b/packages/api-graphql/__tests__/GraphQLAPI.test.ts @@ -14,6 +14,9 @@ import { import { GetThreadQuery } from './fixtures/with-types/API'; import { AWSAppSyncRealTimeProvider } from '../src/Providers/AWSAppSyncRealTimeProvider'; import { Observable, of } from 'rxjs'; +import { GraphQLApiError } from '../src/utils/errors'; +import { NO_ENDPOINT } from '../src/utils/errors/constants'; +import { GraphQLError } from 'graphql'; const serverManagedFields = { id: 'some-id', @@ -61,13 +64,13 @@ const client = { isCancelError, } as V6Client; -afterEach(() => { - jest.restoreAllMocks(); -}); +const mockFetchAuthSession = (Amplify as any).Auth + .fetchAuthSession as jest.Mock; describe('API test', () => { afterEach(() => { jest.clearAllMocks(); + jest.restoreAllMocks(); }); describe('graphql test', () => { @@ -738,32 +741,9 @@ describe('API test', () => { }); test('multi-auth default case api-key, OIDC as auth mode, but no federatedSign', async () => { - Amplify.configure({ - API: { - GraphQL: { - defaultAuthMode: 'apiKey', - apiKey: 'FAKE-KEY', - endpoint: 'https://localhost/graphql', - region: 'local-host-h4x', - }, - }, - }); - - Amplify.configure({ - API: { - GraphQL: { - defaultAuthMode: 'apiKey', - apiKey: 'FAKE-KEY', - endpoint: 'https://localhost/graphql', - region: 'local-host-h4x', - }, - }, - }); - }); - - test('multi-auth default case api-key, OIDC as auth mode, but no federatedSign', async () => { - const prevMockAccessToken = mockAccessToken; - mockAccessToken = null; + mockFetchAuthSession.mockRejectedValueOnce( + new Error('mock failing fetchAuthSession() call here.'), + ); Amplify.configure({ API: { @@ -806,9 +786,6 @@ describe('API test', () => { authMode: 'oidc', }), ).rejects.toThrow('No current user'); - - // Cleanup: - mockAccessToken = prevMockAccessToken; }); test('multi-auth using CUP as auth mode, but no userpool', async () => { @@ -1342,5 +1319,81 @@ describe('API test', () => { }, ); }); + + test('throws a GraphQLResult with NO_ENDPOINT error when endpoint is not configured', () => { + const expectedGraphQLApiError = new GraphQLApiError(NO_ENDPOINT); + + Amplify.configure({ + API: { + GraphQL: { + defaultAuthMode: 'apiKey', + apiKey: 'FAKE-KEY', + region: 'local-host-h4x', + } as any, + }, + }); + + const graphqlVariables = { id: 'some-id' }; + + expect(() => + client.graphql({ + query: typedQueries.getThread, + variables: graphqlVariables, + authMode: 'iam', + }), + ).rejects.toEqual( + expect.objectContaining({ + errors: expect.arrayContaining([ + new GraphQLError( + expectedGraphQLApiError.message, + null, + null, + null, + null, + expectedGraphQLApiError, + ), + ]), + }), + ); + }); + + test('throws a GraphQLResult with NetworkError when the `post()` API throws for network error', () => { + const postAPIThrownError = new Error('Network error'); + jest + .spyOn((raw.GraphQLAPI as any)._api, 'post') + .mockRejectedValueOnce(postAPIThrownError); + + Amplify.configure({ + API: { + GraphQL: { + defaultAuthMode: 'userPool', + endpoint: 'https://localhost/graphql', + region: 'local-host-h4x', + }, + }, + }); + + const graphqlVariables = { id: 'some-id' }; + + expect( + client.graphql({ + query: typedQueries.getThread, + variables: graphqlVariables, + }), + ).rejects.toEqual( + expect.objectContaining({ + errors: expect.arrayContaining([ + new GraphQLError( + postAPIThrownError.message, + null, + null, + null, + null, + postAPIThrownError, + ), + ]), + }), + ); + }); }); }); diff --git a/packages/api-graphql/src/internals/InternalGraphQLAPI.ts b/packages/api-graphql/src/internals/InternalGraphQLAPI.ts index fd3f276b2b6..c0fff702918 100644 --- a/packages/api-graphql/src/internals/InternalGraphQLAPI.ts +++ b/packages/api-graphql/src/internals/InternalGraphQLAPI.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import { DocumentNode, - GraphQLError, OperationDefinitionNode, OperationTypeNode, parse, @@ -25,14 +24,20 @@ import { import { CustomHeaders, RequestOptions } from '@aws-amplify/data-schema-types'; import { AWSAppSyncRealTimeProvider } from '../Providers/AWSAppSyncRealTimeProvider'; -import { - GraphQLAuthError, - GraphQLOperation, - GraphQLOptions, - GraphQLResult, -} from '../types'; +import { GraphQLOperation, GraphQLOptions, GraphQLResult } from '../types'; import { resolveConfig, resolveLibraryOptions } from '../utils'; -import { repackageUnauthError } from '../utils/errors/repackageAuthError'; +import { repackageUnauthorizedError } from '../utils/errors/repackageAuthError'; +import { + NO_API_KEY, + NO_AUTH_TOKEN_HEADER, + NO_ENDPOINT, + NO_SIGNED_IN_USER, + NO_VALID_AUTH_TOKEN, + NO_VALID_CREDENTIALS, +} from '../utils/errors/constants'; +import { GraphQLApiError, createGraphQLResultWithError } from '../utils/errors'; + +import { isGraphQLResponseWithErrors } from './utils/runtimeTypeGuards/isGraphQLResponseWithErrors'; const USER_AGENT_HEADER = 'x-amz-user-agent'; @@ -76,7 +81,7 @@ export class InternalGraphQLAPIClass { switch (authMode) { case 'apiKey': if (!apiKey) { - throw new Error(GraphQLAuthError.NO_API_KEY); + throw new GraphQLApiError(NO_API_KEY); } headers = { 'X-Api-Key': apiKey, @@ -85,33 +90,44 @@ export class InternalGraphQLAPIClass { case 'iam': { const session = await amplify.Auth.fetchAuthSession(); if (session.credentials === undefined) { - throw new Error(GraphQLAuthError.NO_CREDENTIALS); + throw new GraphQLApiError(NO_VALID_CREDENTIALS); } break; } case 'oidc': - case 'userPool': + case 'userPool': { + let token: string | undefined; + try { - const token = ( + token = ( await amplify.Auth.fetchAuthSession() ).tokens?.accessToken.toString(); - - if (!token) { - throw new Error(GraphQLAuthError.NO_FEDERATED_JWT); - } - headers = { - Authorization: token, - }; } catch (e) { - throw new Error(GraphQLAuthError.NO_CURRENT_USER); + // fetchAuthSession failed + throw new GraphQLApiError({ + ...NO_SIGNED_IN_USER, + underlyingError: e, + }); + } + + // `fetchAuthSession()` succeeded but didn't return `tokens`. + // This may happen when unauthenticated access is enabled and there is + // no user signed in. + if (!token) { + throw new GraphQLApiError(NO_VALID_AUTH_TOKEN); } + + headers = { + Authorization: token, + }; break; + } case 'lambda': if ( typeof additionalHeaders === 'object' && !additionalHeaders.Authorization ) { - throw new Error(GraphQLAuthError.NO_AUTH_TOKEN); + throw new GraphQLApiError(NO_AUTH_TOKEN_HEADER); } headers = { @@ -350,18 +366,15 @@ export class InternalGraphQLAPIClass { const endpoint = customEndpoint || appSyncGraphqlEndpoint; if (!endpoint) { - const error = new GraphQLError('No graphql endpoint provided.'); - // TODO(Eslint): refactor this to throw an Error instead of a plain object - // eslint-disable-next-line no-throw-literal - throw { - data: {}, - errors: [error], - }; + throw createGraphQLResultWithError(new GraphQLApiError(NO_ENDPOINT)); } let response: any; try { + // See the inline doc of the REST `post()` API for possible errors to be thrown. + // As these errors are catastrophic they should be caught and handled by GraphQL + // API consumers. const { body: responseBody } = await this._api.post(amplify, { url: new AmplifyUrl(endpoint), options: { @@ -373,39 +386,20 @@ export class InternalGraphQLAPIClass { abortController, }); - const result = await responseBody.json(); - - response = result; - } catch (err) { - // If the exception is because user intentionally - // cancelled the request, do not modify the exception - // so that clients can identify the exception correctly. - if (this.isCancelError(err)) { - throw err; + response = await responseBody.json(); + } catch (error) { + if (this.isCancelError(error)) { + throw error; } - response = { - data: {}, - errors: [ - new GraphQLError( - (err as any).message, - null, - null, - null, - null, - err as any, - ), - ], - }; + response = createGraphQLResultWithError(error as any); } - const { errors } = response; - - if (errors && errors.length) { - throw repackageUnauthError(response); + if (isGraphQLResponseWithErrors(response)) { + throw repackageUnauthorizedError(response); } - return response; + return response as unknown as GraphQLResult; } /** @@ -463,7 +457,7 @@ export class InternalGraphQLAPIClass { .pipe( catchError(e => { if (e.errors) { - throw repackageUnauthError(e); + throw repackageUnauthorizedError(e); } throw e; }), diff --git a/packages/api-graphql/src/internals/utils/runtimeTypeGuards/isGraphQLResponseWithErrors.ts b/packages/api-graphql/src/internals/utils/runtimeTypeGuards/isGraphQLResponseWithErrors.ts new file mode 100644 index 00000000000..64efa23a2cb --- /dev/null +++ b/packages/api-graphql/src/internals/utils/runtimeTypeGuards/isGraphQLResponseWithErrors.ts @@ -0,0 +1,15 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { GraphQLResult } from '../../../types'; + +export function isGraphQLResponseWithErrors( + response: unknown, +): response is GraphQLResult { + if (!response) { + return false; + } + const r = response as GraphQLResult; + + return Array.isArray(r.errors) && r.errors.length > 0; +} diff --git a/packages/api-graphql/src/utils/errors/constants.ts b/packages/api-graphql/src/utils/errors/constants.ts new file mode 100644 index 00000000000..c621897ff08 --- /dev/null +++ b/packages/api-graphql/src/utils/errors/constants.ts @@ -0,0 +1,56 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { AmplifyErrorParams } from '@aws-amplify/core/internals/utils'; + +import { GraphQLAuthError } from '../../types'; + +export const NO_API_KEY: AmplifyErrorParams = { + name: 'NoApiKey', + // ideal: No API key configured. + message: GraphQLAuthError.NO_API_KEY, + recoverySuggestion: + 'The API request was made with `authMode: "apiKey"` but no API Key was passed into `Amplify.configure()`. Review if your API key is passed into the `Amplify.configure()` function.', +}; + +export const NO_VALID_CREDENTIALS: AmplifyErrorParams = { + name: 'NoCredentials', + // ideal: No auth credentials available. + message: GraphQLAuthError.NO_CREDENTIALS, + recoverySuggestion: `The API request was made with \`authMode: "iam"\` but no authentication credentials are available. + +If you intended to make a request using an authenticated role, review if your user is signed in before making the request. + +If you intend to make a request using an unauthenticated role or also known as "guest access", verify if "Auth.Cognito.allowGuestAccess" is set to "true" in the \`Amplify.configure()\` function.`, +}; + +export const NO_VALID_AUTH_TOKEN: AmplifyErrorParams = { + name: 'NoValidAuthTokens', + // ideal: No valid JWT auth token provided to make the API request.. + message: GraphQLAuthError.NO_FEDERATED_JWT, + recoverySuggestion: + 'If you intended to make an authenticated API request, review if the current user is signed in.', +}; + +export const NO_SIGNED_IN_USER: AmplifyErrorParams = { + name: 'NoSignedUser', + // ideal: Couldn't retrieve authentication credentials to make the API request. + message: GraphQLAuthError.NO_CURRENT_USER, + recoverySuggestion: + 'Review the underlying exception field for more details. If you intended to make an authenticated API request, review if the current user is signed in.', +}; + +export const NO_AUTH_TOKEN_HEADER: AmplifyErrorParams = { + name: 'NoAuthorizationHeader', + // ideal: Authorization header not specified. + message: GraphQLAuthError.NO_AUTH_TOKEN, + recoverySuggestion: + 'The API request was made with `authMode: "lambda"` but no `authToken` is set. Review if a valid authToken is passed into the request options or in the `Amplify.configure()` function.', +}; + +export const NO_ENDPOINT: AmplifyErrorParams = { + name: 'NoEndpoint', + message: 'No GraphQL endpoint configured in `Amplify.configure()`.', + recoverySuggestion: + 'Review if the GraphQL API endpoint is set in the `Amplify.configure()` function.', +}; diff --git a/packages/api-graphql/src/utils/errors/createGraphQLResultWithError.ts b/packages/api-graphql/src/utils/errors/createGraphQLResultWithError.ts new file mode 100644 index 00000000000..d53782731d1 --- /dev/null +++ b/packages/api-graphql/src/utils/errors/createGraphQLResultWithError.ts @@ -0,0 +1,15 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { GraphQLError } from 'graphql'; + +import { GraphQLResult } from '../../types'; + +export const createGraphQLResultWithError = ( + error: Error, +): GraphQLResult => { + return { + data: {} as T, + errors: [new GraphQLError(error.message, null, null, null, null, error)], + }; +}; diff --git a/packages/api-graphql/src/utils/errors/index.ts b/packages/api-graphql/src/utils/errors/index.ts index 54c8faa30a8..12d7659575d 100644 --- a/packages/api-graphql/src/utils/errors/index.ts +++ b/packages/api-graphql/src/utils/errors/index.ts @@ -4,3 +4,4 @@ export { GraphQLApiError } from './GraphQLApiError'; export { assertValidationError } from './assertValidationError'; export { APIValidationErrorCode, validationErrorMap } from './validation'; +export { createGraphQLResultWithError } from './createGraphQLResultWithError'; diff --git a/packages/api-graphql/src/utils/errors/repackageAuthError.ts b/packages/api-graphql/src/utils/errors/repackageAuthError.ts index e179c67436d..2a92c1f2f96 100644 --- a/packages/api-graphql/src/utils/errors/repackageAuthError.ts +++ b/packages/api-graphql/src/utils/errors/repackageAuthError.ts @@ -1,23 +1,21 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { AmplifyErrorParams } from '@aws-amplify/core/internals/utils'; - -interface ErrorObject { - errors: AmplifyErrorParams[]; -} +import { GraphQLResult } from '../../types'; /** * Checks to see if the given response or subscription message contains an - * unauth error. If it does, it changes the error message to include instructions + * Unauthorized error. If it does, it changes the error message to include instructions * for the app developer. */ -export function repackageUnauthError(content: T): T { +export function repackageUnauthorizedError>( + content: T, +): T { if (content.errors && Array.isArray(content.errors)) { content.errors.forEach(e => { - if (isUnauthError(e)) { + if (isUnauthorizedError(e)) { e.message = 'Unauthorized'; - e.recoverySuggestion = + (e as any).recoverySuggestion = `If you're calling an Amplify-generated API, make sure ` + `to set the "authMode" in generateClient({ authMode: '...' }) to the backend authorization ` + `rule's auth provider ('apiKey', 'userPool', 'iam', 'oidc', 'lambda')`; @@ -28,7 +26,7 @@ export function repackageUnauthError(content: T): T { return content; } -function isUnauthError(error: any): boolean { +function isUnauthorizedError(error: any): boolean { // Error pattern corresponding to appsync calls if (error?.originalError?.name?.startsWith('UnauthorizedException')) { return true; diff --git a/packages/api-graphql/tsconfig.json b/packages/api-graphql/tsconfig.json index 11d9d83dd6a..162af6598a0 100644 --- a/packages/api-graphql/tsconfig.json +++ b/packages/api-graphql/tsconfig.json @@ -2,7 +2,6 @@ "extends": "../../tsconfig.json", "compilerOptions": { "strictNullChecks": true, - "baseUrl": ".", "paths": { "@aws-amplify/data-schema-types": [ "../../node_modules/@aws-amplify/data-schema-types" diff --git a/packages/api-rest/src/apis/common/internalPost.ts b/packages/api-rest/src/apis/common/internalPost.ts index 4f7a6ccc416..835c3581d56 100644 --- a/packages/api-rest/src/apis/common/internalPost.ts +++ b/packages/api-rest/src/apis/common/internalPost.ts @@ -5,6 +5,7 @@ import { AmplifyClassV6 } from '@aws-amplify/core'; import { InternalPostInput, RestApiResponse } from '../../types'; import { createCancellableOperation } from '../../utils'; +import { CanceledError } from '../../errors'; import { transferHandler } from './handler'; @@ -23,6 +24,33 @@ const cancelTokenMap = new WeakMap, AbortController>(); /** * @internal + * + * REST POST handler to send GraphQL request to given endpoint. By default, it will use IAM to authorize + * the request. In some auth modes, the IAM auth has to be disabled. Here's how to set up the request auth correctly: + * * If auth mode is 'iam', you MUST NOT set 'authorization' header and 'x-api-key' header, since it would disable IAM + * auth. You MUST also set 'input.options.signingServiceInfo' option. + * * The including 'input.options.signingServiceInfo.service' and 'input.options.signingServiceInfo.region' are + * optional. If omitted, the signing service and region will be inferred from url. + * * If auth mode is 'none', you MUST NOT set 'options.signingServiceInfo' option. + * * If auth mode is 'apiKey', you MUST set 'x-api-key' custom header. + * * If auth mode is 'oidc' or 'lambda' or 'userPool', you MUST set 'authorization' header. + * + * To make the internal post cancellable, you must also call `updateRequestToBeCancellable()` with the promise from + * internal post call and the abort controller supplied to the internal post call. + * + * @param amplify the AmplifyClassV6 instance - it may be the singleton used on Web, or an instance created within + * a context created by `runWithAmplifyServerContext` + * @param postInput an object of {@link InternalPostInput} + * @param postInput.url The URL that the POST request sends to + * @param postInput.options Options of the POST request + * @param postInput.abortController The abort controller used to cancel the POST request + * @returns a {@link RestApiResponse} + * + * @throws an {@link Error} with `Network error` as the `message` when the external resource is unreachable due to one + * of the following reasons: + * 1. no network connection + * 2. CORS error + * @throws a {@link CanceledError} when the ongoing POST request get cancelled */ export const post = ( amplify: AmplifyClassV6, diff --git a/packages/api-rest/src/internals/index.ts b/packages/api-rest/src/internals/index.ts index 3ce95df5371..78ef4399d0f 100644 --- a/packages/api-rest/src/internals/index.ts +++ b/packages/api-rest/src/internals/index.ts @@ -1,26 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { post as internalPost } from '../apis/common/internalPost'; - -/** - * Internal-only REST POST handler to send GraphQL request to given endpoint. By default, it will use IAM to authorize - * the request. In some auth modes, the IAM auth has to be disabled. Here's how to set up the request auth correctly: - * * If auth mode is 'iam', you MUST NOT set 'authorization' header and 'x-api-key' header, since it would disable IAM - * auth. You MUST also set 'input.options.signingServiceInfo' option. - * * The including 'input.options.signingServiceInfo.service' and 'input.options.signingServiceInfo.region' are - * optional. If omitted, the signing service and region will be inferred from url. - * * If auth mode is 'none', you MUST NOT set 'options.signingServiceInfo' option. - * * If auth mode is 'apiKey', you MUST set 'x-api-key' custom header. - * * If auth mode is 'oidc' or 'lambda' or 'userPool', you MUST set 'authorization' header. - * - * To make the internal post cancellable, you must also call `updateRequestToBeCancellable()` with the promise from - * internal post call and the abort controller supplied to the internal post call. - * - * @internal - */ -export const post = internalPost; - +export { post } from '../apis/common/internalPost'; export { cancel, updateRequestToBeCancellable, diff --git a/packages/api-rest/src/utils/createCancellableOperation.ts b/packages/api-rest/src/utils/createCancellableOperation.ts index f75010faabe..f2ec4b5c714 100644 --- a/packages/api-rest/src/utils/createCancellableOperation.ts +++ b/packages/api-rest/src/utils/createCancellableOperation.ts @@ -67,6 +67,8 @@ export function createCancellableOperation( const canceledError = new CanceledError({ ...(message && { message }), underlyingError: error, + recoverySuggestion: + 'The API request was explicitly canceled. If this is not intended, validate if you called the `cancel()` function on the API request erroneously.', }); logger.debug(error); throw canceledError; diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index d6a2ee67a36..9fb7fe4eb3d 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -335,7 +335,7 @@ "name": "[API] generateClient (AppSync)", "path": "./dist/esm/api/index.mjs", "import": "{ generateClient }", - "limit": "38.00 kB" + "limit": "38.5 kB" }, { "name": "[API] REST API handlers",