Respecting staletime for errors

[vezaynk]

I posted it on SO but it got auto-deleted before I could add the codesandbox.

Codesandbox: https://codesandbox.io/s/react-query-failed-prefetch-mi96w1?file=/src/App.tsx

Despite staletime being set to 10_000, error queries are retried on every call. This is bad because it overwhelms my API with network calls when there are errors.

Original SO post

It seems that whenever a query throws an error, it's staleTime is forcibly set to 0 and made to be refetched in the next usage. Is it possible to make staleTime for failed queries consistent with succeeded queries?

My issue is that I prefetch a list of queries in a component

for (...) {
   queryClient.prefetch(..., ...); // prefetches every query which needs to be put into cache every time it goes stale
}

and failed queries get "re-prefetched" on every render because all failed queries seem to be immediately marked stale. I thought about a checking to ignore failed queries:

for (...) {
   if (!getQueryState(...)?.error) // don't prefetch previously failed queries
       queryClient.prefetch(..., ...);
}

However this causes a new issue where I can't make the failed queries re-fetch when their error might have gone away (after the staletime).

I guess I could do some math comparing the time of the error with the current time, and deciding whether pre-fetch based on that, but while it would solve the prefetch issue, the same sort of problem happens with components that call useQuery instead which would require me to do funny things with enabled. In short, it would force me to take up a lot of undue complexity to make failed queries behave consistently with succeeding queries.

[vezaynk]

In the meantime, I applied the following diff to my local versions to make it behave the way I need. I can hide this behavior behind a flag and make a proper PR if @TkDodo would be interesting in it!

diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts
index 22dadd37..08dda01f 100644
--- a/packages/query-core/src/query.ts
+++ b/packages/query-core/src/query.ts
@@ -254,8 +254,11 @@ export class Query<
   isStaleByTime(staleTime = 0): boolean {
     return (
       this.state.isInvalidated ||
-      !this.state.dataUpdatedAt ||
-      !timeUntilStale(this.state.dataUpdatedAt, staleTime)
+      (!this.state.dataUpdatedAt && !this.state.errorUpdatedAt) ||
+      !timeUntilStale(
+        Math.max(this.state.errorUpdatedAt ?? 0, this.state.dataUpdatedAt ?? 0),
+        staleTime,
:...skipping...
diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts
index 22dadd37..08dda01f 100644
--- a/packages/query-core/src/query.ts
+++ b/packages/query-core/src/query.ts
@@ -254,8 +254,11 @@ export class Query<
   isStaleByTime(staleTime = 0): boolean {
     return (
       this.state.isInvalidated ||
-      !this.state.dataUpdatedAt ||
-      !timeUntilStale(this.state.dataUpdatedAt, staleTime)
+      (!this.state.dataUpdatedAt && !this.state.errorUpdatedAt) ||
+      !timeUntilStale(
+        Math.max(this.state.errorUpdatedAt ?? 0, this.state.dataUpdatedAt ?? 0),
+        staleTime,
+      )
     )
   }
 
@@ -559,6 +562,7 @@ export class Query<
             fetchFailureReason: error as TError,
             fetchStatus: 'idle',
             status: 'error',
+            isInvalidated: false,
           }
         case 'invalidate':
           return {
diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
index d23850bc..376d1ed2 100644
--- a/packages/query-core/src/queryObserver.ts
+++ b/packages/query-core/src/queryObserver.ts
@@ -714,6 +714,7 @@ function shouldLoadOnMount(
   return (
     options.enabled !== false &&
     !query.state.dataUpdatedAt &&
+    !query.state.errorUpdatedAt &&
     !(query.state.status === 'error' && options.retryOnMount === false)
   )
 }
@@ -753,7 +754,7 @@ function shouldFetchOptionally(
   return (
     options.enabled !== false &&
     (query !== prevQuery || prevOptions.enabled === false) &&
-    (!options.suspense || query.state.status !== 'error') &&
+    !options.suspense &&
     isStale(query, options)
   )
 }

[TkDodo]

Despite staletime being set to 10_000, error queries are retried on every call. This is bad because it overwhelms my API with network calls when there are errors.

staleTime defines how long cached data should be treated as fresh. In an error case, you have no data, so those are treated as immediately stale. There are two ways around it:

1. use errorBoundaries. This will unmount your query so it won't refetch on the next render, because it is not "there" until the boundary gets reset
2. set retryOnMount: false. from the docs:

retryOnMount: boolean
If set to false, the query will not be retried on mount if it contains an error. Defaults to true.

[vezaynk]

retryOnMount only does it for the hooks API. There is no equivalent retryOnMount check when using the imperative API (fetch, prefetch).

Could a config option be added to capture these as well? Or generalize retryOnMount to handle all cases, not just hooks.

[TkDodo]

There is no equivalent retryOnMount check when using the imperative API (fetch, prefetch).

but why would you imperatively call something if it's in a state where you don't want to call it 🙃 . Honestly, a reproduction would be good because right now I'm just guessing. You're saying:

error queries are retried on every call

which is quite ambiguous ...

Or generalize retryOnMount to handle all cases, not just hooks.

there is no mount event if you call queryClient.fetchQuery(...)

[vezaynk]

The expectation I have from the queryClient is that if my staletime is set to 10 seconds, and if I don't invalidate it, every fetch call that comes through it will be read from the cache for the duration of those 10 seconds.

This guarantee holds true for fetch calls which returned data successfully, but not those which failed.

With that in mind, retryOnMount lets us toggle the behavior for hooks. (why is retryOnMount implemented on the react-query level and not the query-core level?)

The use-case I have is a bit tricky to make a codesandbox of while staying concise.

Basically I have one API which returns a list of IDs (List), and one to get their details (Describe).

I have a paginated table to display 5 of the records at a time.

The table calls the List query, and each row calls Describe.

The table has a built-in search which works based on the data provided to each table row. To make it work, I need to have all of the Describe data in the table component in order to populate the search cache.

The way I do it is by prefetching details of every ID, and then providing them by reading from the QueryClient cache.

This works well in the happy path case where every prefetch call is successful, but if it fails, it gets prefetched again on the next render ignoring the staleTime.

I could feasibly implement it myself in my client code by comparing the config StaleTime and ErrorUpdatedAt times but it is burdensome.

[TkDodo]

This guarantee holds true for fetch calls which returned data successfully, but not those which failed.

Because the assumption about what staleTime does is wrong. It only sets the time for data, not error. That is per design. From that perspective, your data is still undefined so there is nothing to deliver from the cache.

why is retryOnMount implemented on the react-query level and not the query-core level?

retryOnMount is just another option of the queryObserver, so it is implemented in the query-core:

query/packages/query-core/src/queryObserver.ts

Lines 681 to 690 in 35055d7

	function shouldLoadOnMount(
	query: Query<any, any, any, any>,
	options: QueryObserverOptions<any, any, any, any>,
	): boolean {
	return (
	options.enabled !== false &&
	!query.state.dataUpdatedAt &&
	!(query.state.status === 'error' && options.retryOnMount === false)
	)
	}

but if it fails, it gets prefetched again on the next render ignoring the staleTime.

can you show some code for this? Are you calling queryClient.prefetchQuery in your render function? I'm having a hard time imagining this ...

[vezaynk]

I'll make a codesandbox later today.

I know it works per design. I'm just hoping we can work out a way to enable error caching within the design as well. Having an errorStaleTime would be really nice for my use-case.