From 878ac3489477138d95bb09feb5d2ed62a22f89d1 Mon Sep 17 00:00:00 2001 From: sangwook Date: Mon, 30 Jun 2025 23:13:14 +0900 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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)