Skip to content

Commit fccef79

Browse files
xiangnuansTkDodo
andauthored
fix(query-core): ensure query refetches on mount/retry when status is error (#9728) (#9927)
* fix(query-core): ensure query refetches on mount/retry when status is error (#9728) * fix: mark queries as invalidated on background errors instead of checking error status - Remove error status check in shouldFetchOn - Set isInvalidated: true in reducer when background error occurs (only if data exists) - Add tests for staleTime: 'static' and non-static queries with background errors This approach centralizes stale logic in isStale/isStaleByTime and prevents regression where staleTime: 'static' queries would incorrectly refetch on window focus after a background error. * refactor: simplify isInvalidated logic in error case Set isInvalidated: true unconditionally since queries with no data are always considered stale per isStaleByTime logic. * Apply suggestion from @TkDodo * Apply suggestions from code review * fix: eslint * fix: disable retry in background error tests to prevent timeout The tests were timing out because when refetch() throws an error, the default retry mechanism (3 retries with exponential backoff) was being triggered. With fake timers, the retry delays weren't being advanced, causing the tests to hang. Adding retry: false to both tests disables retries and allows them to complete immediately after the error, which is appropriate for these tests that specifically check background error behavior. --------- Co-authored-by: Dominik Dorfmeister <[email protected]>
1 parent e907f89 commit fccef79

File tree

4 files changed

+197
-0
lines changed

4 files changed

+197
-0
lines changed

.changeset/open-keys-create.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/query-core': patch
3+
---
4+
5+
Fix: Always treat existing data as stale when query goes into error state.

packages/query-core/src/__tests__/queryObserver.test.tsx

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,87 @@ describe('queryObserver', () => {
13941394
unsubscribe()
13951395
})
13961396

1397+
test('should not refetchOnWindowFocus when staleTime is static and query has background error', async () => {
1398+
const key = queryKey()
1399+
let callCount = 0
1400+
const queryFn = vi.fn(async () => {
1401+
callCount++
1402+
if (callCount === 1) {
1403+
return 'data'
1404+
}
1405+
throw new Error('background error')
1406+
})
1407+
1408+
const observer = new QueryObserver(queryClient, {
1409+
queryKey: key,
1410+
queryFn,
1411+
staleTime: 'static',
1412+
refetchOnWindowFocus: true,
1413+
retry: false,
1414+
})
1415+
1416+
const unsubscribe = observer.subscribe(() => undefined)
1417+
await vi.advanceTimersByTimeAsync(0)
1418+
expect(queryFn).toHaveBeenCalledTimes(1)
1419+
expect(observer.getCurrentResult().data).toBe('data')
1420+
expect(observer.getCurrentResult().status).toBe('success')
1421+
1422+
await observer.refetch()
1423+
await vi.advanceTimersByTimeAsync(0)
1424+
expect(queryFn).toHaveBeenCalledTimes(2)
1425+
expect(observer.getCurrentResult().status).toBe('error')
1426+
expect(observer.getCurrentResult().data).toBe('data')
1427+
1428+
focusManager.setFocused(false)
1429+
focusManager.setFocused(true)
1430+
await vi.advanceTimersByTimeAsync(0)
1431+
expect(queryFn).toHaveBeenCalledTimes(2)
1432+
1433+
unsubscribe()
1434+
})
1435+
1436+
test('should refetchOnWindowFocus when query has background error and staleTime is not static', async () => {
1437+
const key = queryKey()
1438+
let callCount = 0
1439+
const queryFn = vi.fn(async () => {
1440+
callCount++
1441+
if (callCount === 1) {
1442+
return 'data'
1443+
}
1444+
if (callCount === 2) {
1445+
throw new Error('background error')
1446+
}
1447+
return 'new data'
1448+
})
1449+
1450+
const observer = new QueryObserver(queryClient, {
1451+
queryKey: key,
1452+
queryFn,
1453+
staleTime: 1000,
1454+
refetchOnWindowFocus: true,
1455+
retry: false,
1456+
})
1457+
1458+
const unsubscribe = observer.subscribe(() => undefined)
1459+
await vi.advanceTimersByTimeAsync(0)
1460+
expect(queryFn).toHaveBeenCalledTimes(1)
1461+
expect(observer.getCurrentResult().data).toBe('data')
1462+
expect(observer.getCurrentResult().status).toBe('success')
1463+
1464+
await observer.refetch()
1465+
await vi.advanceTimersByTimeAsync(0)
1466+
expect(queryFn).toHaveBeenCalledTimes(2)
1467+
expect(observer.getCurrentResult().status).toBe('error')
1468+
expect(observer.getCurrentResult().data).toBe('data')
1469+
1470+
focusManager.setFocused(false)
1471+
focusManager.setFocused(true)
1472+
await vi.advanceTimersByTimeAsync(0)
1473+
expect(queryFn).toHaveBeenCalledTimes(3)
1474+
1475+
unsubscribe()
1476+
})
1477+
13971478
test('should set fetchStatus to idle when _optimisticResults is isRestoring', () => {
13981479
const key = queryKey()
13991480
const observer = new QueryObserver(queryClient, {

packages/query-core/src/query.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,9 @@ export class Query<
658658
fetchFailureReason: error,
659659
fetchStatus: 'idle',
660660
status: 'error',
661+
// flag existing data as invalidated if we get a background error
662+
// note that "no data" always means stale so we can set unconditionally here
663+
isInvalidated: true,
661664
}
662665
case 'invalidate':
663666
return {
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// @vitest-environment jsdom
2+
import { describe, expect, it, vi } from 'vitest'
3+
import { fireEvent, render } from '@testing-library/react'
4+
import * as React from 'react'
5+
import { ErrorBoundary } from 'react-error-boundary'
6+
import { queryKey } from '@tanstack/query-test-utils'
7+
import {
8+
QueryClient,
9+
QueryClientProvider,
10+
QueryErrorResetBoundary,
11+
useQuery,
12+
} from '..'
13+
14+
describe('issue 9728', () => {
15+
it('should refetch after error when staleTime is Infinity and previous data exists', async () => {
16+
const key = queryKey()
17+
const queryFn = vi.fn()
18+
let count = 0
19+
20+
queryFn.mockImplementation(async () => {
21+
count++
22+
if (count === 2) {
23+
throw new Error('Error ' + count)
24+
}
25+
return 'Success ' + count
26+
})
27+
28+
const queryClient = new QueryClient({
29+
defaultOptions: {
30+
queries: {
31+
retry: false,
32+
staleTime: Infinity,
33+
},
34+
},
35+
})
36+
37+
function Page() {
38+
const [_, forceUpdate] = React.useState(0)
39+
40+
React.useEffect(() => {
41+
forceUpdate(1)
42+
}, [])
43+
44+
const { data, refetch } = useQuery({
45+
queryKey: key,
46+
queryFn,
47+
throwOnError: true,
48+
})
49+
50+
return (
51+
<div>
52+
<div>Data: {data}</div>
53+
<button onClick={() => refetch()}>Refetch</button>
54+
</div>
55+
)
56+
}
57+
58+
function App() {
59+
return (
60+
<QueryErrorResetBoundary>
61+
{({ reset }) => (
62+
<ErrorBoundary
63+
onReset={reset}
64+
fallbackRender={({ resetErrorBoundary }) => (
65+
<div>
66+
<div>Status: error</div>
67+
<button onClick={resetErrorBoundary}>Retry</button>
68+
</div>
69+
)}
70+
>
71+
<React.Suspense fallback={<div>Loading...</div>}>
72+
<Page />
73+
</React.Suspense>
74+
</ErrorBoundary>
75+
)}
76+
</QueryErrorResetBoundary>
77+
)
78+
}
79+
80+
const { getByText, findByText } = render(
81+
<React.StrictMode>
82+
<QueryClientProvider client={queryClient}>
83+
<App />
84+
</QueryClientProvider>
85+
</React.StrictMode>,
86+
)
87+
88+
// 1. First mount -> Success
89+
await findByText('Data: Success 1')
90+
expect(queryFn).toHaveBeenCalledTimes(1)
91+
92+
// 2. Click Refetch -> Triggers fetch -> Fails (Error 2) -> ErrorBoundary
93+
fireEvent.click(getByText('Refetch'))
94+
95+
// Wait for error UI
96+
await findByText('Status: error')
97+
expect(queryFn).toHaveBeenCalledTimes(2)
98+
99+
// 3. Click Retry -> Remounts
100+
// Because staleTime is Infinity and we have Data from (1),
101+
// AND we are in Error state.
102+
fireEvent.click(getByText('Retry'))
103+
104+
// Should call queryFn again (3rd time) and succeed
105+
await findByText('Data: Success 3')
106+
expect(queryFn).toHaveBeenCalledTimes(3)
107+
})
108+
})

0 commit comments

Comments
 (0)