Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: useMutation calls onError if errorPolicy is all #11103

5 changes: 5 additions & 0 deletions .changeset/early-pumpkins-type.md
Original file line number Diff line number Diff line change
@@ -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' .
2 changes: 1 addition & 1 deletion .size-limit.cjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const checks = [
{
path: "dist/apollo-client.min.cjs",
limit: "38000"
limit: "38042"
},
{
path: "dist/main.cjs",
Expand Down
82 changes: 82 additions & 0 deletions src/react/hooks/__tests__/useMutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

});

it(`should ignore errors when errorPolicy is 'ignore'`, async () => {
const errorMock = jest.spyOn(console, "error")
.mockImplementation(() => {});
Expand Down Expand Up @@ -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 () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for adding this test!

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 () => {
Expand Down
11 changes: 10 additions & 1 deletion src/react/hooks/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) => {
Expand Down