invalidateQueries inconsistently cancels fetchQuery #8060

Activity

1. TkDodo commented on Sep 15, 2024

   

   TkDodo

   on Sep 15, 2024

   Collaborator

   More actions

   the sandbox seems to be private
2. SebKranz commented on Sep 15, 2024

   

   SebKranz

   on Sep 15, 2024

   ContributorAuthor

   More actions

   the sandbox seems to be private

   Apologies. Should be public now.
3. TkDodo commented on Sep 15, 2024

   

   TkDodo

   on Sep 15, 2024

   Collaborator

   More actions

   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 :)
4. SebKranz commented on Sep 15, 2024

   

   SebKranz

   on Sep 15, 2024

   ContributorAuthor

   More actions

   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()
         }
5. SebKranz commented on Sep 15, 2024

   

   SebKranz

   on Sep 15, 2024

   Last edited by SebKranz

   ContributorAuthor

   More actions

   A few more thoughts:

   • if invalidation cancels all queries, ensureQueryData will 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 of fetchQuery.
   • Another feature that would be impacted is useSuspenseQuery, since it throws the promise returned by Query.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:

   • useSuspenseQuery goes directly to query.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.
6. DogPawHat commented on Sep 26, 2024

   

   DogPawHat

   on Sep 26, 2024

   Collaborator

   More actions

   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:

   1. Clicking "Invalidate then Navigate" three times quickly triggers the CancelledError
   2. Clicking "Navigate then Invalidate" does not immediately break the app - seems like the query runs twice so it's in fetching for twice as long

   • TODO fix styling
7. birkskyum

   added

   package: react-query

   on Mar 31, 2025