`invalidateQueries` inconsistently cancels `fetchQuery`
Describe the bug
fetchQuery may throw an CancelledError { silent: true, revalidate: undefined } under the following conditions:
- the query is stale (otherwise the fetch won't run).
- the query is invalidated at least one tick after the fetch started.
-
the query is in an
activestate. For example because a component withuseQueryis still mounted.
In the real world, this might happen in combination with react-router, when the user submits a form and then navigates to a new (or the same) page. In my case, we were updating a search param to close a modal after a form submission, which then triggered the loader function containing a fetchQuery call. We're also using a mutation observer[^1] to trigger invalidations, which is why the invalidation happened only after the navigation began.
I fixed the issue by using ensureQueryData instead of fetchQuery. This is the better choice anyways.
Still, this seems like a footgun since it is difficult to catch while testing. Further, if the intended behavior for fetchQuery is to throw a CancelledError on invalidation, it should always do that and not only when a revalidation is triggered.
[^1]: inspired by https://tkdodo.eu/blog/automatic-query-invalidation-after-mutations
Your minimal, reproducible example
https://codesandbox.io/p/sandbox/react-query-cancellederror-on-invalidation-d4g259
Steps to reproduce
See steps above
Expected behavior
Option A: The returned promise should consistently reject, regardless of whether an unrelated observer exists. Option B: The returned promise should consistently resolve with the data returned from this fetch Option C: The QueryClient should treat pending promises as observers, revalidate automatically and always resolve with the result from the latest revalidation.
How often does this bug happen?
Every time
Screenshots or Videos
No response
Platform
Tanstack Query adapter
react-query
TanStack Query version
5.50.1
TypeScript version
No response
Additional context
No response
the sandbox seems to be private
the sandbox seems to be private
Apologies. Should be public now.
The issue seems to be specific to fetchQuery because it's the only imperative API that throws per default (on purpose). I guess we could also reproduce with queryClient.refetchQueries when passing throwOnError: true. Or by simply calling fetchQuery followed by queryClient.cancelQueries()
The Cancellation itself is on purpose, as calling invalidateQueries() will cancel any ongoing fetch. If that comes from fetchQuery, that function will throw.
You could also work around it by passing cancelRefetch: false to invalidateQueries. In that case, it would "pick up" the already running promise instead of cancelling + re-starting.
Option A: The returned promise should consistently reject, regardless of whether an unrelated observer exists.
I'd have to dig deeper to understand why the cancel error is only thrown if there is an active observer 🤔 . I generally agree that it should consistently reject.
Option B: The returned promise should consistently resolve with the data returned from this fetch
The problem is that we can't just catch the error in fetchQuery because what should happen then? We might not have data we can resolve to, so I think this isn't possible.
Option C: The QueryClient should treat pending promises as observers, revalidate automatically and always resolve with the result from the latest revalidation.
That's too big a change, as the only way to get an observer is by creating a QueryObserver, which happens in useQuery.
so I guess we'll have to look into option A :)
Thanks for your time.
I'd have to dig deeper to understand why the cancel error is only thrown if there is an active observer 🤔
From my understanding, invalidation doesn't cancel fetches by itself, but it might trigger a revalidation of active queries. The revalidation always cancels the previous fetchQuery call.
https://github.com/TanStack/query/blob/763abd1628c09a7c7513e55cb30e64041ea1ac8d/packages/query-core/src/queryClient.ts#L292C6-L296C6
So the question is: is there any reason why invalidation should not always cancel any ongoing fetch. If there isn't, perhaps we could add Query.cancel({ silent: true }) here:
invalidateQueries(
filters: InvalidateQueryFilters = {},
options: InvalidateOptions = {},
): Promise<void> {
return notifyManager.batch(() => {
this.#queryCache.findAll(filters).forEach((query) => {
query.invalidate()
+ query.cancel({ silent: true })
})
if (filters.refetchType === 'none') {
return Promise.resolve()
}
A few more thoughts:
- if invalidation cancels all queries,
ensureQueryDatawill also be affected. The reason why it's safe to call currently, is that it only triggers a refetch if the Query is new, therefore bypassing that stale & active case offetchQuery. - ~~Another feature that would be impacted is
useSuspenseQuery, since it throws the promise returned byQuery.fetch(). Currently, if a suspending query invalidates, the fetch continues and the component renders the outdated data. If we implement Option A, suspending components might throw unexpectedly.~~ (EDIT: no, it doesn't care whether the promise succeeds or fails)
Could we perhaps implement Option C by catching a CanceledError in fetchQuery and replacing it with another query.fetch(defaultedOptions, { cancelRefetch: false })?
Naive approach:
fetchQuery<
TQueryFnData,
TError = DefaultError,
TData = TQueryFnData,
TQueryKey extends QueryKey = QueryKey,
TPageParam = never,
>(
options: FetchQueryOptions<
TQueryFnData,
TError,
TData,
TQueryKey,
TPageParam
>,
): Promise<TData> {
const defaultedOptions = this.defaultQueryOptions(options)
// https://github.com/tannerlinsley/react-query/issues/652
if (defaultedOptions.retry === undefined) {
defaultedOptions.retry = false
}
const query = this.#queryCache.build(this, defaultedOptions)
return query.isStaleByTime(
resolveStaleTime(defaultedOptions.staleTime, query),
)
- ? query.fetch(defaultedOptions)
+ ? query.fetch(defaultedOptions).catch((e) => {
+ if (isCancelledError(e)) return query.fetch(defaultedOptions, { cancelRefetch: false })
+ else throw e
+ })
: Promise.resolve(query.state.data as TData)
}
The above solution doesn't cover multiple scenarios:
-
useSuspenseQuerygoes directly toquery.fetch()and therefore would still need to be handled somehow. - What if there are multiple invalidations? Maybe this would need to be done recursively. ]
But in principle something like that might work without a major change.
I've been playing around with this issue a bit, as part of which I redid the react router example with tanstack router:
https://stackblitz.com/edit/tanstack-router-invalidated-cancel-error
A few differences from @SebKranz rr example:
- Clicking "Invalidate then Navigate" three times quickly triggers the
CancelledError - Clicking "Navigate then Invalidate" does not immediately break the app - seems like the query runs twice so it's in
fetchingfor twice as long
- [ ] TODO fix styling