fix: useMutation calls onError when errorPolicy is all (#11103)

apollographql · Jul 27, 2023 · e3d611d · e3d611d
1 parent 506a14c
commit e3d611d
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 2 deletions.
diff --git a/.changeset/early-pumpkins-type.md b/.changeset/early-pumpkins-type.md
@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fixes a bug in `useMutation` so that `onError` is called when an error is returned from the request with `errorPolicy` set to 'all' .
diff --git a/.size-limit.cjs b/.size-limit.cjs
@@ -1,7 +1,7 @@
 const checks = [
   {
     path: "dist/apollo-client.min.cjs",
-    limit: "38000"
+    limit: "38042"
   },
   {
     path: "dist/main.cjs",

diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx
@@ -437,6 +437,50 @@ describe('useMutation Hook', () => {
         expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR);
       });
 
+      it(`should call onError when errorPolicy is 'all'`, async () => {
+        const variables = {
+          description: 'Get milk!'
+        };
+
+        const mocks = [
+          {
+            request: {
+              query: CREATE_TODO_MUTATION,
+              variables
+            },
+            result: {
+              data: CREATE_TODO_RESULT,
+              errors: [new GraphQLError(CREATE_TODO_ERROR)],
+            },
+          }
+        ];
+
+        const onError = jest.fn();
+        const onCompleted = jest.fn();
+
+        const { result } = renderHook(
+          () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: 'all', onError, onCompleted }),
+          { wrapper: ({ children }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )},
+        );
+
+        const createTodo = result.current[0];
+
+        let fetchResult: any;
+        await act(async () => {
+          fetchResult = await createTodo({ variables });
+        });
+
+        expect(fetchResult.data).toEqual(CREATE_TODO_RESULT);
+        expect(fetchResult.errors[0].message).toEqual(CREATE_TODO_ERROR);
+        expect(onError).toHaveBeenCalledTimes(1);
+        expect(onError.mock.calls[0][0].message).toBe(CREATE_TODO_ERROR);
+        expect(onCompleted).not.toHaveBeenCalled();
+      });
+
       it(`should ignore errors when errorPolicy is 'ignore'`, async () => {
         const errorMock = jest.spyOn(console, "error")
           .mockImplementation(() => {});
@@ -476,6 +520,44 @@ describe('useMutation Hook', () => {
         expect(errorMock.mock.calls[0][0]).toMatch("Missing field");
         errorMock.mockRestore();
       });
+
+      it(`should not call onError when errorPolicy is 'ignore'`, async () => {
+        const variables = {
+          description: 'Get milk!'
+        };
+
+        const mocks = [
+          {
+            request: {
+              query: CREATE_TODO_MUTATION,
+              variables,
+            },
+            result: {
+              errors: [new GraphQLError(CREATE_TODO_ERROR)],
+            },
+          }
+        ];
+
+        const onError = jest.fn();
+
+        const { result } = renderHook(
+          () => useMutation(CREATE_TODO_MUTATION, { errorPolicy: "ignore", onError }),
+          { wrapper: ({ children }) => (
+            <MockedProvider mocks={mocks}>
+              {children}
+            </MockedProvider>
+          )},
+        );
+
+        const createTodo = result.current[0];
+        let fetchResult: any;
+        await act(async () => {
+          fetchResult = await createTodo({ variables });
+        });
+
+        expect(fetchResult).toEqual({});
+        expect(onError).not.toHaveBeenCalled();
+      });
     });
 
     it('should return the current client instance in the result object', async () => {

diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts
@@ -88,6 +88,12 @@ export function useMutation<
           ? new ApolloError({ graphQLErrors: errors })
           : void 0;
 
+      const onError = executeOptions.onError || ref.current.options?.onError
+
+      if (error && onError) {
+        onError(error, clientOptions);
+      }
+
       if (
         mutationId === ref.current.mutationId &&
         !clientOptions.ignoreResults
@@ -106,7 +112,10 @@ export function useMutation<
       }
 
       const onCompleted = executeOptions.onCompleted || ref.current.options?.onCompleted
-      onCompleted?.(response.data!, clientOptions);
+
+      if (!error) {
+        onCompleted?.(response.data!, clientOptions);
+      }
 
       return response;
     }).catch((error) => {