fix(api-graphql): update error handling

- Now throw the catastrophic errors thrown from the `post()` API 1. Network connection or CORS caused Network error 2. The cancellation error - Now throw an error when there is no endpoint configured - Using `GraphQLApiError` to throw Auth related errors instead of `Error`
aws-amplify · Mar 25, 2024 · e734eb0 · e734eb0
1 parent 701f219
commit e734eb0
Show file tree

Hide file tree

Showing 14 changed files with 264 additions and 130 deletions.
diff --git a/packages/api-graphql/__tests__/GraphQLAPI.test.ts b/packages/api-graphql/__tests__/GraphQLAPI.test.ts
@@ -5,15 +5,19 @@ import { Amplify as AmplifyCore } from '@aws-amplify/core';
 import * as typedQueries from './fixtures/with-types/queries';
 import { expectGet } from './utils/expects';
 
-import {
-	__amplify,
-	GraphQLResult,
-	GraphQLAuthError,
-	V6Client,
-} from '../src/types';
+import { __amplify, GraphQLResult, V6Client } from '../src/types';
 import { GetThreadQuery } from './fixtures/with-types/API';
 import { AWSAppSyncRealTimeProvider } from '../src/Providers/AWSAppSyncRealTimeProvider';
 import { Observable, of } from 'rxjs';
+import {
+	NO_API_KEY,
+	NO_AUTH_TOKEN_HEADER,
+	NO_ENDPOINT,
+	NO_SIGNED_IN_USER,
+	NO_VALID_AUTH_TOKEN,
+	NO_VALID_CREDENTIALS,
+} from '../src/utils/errors/constants';
+import { NetworkError } from '@aws-amplify/core/internals/utils';
 
 const serverManagedFields = {
 	id: 'some-id',
@@ -54,6 +58,9 @@ jest.mock('aws-amplify', () => {
 	return mockedModule;
 });
 
+const mockFetchAuthSession = (Amplify as any).Auth
+	.fetchAuthSession as jest.Mock;
+
 const client = {
 	[__amplify]: Amplify,
 	graphql,
@@ -762,8 +769,7 @@ describe('API test', () => {
 		});
 
 		test('multi-auth default case api-key, OIDC as auth mode, but no federatedSign', async () => {
-			const prevMockAccessToken = mockAccessToken;
-			mockAccessToken = null;
+			mockFetchAuthSession.mockRejectedValueOnce(new Error('some error'));
 
 			Amplify.configure({
 				API: {
@@ -805,10 +811,7 @@ describe('API test', () => {
 					variables: graphqlVariables,
 					authMode: 'oidc',
 				}),
-			).rejects.toThrow('No current user');
-
-			// Cleanup:
-			mockAccessToken = prevMockAccessToken;
+			).rejects.toThrow(NO_SIGNED_IN_USER.message);
 		});
 
 		test('multi-auth using CUP as auth mode, but no userpool', async () => {
@@ -882,7 +885,7 @@ describe('API test', () => {
 					variables: graphqlVariables,
 					authMode: 'lambda',
 				}),
-			).rejects.toThrow(GraphQLAuthError.NO_AUTH_TOKEN);
+			).rejects.toThrow(NO_AUTH_TOKEN_HEADER.message);
 		});
 
 		test('multi-auth using API_KEY as auth mode, but no api-key configured', async () => {
@@ -904,7 +907,7 @@ describe('API test', () => {
 					variables: graphqlVariables,
 					authMode: 'apiKey',
 				}),
-			).rejects.toThrow(GraphQLAuthError.NO_API_KEY);
+			).rejects.toThrow(NO_API_KEY.message);
 		});
 
 		test('multi-auth using AWS_IAM as auth mode, but no credentials', async () => {
@@ -930,7 +933,7 @@ describe('API test', () => {
 					variables: graphqlVariables,
 					authMode: 'iam',
 				}),
-			).rejects.toThrow(GraphQLAuthError.NO_CREDENTIALS);
+			).rejects.toThrow(NO_VALID_CREDENTIALS.message);
 
 			// Cleanup:
 			mockCredentials = prevMockCredentials;
@@ -1342,5 +1345,77 @@ describe('API test', () => {
 				},
 			);
 		});
+
+		test('throws error when endpoint is not configured', () => {
+			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.toThrow(NO_ENDPOINT.message);
+		});
+
+		test('throws error when fetchAuthSession completes without errors but returns no tokens (unauthenticated access is enabled)', () => {
+			mockFetchAuthSession.mockResolvedValueOnce({
+				tokens: undefined,
+			});
+
+			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.toThrow(NO_VALID_AUTH_TOKEN.message);
+		});
+
+		test('rethrow api-rest post() API thrown NetworkError', () => {
+			jest
+				.spyOn((raw.GraphQLAPI as any)._api, 'post')
+				.mockRejectedValueOnce(new NetworkError('Network error'));
+
+			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.toThrow('Network error');
+		});
 	});
 });
diff --git a/packages/api-graphql/__tests__/internals/generateClient.test.ts b/packages/api-graphql/__tests__/internals/generateClient.test.ts
@@ -5585,14 +5585,11 @@ describe('generateClient', () => {
 				amplify: Amplify,
 			});
 
-			const { data, errors } = await client.queries.echo({
-				argumentContent: 'echo argumentContent value',
-			});
-
-			// TODO: data should actually be null/undefined, pending discussion and fix.
-			// This is not strictly related to custom ops.
-			expect(data).toEqual({});
-			expect(errors).toEqual([{ message: 'Network error' }]);
+			expect(() =>
+				client.queries.echo({
+					argumentContent: 'echo argumentContent value',
+				}),
+			).rejects.toThrow('Network error');
 		});
 
 		test('core error handling', async () => {

diff --git 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 } 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,40 @@ 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);
+					throw new GraphQLApiError({
+						...NO_SIGNED_IN_USER,
+						underlyingError: e,
+					});
+				}
+
+				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,62 +362,30 @@ 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 new GraphQLApiError(NO_ENDPOINT);
 		}
 
-		let response: any;
-
-		try {
-			const { body: responseBody } = await this._api.post(amplify, {
-				url: new AmplifyUrl(endpoint),
-				options: {
-					headers,
-					body,
-					signingServiceInfo,
-					withCredentials,
-				},
-				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 = {
-				data: {},
-				errors: [
-					new GraphQLError(
-						(err as any).message,
-						null,
-						null,
-						null,
-						null,
-						err as any,
-					),
-				],
-			};
-		}
-
-		const { errors } = response;
-
-		if (errors && errors.length) {
-			throw repackageUnauthError(response);
+		// 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: {
+				headers,
+				body,
+				signingServiceInfo,
+				withCredentials,
+			},
+			abortController,
+		});
+
+		const response = await responseBody.json();
+
+		if (isGraphQLResponseWithErrors(response)) {
+			throw repackageUnauthorizedError(response);
 		}
 
-		return response;
+		return response as unknown as GraphQLResult<T>;
 	}
 
 	/**
@@ -463,7 +443,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
@@ -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;
+}