From 878ac3489477138d95bb09feb5d2ed62a22f89d1 Mon Sep 17 00:00:00 2001 From: sangwook Date: Mon, 30 Jun 2025 23:13:14 +0900 Subject: [PATCH 1/6] test(react-query): add tests should retry on mount when throwOnError returns false --- .../src/__tests__/useQuery.test.tsx | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 7eea42110c..6b92e7a4ea 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -5920,6 +5920,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({ @@ -5964,6 +5965,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, @@ -5992,6 +5994,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, @@ -6191,6 +6194,7 @@ describe('useQuery', () => { await sleep(5) return { numbers: { current: { id } } } } + function Test() { const [id, setId] = React.useState(1) @@ -6256,6 +6260,7 @@ describe('useQuery', () => { await sleep(5) return { numbers: { current: { id } } } } + function Test() { const [id, setId] = React.useState(1) @@ -6761,10 +6766,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, ) expect(consoleErrorMock).toHaveBeenCalledTimes(1) @@ -6774,4 +6781,56 @@ 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 ( +
+
{status}
+ {error &&
{error.message}
} +
+ ) + } + + const { unmount, getByTestId } = renderWithClient( + queryClient, + , + ) + + await vi.waitFor(() => + expect(getByTestId('status')).toHaveTextContent('error'), + ) + expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') + expect(fetchCount).toBe(1) + + unmount() + + const initialFetchCount = fetchCount + + renderWithClient(queryClient, ) + + await vi.waitFor(() => + expect(getByTestId('status')).toHaveTextContent('error'), + ) + + expect(fetchCount).toBe(initialFetchCount + 1) + expect(queryFn).toHaveBeenCalledTimes(2) + }) }) From 3225745c6b79135d1cf08fee122ed2e83f0e4d4a Mon Sep 17 00:00:00 2001 From: sangwook Date: Mon, 30 Jun 2025 23:52:02 +0900 Subject: [PATCH 2/6] 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. --- packages/react-query/src/errorBoundaryUtils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react-query/src/errorBoundaryUtils.ts b/packages/react-query/src/errorBoundaryUtils.ts index 28c11c0b10..52f511fb1b 100644 --- a/packages/react-query/src/errorBoundaryUtils.ts +++ b/packages/react-query/src/errorBoundaryUtils.ts @@ -28,13 +28,16 @@ export const ensurePreventErrorBoundaryRetry = < ) => { if ( options.suspense || - options.throwOnError || options.experimental_prefetchInRender ) { // Prevent retrying failed query if the error boundary has not been reset yet if (!errorResetBoundary.isReset()) { options.retryOnMount = false } + } else if (options.throwOnError) { + if (!errorResetBoundary.isReset() && typeof options.throwOnError !== 'function') { + options.retryOnMount = false + } } } From 4c4079021e1a15bf357fef7deb1d73de8c9fa1cd Mon Sep 17 00:00:00 2001 From: sangwook Date: Tue, 1 Jul 2025 00:19:42 +0900 Subject: [PATCH 3/6] 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. --- packages/react-query/src/errorBoundaryUtils.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/react-query/src/errorBoundaryUtils.ts b/packages/react-query/src/errorBoundaryUtils.ts index 52f511fb1b..7e7b44afcd 100644 --- a/packages/react-query/src/errorBoundaryUtils.ts +++ b/packages/react-query/src/errorBoundaryUtils.ts @@ -25,6 +25,7 @@ export const ensurePreventErrorBoundaryRetry = < TQueryKey >, errorResetBoundary: QueryErrorResetBoundaryValue, + query?: Query, ) => { if ( options.suspense || @@ -34,8 +35,12 @@ export const ensurePreventErrorBoundaryRetry = < if (!errorResetBoundary.isReset()) { options.retryOnMount = false } - } else if (options.throwOnError) { - if (!errorResetBoundary.isReset() && typeof options.throwOnError !== 'function') { + } else if (options.throwOnError && !errorResetBoundary.isReset()) { + if (typeof options.throwOnError === 'function') { + if (query?.state.error && shouldThrowError(options.throwOnError, [query.state.error, query])) { + options.retryOnMount = false + } + } else { options.retryOnMount = false } } From 182b2392e0089f9aab1c344606e8938d445f3f82 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 6 Jul 2025 09:59:21 +0900 Subject: [PATCH 4/6] 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`. --- .../src/__tests__/useQuery.test.tsx | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 6b92e7a4ea..ad76e48c80 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -6833,4 +6833,130 @@ describe('useQuery', () => { 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 ( +
+
{status}
+ {error &&
{error.message}
} +
+ ) + } + + const { unmount, getByTestId } = renderWithClient( + queryClient, + ( +
+
error
+
{error?.message}
+
+ )} + > + + , + ) + + await vi.waitFor(() => + expect(getByTestId('status')).toHaveTextContent('error'), + ) + expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') + expect(fetchCount).toBe(1) + + unmount() + + const initialFetchCount = fetchCount + + renderWithClient(queryClient, + ( +
+
error
+
{error?.message}
+
+ )} + > + + + ) + + await vi.waitFor(() => + expect(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 ( +
+
{status}
+ {error &&
{error.message}
} +
+ ) + } + + const { unmount, getByTestId } = renderWithClient( + queryClient, + , + ) + + await vi.waitFor(() => + expect(getByTestId('status')).toHaveTextContent('error'), + ) + expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') + expect(fetchCount).toBe(1) + + unmount() + + const initialFetchCount = fetchCount + + renderWithClient(queryClient, ) + + await vi.waitFor(() => + expect(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) + }) }) From e8e0caaa5b58e130b3fe0c98cc550f762278d6ee Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 6 Jul 2025 10:28:30 +0900 Subject: [PATCH 5/6] 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 --- packages/react-query/src/__tests__/useQuery.test.tsx | 5 +++-- packages/react-query/src/errorBoundaryUtils.ts | 10 +++++----- packages/react-query/src/useBaseQuery.ts | 11 +++++++++-- packages/react-query/src/useQueries.ts | 7 ++++--- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index ad76e48c80..c796a410ab 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -6885,7 +6885,8 @@ describe('useQuery', () => { const initialFetchCount = fetchCount - renderWithClient(queryClient, + renderWithClient( + queryClient, (
@@ -6895,7 +6896,7 @@ describe('useQuery', () => { )} > - + , ) await vi.waitFor(() => diff --git a/packages/react-query/src/errorBoundaryUtils.ts b/packages/react-query/src/errorBoundaryUtils.ts index 7e7b44afcd..ccd3993f2e 100644 --- a/packages/react-query/src/errorBoundaryUtils.ts +++ b/packages/react-query/src/errorBoundaryUtils.ts @@ -27,17 +27,17 @@ export const ensurePreventErrorBoundaryRetry = < errorResetBoundary: QueryErrorResetBoundaryValue, query?: Query, ) => { - if ( - options.suspense || - options.experimental_prefetchInRender - ) { + if (options.suspense || options.experimental_prefetchInRender) { // Prevent retrying failed query if the error boundary has not been reset yet if (!errorResetBoundary.isReset()) { options.retryOnMount = false } } else if (options.throwOnError && !errorResetBoundary.isReset()) { if (typeof options.throwOnError === 'function') { - if (query?.state.error && shouldThrowError(options.throwOnError, [query.state.error, query])) { + if ( + query?.state.error && + shouldThrowError(options.throwOnError, [query.state.error, query]) + ) { options.retryOnMount = false } } else { diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index 06690b544f..4158c5734f 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -53,6 +53,14 @@ export function useBaseQuery< const errorResetBoundary = useQueryErrorResetBoundary() const client = useQueryClient(queryClient) const defaultedOptions = client.defaultQueryOptions(options) + const query = client + .getQueryCache() + .get< + TQueryFnData, + TError, + TQueryData, + TQueryKey + >(defaultedOptions.queryHash) ;(client.getDefaultOptions().queries as any)?._experimental_beforeQuery?.( defaultedOptions, @@ -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 diff --git a/packages/react-query/src/useQueries.ts b/packages/react-query/src/useQueries.ts index a736f5cd1d..6eabef4060 100644 --- a/packages/react-query/src/useQueries.ts +++ b/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) From 199fc0aa567134fc0ceafd55950fa19f2aaed9e5 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 9 Nov 2025 21:59:09 +0900 Subject: [PATCH 6/6] 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 --- .../src/__tests__/useQuery.test.tsx | 65 ++++++++----------- packages/react-query/src/useBaseQuery.ts | 21 +++--- 2 files changed, 38 insertions(+), 48 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 39db9dd969..3c7f7562a8 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -6810,26 +6810,20 @@ describe('useQuery', () => { ) } - const { unmount, getByTestId } = renderWithClient( - queryClient, - , - ) - - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), + const rendered1 = renderWithClient(queryClient, ) + await vi.advanceTimersByTimeAsync(0) + expect(rendered1.getByTestId('status')).toHaveTextContent('error') + expect(rendered1.getByTestId('error')).toHaveTextContent( + 'Simulated 500 error', ) - expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') expect(fetchCount).toBe(1) - - unmount() + rendered1.unmount() const initialFetchCount = fetchCount - renderWithClient(queryClient, ) - - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), - ) + const rendered2 = renderWithClient(queryClient, ) + await vi.advanceTimersByTimeAsync(0) + expect(rendered2.getByTestId('status')).toHaveTextContent('error') expect(fetchCount).toBe(initialFetchCount + 1) expect(queryFn).toHaveBeenCalledTimes(2) @@ -6862,7 +6856,7 @@ describe('useQuery', () => { ) } - const { unmount, getByTestId } = renderWithClient( + const rendered1 = renderWithClient( queryClient, ( @@ -6875,18 +6869,18 @@ describe('useQuery', () => { , ) - - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), + await vi.advanceTimersByTimeAsync(0) + expect(rendered1.getByTestId('status')).toHaveTextContent('error') + expect(rendered1.getByTestId('error')).toHaveTextContent( + 'Simulated 500 error', ) - expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') expect(fetchCount).toBe(1) - unmount() + rendered1.unmount() const initialFetchCount = fetchCount - renderWithClient( + const rendered2 = renderWithClient( queryClient, ( @@ -6899,10 +6893,8 @@ describe('useQuery', () => { , ) - - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), - ) + await vi.advanceTimersByTimeAsync(0) + expect(rendered2.getByTestId('status')).toHaveTextContent('error') // Should not retry because throwOnError returns true expect(fetchCount).toBe(initialFetchCount) @@ -6936,26 +6928,23 @@ describe('useQuery', () => { ) } - const { unmount, getByTestId } = renderWithClient( - queryClient, - , - ) + const rendered1 = renderWithClient(queryClient, ) - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), + await vi.advanceTimersByTimeAsync(0) + expect(rendered1.getByTestId('status')).toHaveTextContent('error') + expect(rendered1.getByTestId('error')).toHaveTextContent( + 'Simulated 500 error', ) - expect(getByTestId('error')).toHaveTextContent('Simulated 500 error') expect(fetchCount).toBe(1) - unmount() + rendered1.unmount() const initialFetchCount = fetchCount - renderWithClient(queryClient, ) + const rendered2 = renderWithClient(queryClient, ) - await vi.waitFor(() => - expect(getByTestId('status')).toHaveTextContent('error'), - ) + 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) diff --git a/packages/react-query/src/useBaseQuery.ts b/packages/react-query/src/useBaseQuery.ts index 4158c5734f..e14a1a5d42 100644 --- a/packages/react-query/src/useBaseQuery.ts +++ b/packages/react-query/src/useBaseQuery.ts @@ -80,7 +80,15 @@ export function useBaseQuery< : 'optimistic' ensureSuspenseTimers(defaultedOptions) - ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query) + ensurePreventErrorBoundaryRetry( + defaultedOptions, + errorResetBoundary, + client + .getQueryCache() + .get( + defaultedOptions.queryHash, + ), + ) useClearResetErrorBoundary(errorResetBoundary) // this needs to be invoked before creating the Observer because that can create a cache entry @@ -134,14 +142,7 @@ export function useBaseQuery< result, errorResetBoundary, throwOnError: defaultedOptions.throwOnError, - query: client - .getQueryCache() - .get< - TQueryFnData, - TError, - TQueryData, - TQueryKey - >(defaultedOptions.queryHash), + query, suspense: defaultedOptions.suspense, }) ) { @@ -162,7 +163,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