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 Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesSilent cancellation continuation
Estimated code review effort: 3 (Moderate) | ~15 minutes 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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
1 participant
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.Closes #8060
Summary by CodeRabbit
Bug Fixes
Tests
fetchQueryresolution during invalidation while an observer is refetching.