useMutation - cancelQueries triggers active useQuery null error

[babycourageous]

Describe the bug
It seems that cancelQueries inside a useMutation somehow triggers a null error in the associated useQuery IF that query is actively fetching (i.e. after being updated). So if you run a mutation and the run it again before the query fully resolves.

The sandboxes below demonstrate that
https://codesandbox.io/s/rq-optupd-quick-forked-6zlmp is a stripped down mutation that only cancels the associated query.
https://codesandbox.io/s/rq-optupd-quick-rbt3k is a full useMutation implementation forked from the docs.

In either case - commenting out cancelQueries seems to prevent the buggy behavior.

To Reproduce
Go to either sandbox
Add a todo and WHILE the todo query is updating in background (you know this by the display text) add another todo. You will see in the console that an error from the todo query fires.

The error is null and doesn't seem to impact the mutation but if you have error toasts or any other query error handling that would all run obviously.

Expected behavior
if a mutation fires while a query is updating in the background the cancelQueries should not force a null error on that query.

Desktop (please complete the following information):

• OS: Mac OS 10.14
• Browser: chrome
• Version: 88

Additional context
This behavior happens in the CodeSandbox environment above but I have experienced it outside of a sandbox as well in React Query v 3.5.16 as well as after updating to the current version of React Query. The one strange thing is that commenting out either cancelQueries or invalidateQueries in my app seems to prevent the behavior - however commenting out invalidateQueries in the sandbox had no effect.

Current Workaround
To suppress any useQuery onError code from running I am currently using the following workaround:

{
  onError: (error) => {
    if (!error) return // exit early if there is no error (i.e. the error is null as is the case in the cancelQueries behavior)
    /* ALL ERROR HANDLING HERE *.
  }
}

[mhrrmk]

It seems cancelQueries has a CancelOptions parameter as the third argument where it has a silent property. Setting it to true seems to prevent triggering the onError of the associated query.

await queryClient.cancelQueries("todos", undefined, { silent: true });

[TkDodo]

Should this maybe default to true then …?

[mhrrmk]

I don't know the exact behaviour of the silent (I was unable to find the documentation for it btw I'd appreciate if you can point to it) but if it doesn't have any other important effect I'd support it being true by default. We're currently working in a project where we're doing optimstic and undoable updates and if it wasn't for this issue it'd take considerably more time to find the culprit. It's a bit strange when none of the actual requests returns an error but onErroris triggered and with a null error object :)

[TkDodo]

@tannerlinsley I tried to debug this with no success - I can't pinpoint where this null error is coming from. Do you have any ideas maybe?

[tannerlinsley]

If we're not using silent in any important/relevant way internally, I would be fine making that the default.

[TkDodo]

@tannerlinsley I gave it another attempt and I think I got the root cause now:

• the retryer cancels the query as expected
• it creates a CancelledError with revert:true and silent: undefined

https://github.com/tannerlinsley/react-query/blob/fbd3488a3c13eeef3d9eb4d5cd536787ac75ecf3/src/core/retryer.ts#L142-L144

I think so far so good because there is actually a fetch going on that we want to cancel.

• because we are not silent, we dispatch an error event:

https://github.com/tannerlinsley/react-query/blob/5e76781a33eec6e28ad4391d393171b7dc7a6345/src/core/query.ts#L439-L444

Also good I think

• Then, in the reducer, we revert back to the previous state, because revert: true (also fine):

https://github.com/tannerlinsley/react-query/blob/5e76781a33eec6e28ad4391d393171b7dc7a6345/src/core/query.ts#L568-L570

Of course, after the revert, in our state machine, error is null, because we are status: success and we didn't have an error before.

• The dispatch is then what causes the observers to be notified, which is important:

https://github.com/tannerlinsley/react-query/blob/5e76781a33eec6e28ad4391d393171b7dc7a6345/src/core/query.ts#L480-L483

But this also causes the onError callbacks of the observers to be fired, even though we now have no error in our state anymore:

https://github.com/tannerlinsley/react-query/blob/3b2c4088e103e4b4ca990662c03fd6e67330b477/src/core/queryObserver.ts#L663-L665

⬆️ so maybe here is where we'd need to additionally check if we are in error state, or if error !== null or so?

Note that we have a check for our onError callback that we call once on the queryCache:

https://github.com/tannerlinsley/react-query/blob/5e76781a33eec6e28ad4391d393171b7dc7a6345/src/core/query.ts#L446-L448

but there it's simpler to check because we have access to the error object, which we don't after revert has reset our error.

---

Two things we could do:

• make a null check as proposed above. It would fix the immediate case I think, but it wouldn't be 100% correct. Suppose we are in error state already (because fetch failed the first time), and are currently fetching, which we are then cancelling (due to calling queryClient.cancelQueries). The revert would then set us back to a state where we also have an error, and we would fire that to our observers :/
• Have CancelledErrors always be silent and remove the silent boolean. Basically, this is a more radical version of defaulting silent to true. As far as I can see, this is undocumented and we set it to true a bunch of times internally anyhow. The question would be: Is there ever a case where we'd want a CancelledError to be not silent? The way I see it, this error is raised so that we can internally revert our state because the current fetch has been cancelled - why would we want to inform observers about that? Maybe I'm missing something?

[TkDodo]

I think I misunderstood what silent means: it doesn’t mean: don’t inform the observers, it means: don’t dispatch the action internally at all. But we need that to revert properly.

i think we never want to inform observers about CancelledErrors, and I think I know how to achieve that know. Will attempt tomorrow.

[babycourageous]

Thanks @TkDodo , @tannerlinsley , @mhrrmk for battling this one!

[tannerlinsley]

🎉 This issue has been resolved in version 3.17.3 🎉

The release is available on:

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

Your semantic-release bot 📦🚀