fix(query-core): don't reject imperative fetch with CancelledError when a superseding fetch is started - #11061
Conversation
…en a superseding fetch is started When a fetch is silently cancelled because a new fetch supersedes it (e.g. `invalidateQueries` refetching an active observer with `cancelRefetch: true`), callers that joined the in-flight fetch via the early-return in `Query.fetch` received the raw retryer promise, which rejects with a silent `CancelledError`. The async `fetch` body already piggybacks onto the superseding fetch on silent cancellation, but the early-return path bypassed it. Route both the early-return and the catch-block piggyback through a shared helper that follows the chain of superseding fetches until one settles, while still rethrowing when the silent cancellation isn't from a superseding fetch (e.g. query destroyed). Closes TanStack#8060
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthrough
ChangesSilent cancellation continuation
Estimated code review effort: 3 (Moderate) | ~15 minutes Merge Risk: ⚪ Minimal · up to The change makes imperative fetches follow superseding refetches instead of surfacing a cancellation error; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant QueryClient
participant Query
participant QueryObserver
QueryClient->>Query: start imperative fetch
QueryObserver->>Query: refetch after invalidation
Query-->>Query: silently cancel superseded retryer
Query-->>QueryClient: follow latest retryer promise
Query-->>QueryClient: resolve with superseding result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/query-core/src/__tests__/queryClient.test.tsx (1)
1060-1094: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrengthen the assertion to verify fresh data from the superseding fetch.
expect.any(Number)would pass even iffetchPromiseresolved with stale data from the priming fetch (count = 1) instead of fresh data from the superseding fetch (count = 2). Asserting the specific value verifies the correct fetch result is propagated.♻️ Proposed fix
- await expect(fetchPromise).resolves.toEqual(expect.any(Number)) + await expect(fetchPromise).resolves.toBe(2)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/query-core/src/__tests__/queryClient.test.tsx` around lines 1060 - 1094, Strengthen the final assertion in the invalidation/refetch test so fetchPromise resolves specifically to 2, confirming it receives the superseding fetch’s fresh result rather than the priming result of 1. Keep the existing test flow and cleanup unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/query-core/src/__tests__/queryClient.test.tsx`:
- Around line 1060-1094: Strengthen the final assertion in the
invalidation/refetch test so fetchPromise resolves specifically to 2, confirming
it receives the superseding fetch’s fresh result rather than the priming result
of 1. Keep the existing test flow and cleanup unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58990ea9-f848-48e9-8acb-57928aedf22e
📒 Files selected for processing (2)
packages/query-core/src/__tests__/queryClient.test.tsxpackages/query-core/src/query.ts
|
Addressed the assertion-strengthening review in 2c8a3ec. I used |
|
View your CI Pipeline Execution ↗ for commit 20eacbc
☁️ Nx Cloud last updated this comment at |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
TkDodo
left a comment
TkDodo
left a comment
There was a problem hiding this comment.
I forked the initial reproduction from the issue and used the preview build to check it, but it seems to still throw the same unexpected CancelledError:
2 participants
Problem
When a fetch is silently cancelled because a new fetch supersedes it (e.g.
invalidateQueriesrefetching an active observer withcancelRefetch: true),callers that joined the in-flight fetch via the early-return in
Query.fetchreceived the raw retryer promise, which rejects with a silent
CancelledError.The async
fetchbody already piggybacks onto the superseding fetch on silentcancellation, but the early-return path bypassed it, so an imperative
fetchQuerycould reject with aCancelledErroreven though the querycontinued fetching.
Fix
Route both the early-return and the catch-block piggyback through a shared
helper that follows the chain of superseding fetches until one settles, while
still rethrowing when the silent cancellation isn't from a superseding fetch
(e.g. query destroyed).
Testing
Added a regression test in
packages/query-core/src/__tests__/queryClient.test.tsxcovering the scenario where an imperative fetch is superseded by an
invalidateQueries-triggered refetch, asserting the imperative call resolveswith the fresh data instead of rejecting with a
CancelledError.Checklist
query-coreRelease Impact
Low risk bug fix in
query-core. The change only corrects how imperative fetchesfollow a superseding refetch; it should not affect callers outside the failed
CancelledErrorpath.Closes #8060
Summary by CodeRabbit
Bug Fixes
Tests