[Question] Syntax Errors in onCompleted causes onError to occur

[dep]

We ran into this in our platform this week and we thought it was interesting. Essentially a syntax error in onCompleted caused the JS to run onError. Here's an example:

const [createDashboard, { loading: createLoading }] = useMutation(CreateDashboard, {
  // Mutation was successful so onCompleted fires...
  onCompleted({ 
    createDashboard: { 
      dashboard: { dashboardUrl } 
    } 
  }) {
    toggleModal();
    // There was an error in this function...
    navigateToDashboard(dashboardUrl);
  },
  onError: () => {
    // Since there was an error in the function above, this code ran...
    showToast("Unable to create dashboard, please try again.", { variant: "error" });
  }
});

The Mutation was fine. There was a 200 from the GQL event. We found it interesting that since there was a JS error in onCompleted, the execution jumped over into onError, which made us initially feel like there was a GQL problem. Overall it just felt unintuitive.

I imagine there was probably a lot of thought that went into this behavior. Curious your thoughts!

[jerelmiller]

Hey @dep 👋

You're right, I wouldn't expect this myself! I don't think this is intentional behavior, more I think this is an unintended side effect of the way the code is currently written.

Looking at the code, I can see the onCompleted call is made in the .then callback to the promise returned by the mutate function:

apollo-client/src/react/hooks/useMutation.ts

Lines 179 to 181 in 143ae5a

	onCompleted?.(
	response.data!,
	clientOptions as MutationOptions<TData, OperationVariables>

We use .catch here to handle rejected promises from the mutate call, but I think that syntax error is getting caught up in that .catch callback since .catch will handle errors from any previous promise. From there, we call onError if its available:

apollo-client/src/react/hooks/useMutation.ts

Lines 205 to 209 in 143ae5a

	if (onError) {
	onError(
	error,
	clientOptions as MutationOptions<TData, OperationVariables>
	);

I think we could probably improve this by moving the error handling to the 2nd argument of the first .then so that it only handles errors created by the mutate promise. Let me take this to our next team meeting on Monday and double check that we would be ok with this approach.

Thanks for reporting!

[dep]

Appreciate that, and look forward to the follow-up @jerelmiller !