fix(query-core): avoid throwing promise errors when data exists #10022
Conversation
🦋 Changeset detectedLatest commit: cf7059e 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 |
📝 WalkthroughWalkthroughRefines thenable handling in Changes
Sequence Diagram(s)(omitted — changes do not introduce a multi-component sequential feature flow that benefits from a diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-query/src/__tests__/useQuery.promise.test.tsx (1)
1432-1497: Good regression test for the core issue.The test correctly validates that
fetchNextPageerrors with existing data don't propagate to the Error Boundary, directly addressing issue #9938. The structure follows existing patterns in this file.One minor consideration: other tests that involve errors (e.g., lines 496-498) mock
console.errorto suppress React's error logging noise. Since this test involves an error being thrown (even though it's caught internally), you might see spurious console output during test runs. Consider adding the mock if you observe test noise:🔧 Optional: Suppress console.error noise
it('should not throw to error boundary for refetch errors in infinite queries', async () => { const key = queryKey() const renderStream = createRenderStream({ snapshotDOM: true }) + const consoleMock = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined) function Page() { // ... component code } // ... test assertions expect(rendered.queryByText('error boundary')).toBeNull() + consoleMock.mockRestore() })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/query-core/src/queryObserver.tspackages/react-query/src/__tests__/useQuery.promise.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 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/query-core/src/queryObserver.tspackages/react-query/src/__tests__/useQuery.promise.test.tsx
🧬 Code graph analysis (2)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/thenable.ts (1)
PendingThenable(37-37)
packages/react-query/src/__tests__/useQuery.promise.test.tsx (1)
packages/query-core/src/queryObserver.ts (1)
query(700-717)
🔇 Additional comments (2)
packages/query-core/src/queryObserver.ts (2)
595-603: Core fix correctly aligns promise behavior with Suspense defaults.The introduction of
hasResultDataandisErrorWithoutDataflags elegantly distinguishes between:
- Initial errors (no data) → reject thenable → trigger Error Boundary
- Subsequent errors with existing data → keep thenable resolved → no Error Boundary
This matches how
useSuspenseInfiniteQueryhandles subsequent page errors, resolving the behavioral inconsistency reported in issue #9938.
626-636: Thenable recreation logic correctly handles state transitions.The updated conditions ensure proper behavior:
- Fulfilled → Error with data:
isErrorWithoutDataisfalse, so we don't recreate unless data actually changed. The thenable stays resolved, preventing Error Boundary activation.- Rejected → Data available:
!isErrorWithoutDatatriggers recreation when data exists, allowing recovery from error states.
HyeongSeoku
changed the title
experimental_prefetchInRenderexperimental_prefetchInRender
HyeongSeoku
changed the title
experimental_prefetchInRender1 participant
🎯 Changes
Fixes: #9938
experimental_prefetchInRenderpromise rejection behavior with Suspense defaults.useInfiniteQuery).🧠 Motivation
useSuspenseInfiniteQueryonly throws to the Error Boundary when there is no data to show. However,useInfiniteQuerycombined withexperimental_prefetchInRenderwas rejecting the promise even when data existed (e.g. page 2 fetch errors), which made Error Boundary appear unexpectedly. This change makes both paths consistent.🔧 Implementation Notes
packages/query-core/src/queryObserver.ts.packages/react-query/src/__tests__/useQuery.promise.test.tsxto ensure refetch/page errors with existing data do not throw to Error Boundary.🧪 Testing
✅ Checklist
📷 Optional Evidence
Before: fetch next page error triggers Error Boundary
After: fetch next page error does not trigger Error Boundary;
isFetchNextPageError: trueis shownSummary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.