useQuery + invalidateQueries intermittently stale UI

[jgarplind]

This is more of an issue, but not consistently reproducible, so I'm hoping to get some insights by posting here!

UPDATE: Same error happens intermittently in another context where no initialData is used.

Main issue

Stale UI, not updating as per latest data after cache invalidation.

Key information

• Rarely, but repeatedly (we get user reports and see error logs stemming from this every few days, and these particular flows are only used by 10s of users), the UI is left in a stale state (reflecting the pre-mutation markup), despite the onSuccess hook declaring that the loading state should not be let go until the invalidations are completed (as per https://tkdodo.eu/blog/mastering-mutations-in-react-query#awaited-promises).
• Underlying is a CQRS pattern, and we have cross-examined the backend logic - there is no way that the GET stemming from the cache invalidation could retrieve pre-mutation data - the mutation does not return until the data stores reflect the latest state.

(New simpler) Code example

We have a few queries set up using this structure (omitting most markup for brevity):

Route.tsx

const { data: route } = useQuery({
  queryKey: ["routes", "detail", routeId],
  queryFn: () => {
    return fetchRoute(routeId);
  }
});

const { mutate: finishRouteMutation } = useMutation({
  mutationFn: ({ routeId }: { routeId: string }) => {
    return finishRoute(routeId);
  },
  onSuccess: () => {
    return queryClient.invalidateQueries({ queryKey: ["routes"] });
  },
});

return (
  {
    route.status === "finished" ? 
      <span>"All good!"</span> : 
      <button onClick={() => { finishRouteMutation({ routeId }); }}>Finish</button>
  }

Questions

Any ideas?

Any fundamental mistakes in this setup?

Really grateful for any pointers here! Thanks 🙏

[ynifamily3]

How about including initialRoute.id in queryKey?

[jgarplind]

Apologies, I redacted that by mistake. It is there already, editing the main post now.

[jgarplind]

Updated again, considering a simpler example (with no initialData) which causes the same issue.

[TkDodo]

the code looks fine. Usually when this happens, the backend doesn't consistently await the database-write (or whatever it is that the backend is doing) before finishing the mutation.

suppose the backend endpoint looks something like this:

finishRoute: async(input) => {
  await db.updateRoute(input)
  return { status: 'ok' }
}

if you forget the await before the db operation, the mutation will come back successfully rather fast, but the update hasn't really happened yet. Then, the invalidation runs into a race condition with the db write, which means it might return old data.

Again, the mutation code looks absolutely fine. You can double-check the network tab to see what the GET request returns. If you see "old" data over the wire, this could be it.

Similarly, if the invalidation gets a response from the browser cache (not sure if you're using browser caching), the backend would never be hit for the invalidation request.

For both scenarios, you will find the answer in the network tab.

[jgarplind]

Thanks for confirming my suspicions.

We do believe that the backend code is rock solid in this regard, but we will have to revisit that.

I'll try to feed back any insights in this discussion thread.

[jgarplind]

@TkDodo I think I finally have a step-by-step recording of one scenario which is causing this behavior for us. The issue is most likely at the client side, but I don't know if I should "blame" the browser, this library, or something else yet. Maybe this added information will let you have enough context to help "point fingers"?

It seems to reproduce like this:

1. A request is made through useQuery. Probably due to stale cache and revalidate on window focus (mobile user unlocking the screen or similar)
2. Around 1 second later, just as the initial request is about to be finished, another user interaction causes the query key from no. 1 to be invalidated again, causing another request to be fired.
3. On the same millisecond (if we can even trust our tools with this level of precision), the response from no. 1 is returned, and it appears as if the response somehow satisfies the 2nd request with the same response payload, despite there being 2 discrete requests (according to Sentry anyway).
   
   These are requests to the same URL, but only one request is recorded in our backend logs
   At this point, I'm not sure if Sentry, iOS Safari, some React Query de-deplucation logic or my fundamental understanding of browsers and HTTP is to blame, but I don't expect the second request to be satisfied by the first response under any circumstance short of some service worker shenanigans that we are not using here.
4. Summing things up, it appears as if the response from no. 2 is essentially a duplicate of the response from no. 1, leading to a stale state until the next interaction.

So... are there any insights to be had here on the React Query side of things?

Looking into some similar topics, I found this: #608 (reply in thread). If I read into things correctly, it essentially says that a mutation with onSuccess: () => invalidateQueries({ queryKey: ["x"] }) can have issues with race conditions if other (slowish) requests to the same query key are in progress by the time the mutationFn is finished. Such race conditions are of course only an issue if the initial request made its server-side evaluation before the mutation was written to the data store, which should be unlikely, but possible, given the multi-threaded, non-sequential nature of most API servers.

... all things considered, I think I may have a pretty good grip of what is going on now, and I think it is reasonable to "blame" the "promise clumping" of React Query, but as a last reach out - do you happen to have any strategies short of "do less things"/"lock down the UI while state changes are in progress" to share?

[jgarplind]

Just had another look at the docs, and I think we should prefer refetch (or refetchQueries) over invalidateQueries in these scenarios where many requests to the same key could occur, and where we want to prioritize the latest request?

[TkDodo]

I don't think so. invaliadateQueries, per default since v4, fires with cancelRefetch: true, so if another request for the key is in flight, it will be cancelled and a new one should be fired. invalidateQueries also calls refetchQueries under the hood to do that, so switching the two shouldn't yield any differences.

[jgarplind]

Ah, too bad, I was hoping to have found an oversight that could explain our issues.

I will add more log points, and try moving from invalidateQueries to refetchQueries (despite having checked the source code and understood that invalidate is just a small wrapper under these circumstances), to catch it more frequently and hopefully be able to loop back with more insights.