fix(react-query): allow retryOnMount when throwOnError is function (#9336) (#9338)

* test(react-query): add tests should retry on mount when throwOnError returns false

* fix(react-query): refine throwOnError retry logic for error boundary cases
When throwOnError is a function, allow retryOnMount to proceed even when error boundary hasn't reset,
while maintaining the prevention behavior for boolean throwOnError values.

* fix(react-query): enhance throwOnError logic in error boundaries

Refine behavior to handle function-type throwOnError, allowing retries when appropriate. Ensure boolean throwOnError values still prevent retries when the error boundary isn't reset.

* test: Add tests for `throwOnError` behavior in `useQuery`

This commit introduces tests to verify the behavior of the `throwOnError` callback in scenarios where `retryOnMount` is enabled. It ensures proper handling of retries based on the error state and the `throwOnError` function's return value. These tests improve the reliability and coverage of error handling logic in `useQuery`.

* fix(react-query): improve throwOnError logic with query state validation

- Pass query object to ensurePreventErrorBoundaryRetry for accurate state checking
- Preserve query deduplication behavior while fixing throwOnError function handling
- Fixes issue where throwOnError function couldn't access query error state

* fix(react-query): fix test flakiness and query cache timing (#9338)

- Replace vi.waitFor with vi.advanceTimersByTimeAsync in tests
- Use separate render results to avoid stale DOM references
- Inline fresh query lookup in ensurePreventErrorBoundaryRetry after _experimental_beforeQuery

* ci: apply automated fixes

* fix(react-query): refine retryOnMount logic and error boundary handling

- move query retrieval after `_experimental_beforeQuery` in `useBaseQuery`
- refactor `ensurePreventErrorBoundaryRetry` for better clarity
- make `query` parameter required in `ensurePreventErrorBoundaryRetry`

* ci: apply automated fixes

* feat(react-query): make query param required but nullable in ensurePreventErrorBoundaryRetry

* Apply suggestions from code review

* chore: size-limit

* chore: changeset

---------

Co-authored-by: Dominik Dorfmeister <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

Author: 3 people

.changeset/long-ties-rescue.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/react-query': patch
+---
+
+fix(react-query): allow retryOnMount when throwOnError is function

.size-limit.json

@@ -2,13 +2,13 @@
   {
     "name": "react full",
     "path": "packages/react-query/build/modern/index.js",
-    "limit": "12.10 kB",
+    "limit": "12.13 kB",
     "ignore": ["react", "react-dom"]
   },
   {
     "name": "react minimal",
     "path": "packages/react-query/build/modern/index.js",
-    "limit": "9.12 kB",
+    "limit": "9.15 kB",
     "import": "{ useQuery, QueryClient, QueryClientProvider }",
     "ignore": ["react", "react-dom"]
   }

packages/react-query/src/__tests__/useQuery.test.tsx

@@ -5921,6 +5921,7 @@ describe('useQuery', () => {
     it('should be able to toggle subscribed', async () => {
       const key = queryKey()
       const queryFn = vi.fn(() => Promise.resolve('data'))
+
       function Page() {
         const [subscribed, setSubscribed] = React.useState(true)
         const { data } = useQuery({
@@ -5965,6 +5966,7 @@ describe('useQuery', () => {
     it('should not be attached to the query when subscribed is false', async () => {
       const key = queryKey()
       const queryFn = vi.fn(() => Promise.resolve('data'))
+
       function Page() {
         const { data } = useQuery({
           queryKey: key,
@@ -5993,6 +5995,7 @@ describe('useQuery', () => {
     it('should not re-render when data is added to the cache when subscribed is false', async () => {
       const key = queryKey()
       let renders = 0
+
       function Page() {
         const { data } = useQuery({
           queryKey: key,
@@ -6192,6 +6195,7 @@ describe('useQuery', () => {
       await sleep(5)
       return { numbers: { current: { id } } }
     }
+
     function Test() {
       const [id, setId] = React.useState(1)
 
@@ -6257,6 +6261,7 @@ describe('useQuery', () => {
       await sleep(5)
       return { numbers: { current: { id } } }
     }
+
     function Test() {
       const [id, setId] = React.useState(1)
 
@@ -6762,10 +6767,12 @@ describe('useQuery', () => {
   it('should console.error when there is no queryFn', () => {
     const consoleErrorMock = vi.spyOn(console, 'error')
     const key = queryKey()
+
     function Example() {
       useQuery({ queryKey: key })
       return <></>
     }
+
     renderWithClient(queryClient, <Example />)
 
     expect(consoleErrorMock).toHaveBeenCalledTimes(1)
@@ -6775,4 +6782,172 @@ describe('useQuery', () => {
 
     consoleErrorMock.mockRestore()
   })
+
+  it('should retry on mount when throwOnError returns false', async () => {
+    const key = queryKey()
+    let fetchCount = 0
+    const queryFn = vi.fn().mockImplementation(() => {
+      fetchCount++
+      console.log(`Fetching... (attempt ${fetchCount})`)
+      return Promise.reject(new Error('Simulated 500 error'))
+    })
+
+    function Component() {
+      const { status, error } = useQuery({
+        queryKey: key,
+        queryFn,
+        throwOnError: () => false,
+        retryOnMount: true,
+        staleTime: Infinity,
+        retry: false,
+      })
+
+      return (
+        <div>
+          <div data-testid="status">{status}</div>
+          {error && <div data-testid="error">{error.message}</div>}
+        </div>
+      )
+    }
+
+    const rendered1 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent(
+      'Simulated 500 error',
+    )
+    expect(fetchCount).toBe(1)
+    rendered1.unmount()
+
+    const initialFetchCount = fetchCount
+
+    const rendered2 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
+
+    expect(fetchCount).toBe(initialFetchCount + 1)
+    expect(queryFn).toHaveBeenCalledTimes(2)
+  })
+
+  it('should not retry on mount when throwOnError function returns true', async () => {
+    const key = queryKey()
+    let fetchCount = 0
+    const queryFn = vi.fn().mockImplementation(() => {
+      fetchCount++
+      console.log(`Fetching... (attempt ${fetchCount})`)
+      return Promise.reject(new Error('Simulated 500 error'))
+    })
+
+    function Component() {
+      const { status, error } = useQuery({
+        queryKey: key,
+        queryFn,
+        throwOnError: () => true,
+        retryOnMount: true,
+        staleTime: Infinity,
+        retry: false,
+      })
+
+      return (
+        <div>
+          <div data-testid="status">{status}</div>
+          {error && <div data-testid="error">{error.message}</div>}
+        </div>
+      )
+    }
+
+    const rendered1 = renderWithClient(
+      queryClient,
+      <ErrorBoundary
+        fallbackRender={({ error }) => (
+          <div>
+            <div data-testid="status">error</div>
+            <div data-testid="error">{error?.message}</div>
+          </div>
+        )}
+      >
+        <Component />
+      </ErrorBoundary>,
+    )
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent(
+      'Simulated 500 error',
+    )
+    expect(fetchCount).toBe(1)
+
+    rendered1.unmount()
+
+    const initialFetchCount = fetchCount
+
+    const rendered2 = renderWithClient(
+      queryClient,
+      <ErrorBoundary
+        fallbackRender={({ error }) => (
+          <div>
+            <div data-testid="status">error</div>
+            <div data-testid="error">{error?.message}</div>
+          </div>
+        )}
+      >
+        <Component />
+      </ErrorBoundary>,
+    )
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
+
+    // Should not retry because throwOnError returns true
+    expect(fetchCount).toBe(initialFetchCount)
+    expect(queryFn).toHaveBeenCalledTimes(1)
+  })
+
+  it('should handle throwOnError function based on actual error state', async () => {
+    const key = queryKey()
+    let fetchCount = 0
+    const queryFn = vi.fn().mockImplementation(() => {
+      fetchCount++
+      console.log(`Fetching... (attempt ${fetchCount})`)
+      return Promise.reject(new Error('Simulated 500 error'))
+    })
+
+    function Component() {
+      const { status, error } = useQuery({
+        queryKey: key,
+        queryFn,
+        throwOnError: (error) => error.message.includes('404'),
+        retryOnMount: true,
+        staleTime: Infinity,
+        retry: false,
+      })
+
+      return (
+        <div>
+          <div data-testid="status">{status}</div>
+          {error && <div data-testid="error">{error.message}</div>}
+        </div>
+      )
+    }
+
+    const rendered1 = renderWithClient(queryClient, <Component />)
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent(
+      'Simulated 500 error',
+    )
+    expect(fetchCount).toBe(1)
+
+    rendered1.unmount()
+
+    const initialFetchCount = fetchCount
+
+    const rendered2 = renderWithClient(queryClient, <Component />)
+
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
+
+    // Should retry because throwOnError returns false (500 error doesn't include '404')
+    expect(fetchCount).toBe(initialFetchCount + 1)
+    expect(queryFn).toHaveBeenCalledTimes(2)
+  })
 })

packages/react-query/src/errorBoundaryUtils.ts

@@ -25,11 +25,17 @@ export const ensurePreventErrorBoundaryRetry = <
     TQueryKey
   >,
   errorResetBoundary: QueryErrorResetBoundaryValue,
+  query: Query<TQueryFnData, TError, TQueryData, TQueryKey> | undefined,
 ) => {
+  const throwOnError =
+    query?.state.error && typeof options.throwOnError === 'function'
+      ? shouldThrowError(options.throwOnError, [query.state.error, query])
+      : options.throwOnError
+
   if (
     options.suspense ||
-    options.throwOnError ||
-    options.experimental_prefetchInRender
+    options.experimental_prefetchInRender ||
+    throwOnError
   ) {
     // Prevent retrying failed query if the error boundary has not been reset yet
     if (!errorResetBoundary.isReset()) {

packages/react-query/src/useBaseQuery.ts

@@ -53,11 +53,19 @@ export function useBaseQuery<
   const errorResetBoundary = useQueryErrorResetBoundary()
   const client = useQueryClient(queryClient)
   const defaultedOptions = client.defaultQueryOptions(options)
-
   ;(client.getDefaultOptions().queries as any)?._experimental_beforeQuery?.(
     defaultedOptions,
   )
 
+  const query = client
+    .getQueryCache()
+    .get<
+      TQueryFnData,
+      TError,
+      TQueryData,
+      TQueryKey
+    >(defaultedOptions.queryHash)
+
   if (process.env.NODE_ENV !== 'production') {
     if (!defaultedOptions.queryFn) {
       console.error(
@@ -72,8 +80,7 @@ export function useBaseQuery<
     : 'optimistic'
 
   ensureSuspenseTimers(defaultedOptions)
-  ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary)
-
+  ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query)
   useClearResetErrorBoundary(errorResetBoundary)
 
   // this needs to be invoked before creating the Observer because that can create a cache entry
@@ -127,14 +134,7 @@ export function useBaseQuery<
       result,
       errorResetBoundary,
       throwOnError: defaultedOptions.throwOnError,
-      query: client
-        .getQueryCache()
-        .get<
-          TQueryFnData,
-          TError,
-          TQueryData,
-          TQueryKey
-        >(defaultedOptions.queryHash),
+      query,
       suspense: defaultedOptions.suspense,
     })
   ) {
@@ -155,7 +155,7 @@ export function useBaseQuery<
       ? // Fetch immediately on render in order to ensure `.promise` is resolved even if the component is unmounted
         fetchOptimistic(defaultedOptions, observer, errorResetBoundary)
       : // subscribe to the "cache promise" so that we can finalize the currentThenable once data comes in
-        client.getQueryCache().get(defaultedOptions.queryHash)?.promise
+        query?.promise
 
     promise?.catch(noop).finally(() => {
       // `.updateResult()` will trigger `.#currentThenable` to finalize

packages/react-query/src/useQueries.ts

@@ -242,9 +242,10 @@ export function useQueries<
     [queries, client, isRestoring],
   )
 
-  defaultedQueries.forEach((query) => {
-    ensureSuspenseTimers(query)
-    ensurePreventErrorBoundaryRetry(query, errorResetBoundary)
+  defaultedQueries.forEach((queryOptions) => {
+    ensureSuspenseTimers(queryOptions)
+    const query = client.getQueryCache().get(queryOptions.queryHash)
+    ensurePreventErrorBoundaryRetry(queryOptions, errorResetBoundary, query)
   })
 
   useClearResetErrorBoundary(errorResetBoundary)