Merge pull request #9410 from redaid113/persisted-query-graphqlError-bug

Consider `networkError.result.errors` in addition to `result.errors` in `PersistedQueryLink`
apollographql · May 2, 2022 · 66e1750 · 66e1750
2 parents 3578a2c + b0fe774
commit 66e1750
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## Apollo Client 3.6.2 (unreleased)
+
+- Consider `networkError.result.errors` in addition to `result.errors` in `PersistedQueryLink`. <br/>
+  [@redaid113](https://github.com/redaid113) and [@benjamn](https://github.com/benjamn) in [#9410](https://github.com/apollographql/apollo-client/pull/9410)
+
 ## Apollo Client 3.6.1 (2022-04-28)
 
 ### Bug Fixes

diff --git a/src/link/persisted-queries/__tests__/index.ts b/src/link/persisted-queries/__tests__/index.ts
@@ -489,4 +489,33 @@ describe('failure path', () => {
       },
     );
   });
+
+  itAsync('handles 400 response network error and graphql error without disabling persistedQuery support', (resolve, reject) => {
+    let failed = false;
+    fetch.mockResponseOnce(response);
+
+    const fetcher = (...args: any[]) => {
+      if (!failed) {
+        failed = true;
+        return Promise.resolve({
+          json: () => Promise.resolve(errorResponse),
+          text: () => Promise.resolve(errorResponse),
+          status: 400,
+        });
+      }
+      return fetch(...args);
+    };
+
+    const link = createPersistedQuery({ sha256 }).concat(
+      createHttpLink({ fetch: fetcher } as any),
+    );
+
+    execute(link, { query, variables }).subscribe(result => {
+      expect(result.data).toEqual(data);
+      const [, success] = fetch.mock.calls[0];
+      expect(JSON.parse(success!.body!.toString()).query).toBe(queryString);
+      expect(JSON.parse(success!.body!.toString()).extensions).not.toBeUndefined();
+      resolve();
+    }, reject);
+  });
 });
diff --git a/src/link/persisted-queries/index.ts b/src/link/persisted-queries/index.ts
@@ -13,13 +13,16 @@ import {
   Observer,
   ObservableSubscription,
   compact,
+  isNonEmptyArray,
 } from '../../utilities';
+import { NetworkError } from '../../errors';
+import { ServerError } from '../utils';
 
 export const VERSION = 1;
 
 export interface ErrorResponse {
   graphQLErrors?: readonly GraphQLError[];
-  networkError?: Error;
+  networkError?: NetworkError;
   response?: ExecutionResult;
   operation: Operation;
 }
@@ -46,18 +49,29 @@ export namespace PersistedQueryLink {
   export type Options = SHA256Options | GenerateHashOptions;
 }
 
+function collectErrorsByMessage<TError extends Error>(
+  graphQLErrors: TError[] | readonly TError[] | undefined,
+): Record<string, TError> {
+  const collected: Record<string, TError> = Object.create(null);
+  if (isNonEmptyArray(graphQLErrors)) {
+    graphQLErrors.forEach(error => collected[error.message] = error);
+  }
+  return collected;
+}
+
 const defaultOptions = {
   disable: ({ graphQLErrors, operation }: ErrorResponse) => {
+    const errorMessages = collectErrorsByMessage(graphQLErrors);
+
     // if the server doesn't support persisted queries, don't try anymore
-    if (
-      graphQLErrors &&
-      graphQLErrors.some(
-        ({ message }) => message === 'PersistedQueryNotSupported',
-      )
-    ) {
+    if (errorMessages.PersistedQueryNotSupported) {
       return true;
     }
 
+    if (errorMessages.PersistedQueryNotFound) {
+      return false;
+    }
+
     const { response } = operation.getContext();
     // if the server responds with bad request
     // apollo-server responds with 400 for GET and 500 for POST when no query is found
@@ -158,30 +172,41 @@ export const createPersistedQueryLink = (
         {
           response,
           networkError,
-        }: { response?: ExecutionResult; networkError?: Error },
+        }: { response?: ExecutionResult; networkError?: ServerError },
         cb: () => void,
       ) => {
         if (!retried && ((response && response.errors) || networkError)) {
           retried = true;
 
+          const graphQLErrors: GraphQLError[] = [];
+
+          const responseErrors = response && response.errors;
+          if (isNonEmptyArray(responseErrors)) {
+            graphQLErrors.push(...responseErrors);
+          }
+
+          // Network errors can return GraphQL errors on for example a 403
+          const networkErrors =
+            networkError &&
+            networkError.result &&
+            networkError.result.errors as GraphQLError[];
+          if (isNonEmptyArray(networkErrors)) {
+            graphQLErrors.push(...networkErrors);
+          }
+
           const disablePayload = {
             response,
             networkError,
             operation,
-            graphQLErrors: response ? response.errors : undefined,
+            graphQLErrors: isNonEmptyArray(graphQLErrors) ? graphQLErrors : void 0,
           };
 
           // if the server doesn't support persisted queries, don't try anymore
           supportsPersistedQueries = !disable(disablePayload);
 
           // if its not found, we can try it again, otherwise just report the error
           if (
-            (response &&
-              response.errors &&
-              response.errors.some(
-                ({ message }: { message: string }) =>
-                  message === 'PersistedQueryNotFound',
-              )) ||
+            collectErrorsByMessage(graphQLErrors).PersistedQueryNotFound ||
             !supportsPersistedQueries
           ) {
             // need to recall the link chain
@@ -213,7 +238,7 @@ export const createPersistedQueryLink = (
         next: (response: ExecutionResult) => {
           retry({ response }, () => observer.next!(response));
         },
-        error: (networkError: Error) => {
+        error: (networkError: ServerError) => {
           retry({ networkError }, () => observer.error!(networkError));
         },
         complete: observer.complete!.bind(observer),