fix(query-core): ensure query refetches on mount/retry when status is error (#9728) #9927
Conversation
🦋 Changeset detectedLatest commit: 909a504 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds a changeset for a patch to Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-11-22T09:06:05.219ZApplied to files:
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
xiangnuans
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/open-keys-create.md (1)
1-5: Changeset scope and message look accurate; optionally mention focus/reconnect.Patch scope to
@tanstack/query-coreand the summary align with theshouldFetchOnchange. Note that the implementation also affects refetch on window focus and reconnect via the same helper, which is fine but not called out here; up to you if you want to spell that out in the note.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/open-keys-create.md(1 hunks)packages/query-core/src/queryObserver.ts(1 hunks)packages/react-query/src/__tests__/issue-9728.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/react-query/src/__tests__/issue-9728.test.tsx
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
.changeset/open-keys-create.md
🧬 Code graph analysis (2)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/query.ts (1)
isStale(296-306)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
packages/query-core/src/queryObserver.ts (1)
refetch(298-304)
🔇 Additional comments (4)
packages/query-core/src/queryObserver.ts (1)
771-791: Error-awareshouldFetchOnlogic matches the intended fix.Adding
query.state.status === 'error'to the fetch predicate ensures that enabled queries with non‑static staleTime will refetch on mount/focus/reconnect when they are in an error state, even if their data isn’t time‑stale, which is exactly what issue 9728 describes. Guarding withvalue !== falsepreserves opt‑out viarefetchOn* = false, and the existingstaleTime === 'static'early return means static queries keep their previous behavior.packages/react-query/src/__tests__/issue-9728.test.tsx (3)
16-36: Deterministic queryFn and client config nicely model the regression.Using a
vi.fnwith acount‑based async implementation plusretry: falseandstaleTime: Infinityprovides a clear, deterministic setup for success→error→success, matching the reported bug scenario.
37-78: Error boundary + reset wiring correctly exercises “retry via remount”.
Pagethrowing viathrowOnError: trueunderErrorBoundaryandQueryErrorResetBoundary, with the Retry button callingresetErrorBoundary, ensures the query is remounted after an error in the presence of existing data—exactly the path the core fix targets.
88-107: Assertions cover initial success, error transition, and final successful refetch.The three phases—first render success (1 call), refetch leading to error UI (2 calls), and Retry leading to “Success 3” (3 calls)—give good coverage for the bug and will regress if
shouldFetchOnstops refetching on error with non‑stale data.
|
View your CI Pipeline Execution ↗ for commit 909a504
☁️ Nx Cloud last updated this comment at |
…king 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.
…nuans/query into fix/9728-retry-after-error
Set isInvalidated: true unconditionally since queries with no data are always considered stale per isStaleByTime logic.
…nuans/query into fix/9728-retry-after-error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
38-42: Consider removing the unused forceUpdate pattern.The
useStateanduseEffectcombination forces a re-render on mount, but this doesn't appear necessary for the test scenario. The test assertions pass without relying on this behavior, and it may confuse future readers.🔎 Suggested simplification
function Page() { - const [_, forceUpdate] = React.useState(0) - - React.useEffect(() => { - forceUpdate(1) - }, []) - const { data, refetch } = useQuery({ queryKey: key, queryFn, throwOnError: true, })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/react-query/src/__tests__/issue-9728.test.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/react-query/src/__tests__/issue-9728.test.tsx
🔇 Additional comments (1)
packages/react-query/src/__tests__/issue-9728.test.tsx (1)
1-108: Well-structured regression test for issue 9728.The test correctly validates that queries in error state with
staleTime: Infinitywill refetch on mount/retry. The mock setup, error boundary integration, and async assertions are implemented properly. The three-step test flow (success → error → retry success) clearly demonstrates the fix.Based on learnings, the
asyncqueryFn with side effects (incrementingcount) follows the recommended pattern.
|
your new tests don’t seem to pass |
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.
|
Thanks for catching that! The tests were timing out because when I've fixed this by adding The tests are now passing ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9927 +/- ##
===========================================
+ Coverage 45.79% 59.57% +13.77%
===========================================
Files 200 129 -71
Lines 8520 5734 -2786
Branches 1975 1582 -393
===========================================
- Hits 3902 3416 -486
+ Misses 4158 2001 -2157
+ Partials 460 317 -143
🚀 New features to boost your workflow:
|
|
🎉 This PR has been released! Thank you for your contribution! |
2 participants
error (#9728)
🎯 Changes
shouldFetchOninpackages/query-core/src/queryObserver.ts:query.state.status === 'error'.refetchOnWindowFocus,refetchOnMount, or manual retry via ErrorBoundary), it will trigger a fetch if the query is in an error state, even if the data is technically considered "fresh" (not stale) due tostaleTime: Infinity.packages/react-query/src/__tests__/issue-9728.test.tsxto reproduce the issue and verify the fix.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.