Unify error behavior for queries (#12457)

Author: jerelmiller

.changeset/four-ghosts-watch.md

@@ -0,0 +1,5 @@
+---
+"@apollo/client": major
+---
+
+Network errors triggered by queries now adhere to the `errorPolicy`. This means that GraphQL errors and network errors now behave the same way. Previously promise-based APIs, such as `client.query`, would reject the promise with the network error even if `errorPolicy` was set to `ignore`. The promise is now resolved with the `error` property set to the network error instead.

.size-limits.json

@@ -1,6 +1,6 @@
 {
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 42028,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 37470,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32524,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27415
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 42138,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 37491,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32521,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27439
 }

src/__tests__/client.ts

@@ -7,6 +7,7 @@ import {
   print,
   visit,
 } from "graphql";
+import { GraphQLFormattedError } from "graphql";
 import { gql } from "graphql-tag";
 import { assign, cloneDeep } from "lodash";
 import { EMPTY, EmptyError, Observable, of, Subscription } from "rxjs";
@@ -2636,6 +2637,152 @@ describe("client", () => {
     );
   });
 
+  it("rejects network errors", async () => {
+    const query = gql`
+      query {
+        posts {
+          foo
+          __typename
+        }
+      }
+    `;
+    const error = new Error("Oops");
+    const link = mockSingleLink({
+      request: { query },
+      error,
+    });
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    await expect(client.query({ query })).rejects.toThrow(error);
+  });
+
+  it("resolves partial data and GraphQL errors when errorPolicy is 'all'", async () => {
+    const query = gql`
+      query {
+        posts {
+          foo
+          __typename
+        }
+      }
+    `;
+    const errors: GraphQLFormattedError[] = [
+      { message: 'Cannot query field "foo" on type "Post".' },
+    ];
+    const link = mockSingleLink({
+      request: { query },
+      result: { data: { posts: null }, errors },
+    });
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    await expect(
+      client.query({ query, errorPolicy: "all" })
+    ).resolves.toEqualApolloQueryResult({
+      data: { posts: null },
+      error: new CombinedGraphQLErrors([
+        { message: 'Cannot query field "foo" on type "Post".' },
+      ]),
+      loading: false,
+      networkStatus: NetworkStatus.error,
+      partial: false,
+    });
+  });
+
+  it("resolves with network error when errorPolicy is 'all'", async () => {
+    const query = gql`
+      query {
+        posts {
+          foo
+          __typename
+        }
+      }
+    `;
+    const error = new Error("Oops");
+    const link = mockSingleLink({
+      request: { query },
+      error,
+    });
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    await expect(
+      client.query({ query, errorPolicy: "all" })
+    ).resolves.toEqualApolloQueryResult({
+      data: undefined,
+      error,
+      loading: false,
+      networkStatus: NetworkStatus.error,
+      partial: true,
+    });
+  });
+
+  it("resolves partial data and strips errors when errorPolicy is 'ignore'", async () => {
+    const query = gql`
+      query {
+        posts {
+          foo
+          __typename
+        }
+      }
+    `;
+    const errors: GraphQLFormattedError[] = [
+      { message: 'Cannot query field "foo" on type "Post".' },
+    ];
+    const link = mockSingleLink({
+      request: { query },
+      result: { data: { posts: null }, errors },
+    });
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    await expect(
+      client.query({ query, errorPolicy: "ignore" })
+    ).resolves.toEqualApolloQueryResult({
+      data: { posts: null },
+      loading: false,
+      networkStatus: NetworkStatus.ready,
+      partial: false,
+    });
+  });
+
+  it("resolves with no data or errors for network error when errorPolicy is 'ignore'", async () => {
+    const query = gql`
+      query {
+        posts {
+          foo
+          __typename
+        }
+      }
+    `;
+    const error = new Error("Oops");
+    const link = mockSingleLink({
+      request: { query },
+      error,
+    });
+    const client = new ApolloClient({
+      link,
+      cache: new InMemoryCache(),
+    });
+
+    await expect(
+      client.query({ query, errorPolicy: "ignore" })
+    ).resolves.toEqualApolloQueryResult({
+      data: undefined,
+      loading: false,
+      networkStatus: NetworkStatus.ready,
+      partial: true,
+    });
+  });
+
   it("should warn if server returns wrong data", async () => {
     using _consoleSpies = spyOnConsole.takeSnapshots("error");
     const query = gql`

src/core/QueryManager.ts

@@ -1210,6 +1210,7 @@ export class QueryManager {
     >
   ): Observable<ApolloQueryResult<TData>> {
     const requestId = (queryInfo.lastRequestId = this.generateRequestId());
+    const { errorPolicy } = options;
 
     // Performing transformForLink here gives this.cache a chance to fill in
     // missing fragment definitions (for example) before sending this document
@@ -1221,21 +1222,9 @@ export class QueryManager {
       options.context,
       options.variables
     ).pipe(
-      catchError((error) => {
-        error = maybeWrapError(error);
-
-        // Avoid storing errors from older interrupted queries.
-        if (requestId >= queryInfo.lastRequestId) {
-          queryInfo.resetLastWrite();
-          queryInfo.reset();
-        }
-
-        throw error;
-      }),
       map((result) => {
         const graphQLErrors = getGraphQLErrorsFromResult(result);
         const hasErrors = graphQLErrors.length > 0;
-        const { errorPolicy } = options;
 
         // If we interrupted this request by calling getResultsFromLink again
         // with the same QueryInfo object, we ignore the old results.
@@ -1278,6 +1267,30 @@ export class QueryManager {
         }
 
         return aqr;
+      }),
+      catchError((error) => {
+        error = maybeWrapError(error);
+
+        // Avoid storing errors from older interrupted queries.
+        if (requestId >= queryInfo.lastRequestId && errorPolicy === "none") {
+          queryInfo.resetLastWrite();
+          queryInfo.reset();
+          throw error;
+        }
+
+        const aqr: ApolloQueryResult<TData> = {
+          data: undefined,
+          loading: false,
+          networkStatus: NetworkStatus.ready,
+          partial: true,
+        };
+
+        if (errorPolicy !== "ignore") {
+          aqr.error = error;
+          aqr.networkStatus = NetworkStatus.error;
+        }
+
+        return of(aqr);
       })
     );
   }

src/core/__tests__/ApolloClient/general.test.ts

@@ -280,6 +280,61 @@ describe("ApolloClient", () => {
     await expect(stream).not.toEmitAnything();
   });
 
+  it("handles network errors with errorPolicy: 'all'", async () => {
+    const stream = getObservableStream({
+      query: gql`
+        query people {
+          allPeople(first: 1) {
+            people {
+              name
+            }
+          }
+        }
+      `,
+      queryOptions: {
+        errorPolicy: "all",
+      },
+      error: new Error("Network error"),
+    });
+
+    await expect(stream).toEmitApolloQueryResult({
+      data: undefined,
+      error: new Error("Network error"),
+      loading: false,
+      networkStatus: NetworkStatus.error,
+      partial: true,
+    });
+
+    await expect(stream).not.toEmitAnything();
+  });
+
+  it("handles network errors with errorPolicy: 'ignore'", async () => {
+    const stream = getObservableStream({
+      query: gql`
+        query people {
+          allPeople(first: 1) {
+            people {
+              name
+            }
+          }
+        }
+      `,
+      queryOptions: {
+        errorPolicy: "ignore",
+      },
+      error: new Error("Network error"),
+    });
+
+    await expect(stream).toEmitApolloQueryResult({
+      data: undefined,
+      loading: false,
+      networkStatus: NetworkStatus.ready,
+      partial: true,
+    });
+
+    await expect(stream).not.toEmitAnything();
+  });
+
   // Determine how/if we want to change this at all. ObservableQuery no longer
   // emits errors to its observers and instead emits a `next` event with an
   // `error` property.

src/react/hooks/__tests__/useLazyQuery.test.tsx

@@ -1783,12 +1783,6 @@ describe("useLazyQuery Hook", () => {
       });
     }
 
-    // TODO: Determine if this is the correct behavior. This is different than
-    // 3.x where this resolves with an `ApolloQueryResult`.
-    // https://github.com/apollographql/apollo-client/issues/10787 wants this
-    // behavior
-    // https://github.com/apollographql/apollo-client/issues/9142#issuecomment-1118972947
-    // justifies the old behavior
     await expect(execute()).rejects.toEqual(
       new CombinedGraphQLErrors([{ message: "error 1" }])
     );
@@ -2721,9 +2715,6 @@ describe("useLazyQuery Hook", () => {
       await expect(takeSnapshot).not.toRerender();
     });
 
-    // If there was any data to report, errorPolicy:"all" would report both
-    // result.data and result.error, but there is no GraphQL data when we
-    // encounter a network error, so the test again captures desired behavior.
     it('handles errorPolicy:"all" appropriately', async () => {
       const networkError = new Error("from the network");
 
@@ -2768,7 +2759,13 @@ describe("useLazyQuery Hook", () => {
 
       const [execute] = getCurrentSnapshot();
 
-      await expect(execute()).rejects.toEqual(networkError);
+      await expect(execute()).resolves.toEqualApolloQueryResult({
+        data: undefined,
+        error: networkError,
+        loading: false,
+        networkStatus: NetworkStatus.error,
+        partial: true,
+      });
 
       {
         const [, result] = await takeSnapshot();
@@ -2787,9 +2784,6 @@ describe("useLazyQuery Hook", () => {
       await expect(takeSnapshot).not.toRerender();
     });
 
-    // Technically errorPolicy:"ignore" is supposed to throw away result.error,
-    // but in the case of network errors, since there's no actual data to
-    // report, it's useful/important that we report result.error anyway.
     it('handles errorPolicy:"ignore" appropriately', async () => {
       const networkError = new Error("from the network");
 
@@ -2834,17 +2828,21 @@ describe("useLazyQuery Hook", () => {
 
       const [execute] = getCurrentSnapshot();
 
-      await expect(execute()).rejects.toEqual(networkError);
+      await expect(execute()).resolves.toEqualApolloQueryResult({
+        data: undefined,
+        loading: false,
+        networkStatus: NetworkStatus.ready,
+        partial: true,
+      });
 
       {
         const [, result] = await takeSnapshot();
 
         expect(result).toEqualLazyQueryResult({
           data: undefined,
-          error: networkError,
           called: true,
           loading: false,
-          networkStatus: NetworkStatus.error,
+          networkStatus: NetworkStatus.ready,
           previousData: undefined,
           variables: {},
         });