From e3d611daf7a014e5c92d6bed75d67b9187437eda Mon Sep 17 00:00:00 2001
From: Cayla Hamann <38332422+caylahamann@users.noreply.github.com>
Date: Thu, 27 Jul 2023 19:43:45 -0400
Subject: [PATCH] fix: useMutation calls onError when errorPolicy is all
 (#11103)

---
 .changeset/early-pumpkins-type.md             |  5 ++
 .size-limit.cjs                               |  2 +-
 .../hooks/__tests__/useMutation.test.tsx      | 82 +++++++++++++++++++
 src/react/hooks/useMutation.ts                | 11 ++-
 4 files changed, 98 insertions(+), 2 deletions(-)
 create mode 100644 .changeset/early-pumpkins-type.md

diff --git a/.changeset/early-pumpkins-type.md b/.changeset/early-pumpkins-type.md
new file mode 100644
index 00000000000..d706a6d1f68
--- /dev/null
+++ 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
index 9de3cde6686..54cacceb903 100644
--- 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
index c3167a60164..63d73194b09 100644
--- 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
index 813db9b5a7e..f2ecd587b7e 100644
--- 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) => {