Respect errorPolicy for mutation and subscription results. (#7003)

Using the default ErrorPolicy of "none" for queries means no data will be written to the cache when a GraphQL result has errors. Queries can use errorPolicy:"ignore" and errorPolicy:"all" to ensure data is written to the cache in spite of GraphQL errors, but mutations and subscriptions were previously limited to the default policy, "none". This commit makes mutations and subscriptions respect the non-default "ignore" and "all" ErrorPolicy values, just as queries do, hopefully addressing #6965.
apollographql · Sep 11, 2020 · 62facae · 62facae
1 parent f2b60c5
commit 62facae
Show file tree

Hide file tree

Showing 6 changed files with 166 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -42,6 +42,9 @@
 - Move `apollo-link-persisted-queries` implementation to `@apollo/client/link/persisted-queries`. Try running our [automated imports transform](https://github.com/apollographql/apollo-client/tree/main/codemods/ac2-to-ac3) to handle this conversion, if you're using `apollo-link-persisted-queries`. <br/>
   [@hwillson](https://github.com/hwillson) in [#6837](https://github.com/apollographql/apollo-client/pull/6837)
 
+- Support non-default `ErrorPolicy` values (that is, `"ignore"` and `"all"`, in addition to the default value `"none"`) for mutations and subscriptions, like we do for queries. <br/>
+  [@benjamn](https://github.com/benjamn) in [#7003](https://github.com/apollographql/apollo-client/pull/7003)
+
 - Remove invariant forbidding a `FetchPolicy` of `cache-only` in `ObservableQuery#refetch`. <br/>
   [@benjamn](https://github.com/benjamn) in [ccb0a79a](https://github.com/apollographql/apollo-client/pull/6774/commits/ccb0a79a588721f08bf87a131c31bf37fa3238e5), fixing [#6702](https://github.com/apollographql/apollo-client/issues/6702)
 

diff --git a/src/__tests__/__snapshots__/mutationResults.ts.snap b/src/__tests__/__snapshots__/mutationResults.ts.snap
@@ -0,0 +1,40 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`mutation results should write results to cache according to errorPolicy 1`] = `Object {}`;
+
+exports[`mutation results should write results to cache according to errorPolicy 2`] = `
+Object {
+  "Person:{\\"name\\":\\"Jenn Creighton\\"}": Object {
+    "__typename": "Person",
+    "name": "Jenn Creighton",
+  },
+  "ROOT_MUTATION": Object {
+    "__typename": "Mutation",
+    "newPerson({\\"name\\":\\"Jenn Creighton\\"})": Object {
+      "__ref": "Person:{\\"name\\":\\"Jenn Creighton\\"}",
+    },
+  },
+}
+`;
+
+exports[`mutation results should write results to cache according to errorPolicy 3`] = `
+Object {
+  "Person:{\\"name\\":\\"Ellen Shapiro\\"}": Object {
+    "__typename": "Person",
+    "name": "Ellen Shapiro",
+  },
+  "Person:{\\"name\\":\\"Jenn Creighton\\"}": Object {
+    "__typename": "Person",
+    "name": "Jenn Creighton",
+  },
+  "ROOT_MUTATION": Object {
+    "__typename": "Mutation",
+    "newPerson({\\"name\\":\\"Ellen Shapiro\\"})": Object {
+      "__ref": "Person:{\\"name\\":\\"Ellen Shapiro\\"}",
+    },
+    "newPerson({\\"name\\":\\"Jenn Creighton\\"})": Object {
+      "__ref": "Person:{\\"name\\":\\"Jenn Creighton\\"}",
+    },
+  },
+}
+`;
diff --git a/src/__tests__/mutationResults.ts b/src/__tests__/mutationResults.ts
@@ -1,5 +1,6 @@
 import { cloneDeep } from 'lodash';
 import gql from 'graphql-tag';
+import { GraphQLError } from 'graphql';
 
 import { ApolloClient } from '../core';
 import { InMemoryCache } from '../cache';
@@ -306,6 +307,99 @@ describe('mutation results', () => {
     });
   });
 
+  itAsync("should write results to cache according to errorPolicy", async (resolve, reject) => {
+    const expectedFakeError = new GraphQLError("expected/fake error");
+
+    const client = new ApolloClient({
+      cache: new InMemoryCache({
+        typePolicies: {
+          Person: {
+            keyFields: ["name"],
+          },
+        },
+      }),
+
+      link: new ApolloLink(operation => new Observable(observer => {
+        observer.next({
+          errors: [
+            expectedFakeError,
+          ],
+          data: {
+            newPerson: {
+              __typename: "Person",
+              name: operation.variables.newName,
+            },
+          },
+        });
+        observer.complete();
+      })).setOnError(reject),
+    });
+
+    const mutation = gql`
+      mutation AddNewPerson($newName: String!) {
+        newPerson(name: $newName) {
+          name
+        }
+      }
+    `;
+
+    await client.mutate({
+      mutation,
+      variables: {
+        newName: "Hugh Willson",
+      },
+    }).then(() => {
+      reject("should have thrown for default errorPolicy");
+    }, error => {
+      expect(error.message).toBe(expectedFakeError.message);
+    });
+
+    expect(client.cache.extract()).toMatchSnapshot();
+
+    const ignoreErrorsResult = await client.mutate({
+      mutation,
+      errorPolicy: "ignore",
+      variables: {
+        newName: "Jenn Creighton",
+      },
+    });
+
+    expect(ignoreErrorsResult).toEqual({
+      data: {
+        newPerson: {
+          __typename: "Person",
+          name: "Jenn Creighton",
+        },
+      },
+    });
+
+    expect(client.cache.extract()).toMatchSnapshot();
+
+    const allErrorsResult = await client.mutate({
+      mutation,
+      errorPolicy: "all",
+      variables: {
+        newName: "Ellen Shapiro",
+      },
+    });
+
+    expect(allErrorsResult).toEqual({
+      data: {
+        newPerson: {
+          __typename: "Person",
+          name: "Ellen Shapiro",
+        },
+      },
+      errors: [
+        expectedFakeError,
+      ],
+    });
+
+    expect(client.cache.extract()).toMatchSnapshot();
+
+    resolve();
+  });
+
   itAsync("should warn when the result fields don't match the query fields", (resolve, reject) => {
     let handle: any;
     let subscriptionHandle: Subscription;

diff --git a/src/core/QueryInfo.ts b/src/core/QueryInfo.ts
@@ -2,7 +2,7 @@ import { DocumentNode, GraphQLError } from 'graphql';
 import { equal } from "@wry/equality";
 
 import { Cache, ApolloCache } from '../cache';
-import { WatchQueryOptions } from './watchQueryOptions';
+import { WatchQueryOptions, ErrorPolicy } from './watchQueryOptions';
 import { ObservableQuery } from './ObservableQuery';
 import { QueryListener } from './types';
 import { FetchResult } from '../link/core';
@@ -288,15 +288,7 @@ export class QueryInfo {
       this.diff = { result: result.data, complete: true };
 
     } else if (allowCacheWrite) {
-      const ignoreErrors =
-        options.errorPolicy === 'ignore' ||
-        options.errorPolicy === 'all';
-      let writeWithErrors = !graphQLResultHasError(result);
-      if (!writeWithErrors && ignoreErrors && result.data) {
-        writeWithErrors = true;
-      }
-
-      if (writeWithErrors) {
+      if (shouldWriteResult(result, options.errorPolicy)) {
         // Using a transaction here so we have a chance to read the result
         // back from the cache before the watch callback fires as a result
         // of writeQuery, so we can store the new diff quietly and ignore
@@ -405,3 +397,17 @@ export class QueryInfo {
     return error;
   }
 }
+
+export function shouldWriteResult<T>(
+  result: FetchResult<T>,
+  errorPolicy: ErrorPolicy = "none",
+) {
+  const ignoreErrors =
+    errorPolicy === "ignore" ||
+    errorPolicy === "all";
+  let writeWithErrors = !graphQLResultHasError(result);
+  if (!writeWithErrors && ignoreErrors && result.data) {
+    writeWithErrors = true;
+  }
+  return writeWithErrors;
+}
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
@@ -39,7 +39,7 @@ import {
 } from './types';
 import { LocalState } from './LocalState';
 
-import { QueryInfo, QueryStoreValue } from './QueryInfo';
+import { QueryInfo, QueryStoreValue, shouldWriteResult } from './QueryInfo';
 
 const { hasOwnProperty } = Object.prototype;
 
@@ -192,6 +192,7 @@ export class QueryManager<TStore> {
             result: { data: optimistic },
             document: mutation,
             variables: variables,
+            errorPolicy,
             queryUpdatersById: generateUpdateQueriesInfo(),
             update: updateWithProxyFn,
           }, cache);
@@ -235,6 +236,7 @@ export class QueryManager<TStore> {
                 result,
                 document: mutation,
                 variables,
+                errorPolicy,
                 queryUpdatersById: generateUpdateQueriesInfo(),
                 update: updateWithProxyFn,
               }, self.cache);
@@ -588,6 +590,7 @@ export class QueryManager<TStore> {
   public startGraphQLSubscription<T = any>({
     query,
     fetchPolicy,
+    errorPolicy,
     variables,
     context = {},
   }: SubscriptionOptions): Observable<FetchResult<T>> {
@@ -601,10 +604,10 @@ export class QueryManager<TStore> {
         variables,
         false,
       ).map(result => {
-        if (!fetchPolicy || fetchPolicy !== 'no-cache') {
+        if (fetchPolicy !== 'no-cache') {
           // the subscription interface should handle not sending us results we no longer subscribe to.
           // XXX I don't think we ever send in an object with errors, but we might in the future...
-          if (!graphQLResultHasError(result)) {
+          if (shouldWriteResult(result, errorPolicy)) {
             this.cache.write({
               query,
               result: result.data,
@@ -1078,6 +1081,7 @@ function markMutationResult<TStore, TData>(
     result: FetchResult<TData>;
     document: DocumentNode;
     variables: any;
+    errorPolicy: ErrorPolicy;
     queryUpdatersById: Record<string, QueryWithUpdater>;
     update:
       ((cache: ApolloCache<TStore>, mutationResult: Object) => void) |
@@ -1086,7 +1090,7 @@ function markMutationResult<TStore, TData>(
   cache: ApolloCache<TStore>,
 ) {
   // Incorporate the result from this mutation into the store
-  if (!graphQLResultHasError(mutation.result)) {
+  if (shouldWriteResult(mutation.result, mutation.errorPolicy)) {
     const cacheWrites: Cache.WriteOptions[] = [{
       result: mutation.result.data,
       dataId: 'ROOT_MUTATION',

diff --git a/src/core/watchQueryOptions.ts b/src/core/watchQueryOptions.ts
@@ -169,6 +169,11 @@ export interface SubscriptionOptions<TVariables = OperationVariables, TData = an
    */
   fetchPolicy?: FetchPolicy;
 
+  /**
+   * Specifies the {@link ErrorPolicy} to be used for this operation
+   */
+  errorPolicy?: ErrorPolicy;
+
   /**
    * Context object to be passed through the link execution chain.
    */