useIsMutating return wrong count of mutating calls

[FeliceGeracitano]

Describe the bug

useIsMutating return wrong count of pending mutation when a failure occur

Your minimal, reproducible example

https://codesandbox.io/s/great-zhukovsky-vovpcg?file=/src/useCustomMutations.js

Steps to reproduce

1 - Have custom useMutation function with callbacks

export const useCallBackMutation = () => {
  return useMutation(
    async () => {
      await new Promise((_, rej) => setTimeout(() => rej(true), 2000));
      return true;
    },
    {
      mutationKey: CALL_BACK_MUTATION_KEY,
      onError: async (error) => {
        return Promise.reject(error);
      },
      onSuccess: async (data) => {
        return data;
      }
    }
  );
};

2 - Listen for useIsMutation value

const value = useIsMutations()

3 - check number is incorrect when useMutation fails

1 - # instead of 0

Expected behavior

useIsMutations() should return 0, when there are no mutations pending

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

• OSX
• Chrome 98

react-query version

3.34

TypeScript version

No response

Additional context

No response

[FeliceGeracitano]

As you can see from the reproducible example https://codesandbox.io/s/great-zhukovsky-vovpcg?file=/src/useCustomMutations.js

the workaround is to use the try/catch approach

[TkDodo]

hmm, the callbacks don't need to reject or resolve the promise again? If the mutationFn rejects, the mutation goes to error state. Just remove the callbacks:

  return useMutation(async () => {
    await new Promise((_, rej) => setTimeout(() => rej(true), 2000));
    return true;
  });

adapted codesandbox

the returned promise from onError is awaited by react-query so that you can return queryClient.invalidateQueries('my-query') or so after a mutation.

From whatI can see, if the promises returned from the onError callback reject, we will never go into error state:

if onError or onSettled rejects, the dispatch({ type: error})` is not reached:

https://github.com/tannerlinsley/react-query/blob/00d0c527bc67cb0db7be9682565c60ffafc1ad51/src/core/mutation.ts#L258-L277

if the callback for onSuccess fails, we will go into the catch and thus in the error state.

funny thing: If the mutationFn succeeds, but then the onSettled callback rejects, we will call the onError callback and then again the onSettled callback.

[FeliceGeracitano]

@TkDodo thank u i see now,

i used the callbacks to perform side effects and I wanted the caller to get the error too,
but i see no need to reject onError because is already propagating
thanks for the response!

[FeliceGeracitano]

@TkDodo Although the count seems wrong to me anyway, and could lead to a never ending loading state
thanks again :)

[TkDodo]

@FeliceGeracitano I think I have a fix for v4, we can just go to error state if one of the callbacks errors.

One question is: if the mutationFn throws an error, and then the onSettled callback throws another, but different error, which one would we expect to show up in the error state? The original one or the one from onSettled 🤔

[tannerlinsley]

🎉 This issue has been resolved in version 4.0.0-alpha.17 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[FeliceGeracitano]

@TkDodo

That's a good question, i think depends on how onError, onSettled & onSuccess are meant to be used.

If they are just 'hooks' to events, the original error should be the one propagated, those call back should just run side-effect probably

[TkDodo]

yes, I also went with the initial error 👍

[tannerlinsley]

🎉 This issue has been resolved in version 4.0.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀