Skip to content

fix(query-core): handle combine throwing in QueriesObserver notify#10137

Open
veeceey wants to merge 1 commit intoTanStack:mainfrom
veeceey:fix/issue-10129-suspense-queries-combine
Open

fix(query-core): handle combine throwing in QueriesObserver notify#10137
veeceey wants to merge 1 commit intoTanStack:mainfrom
veeceey:fix/issue-10129-suspense-queries-combine

Conversation

@veeceey
Copy link

@veeceey veeceey commented Feb 15, 2026

Fixes #10129

When useSuspenseQueries is used with combine, the types narrow data to always be defined. But if a query transitions back to pending/error state (e.g. via queryClient.resetQueries()), the combine function gets called with results where data is undefined, causing a runtime TypeError.

This happens because QueriesObserver.#notify() calls combine as an optimization check — to see if the combined result changed before notifying listeners. The listeners themselves receive raw results (not the combined one), so the combine call is purely for deduplication.

The fix wraps the combine call in a try/catch. If combine throws, we treat it as a change and notify listeners anyway. This lets the framework-level code (React Suspense / Error Boundary) handle the state transition properly.

Added a test that reproduces the exact scenario: subscribe to a QueriesObserver with a combine function that accesses data properties, let queries resolve, then call resetQueries — previously this would throw, now it correctly notifies listeners with the pending result.

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling in query observation notifications to ensure listeners are properly notified even when data transformation encounters errors.
  • Tests

    • Added test coverage for notification behavior during query state transitions with error scenarios.

When useSuspenseQueries is used with combine and a query transitions
to pending/error state (e.g. after resetQueries), the combine function
throws because data is undefined despite types narrowing it as defined.

The #notify method calls combine as an optimization to check if the
combined result changed. If combine throws during this check, we now
catch the error and still notify listeners so the framework can
re-suspend or show an error boundary.

Fixes TanStack#10129
@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2026

⚠️ No Changeset found

Latest commit: c380c87

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

The changes add error handling to the QueriesObserver.notify() method to safely handle failures when the combine function throws an error. A corresponding test validates that listeners receive notifications even when the combine function fails during query state transitions, addressing a bug where the combine function was called with pending/error results despite type signatures implying data is always defined.

Changes

Cohort / File(s) Summary
Core Implementation
packages/query-core/src/queriesObserver.ts
Added try/catch error handling in notify method around newResult computation. Introduced shouldNotify flag logic to ensure notifications proceed even if combine function throws, while maintaining existing result update flow.
Test Coverage
packages/query-core/src/__tests__/queriesObserver.test.tsx
Added new test asserting that listeners are notified with pending status and undefined data even when the combine function throws after a query reset.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

package: query-core

Suggested reviewers

  • TkDodo

Poem

🐰 A query observer learns to catch and care,
When combine functions stumble through the air,
Listeners still hear the tale they're told,
Even when promises haven't quite unfold! ✨

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: handling the combine function throwing errors in QueriesObserver's notify method, which is the core change.
Description check ✅ Passed The description covers the problem, solution, and reasoning well. However, the checklist items (Contributing guide confirmation and test verification) are not marked as completed.
Linked Issues check ✅ Passed The PR correctly addresses issue #10129 by wrapping the combine call in try/catch to handle throwing, which allows framework-level code to handle state transitions instead of propagating the error.
Out of Scope Changes check ✅ Passed All changes are directly within scope: the test reproduces the exact issue scenario, and the implementation fix in queriesObserver.ts handles combine throwing as required by #10129.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
packages/query-core/src/queriesObserver.ts (1)

300-313: Solid fix for the combine-throws-on-reset scenario.

The try/catch approach is a pragmatic solution: it lets the notification propagate so framework-level Suspense/ErrorBoundary can handle the state transition, which is the right call.

One subtlety worth noting: inside #combineResult, #lastCombine and #lastResult are assigned before combine(input) is invoked (lines 235–236). If combine throws, those caching fields are updated but #combinedResult is not. This leaves the internal memoization in an inconsistent state. In practice this is safe because #onUpdate always creates a new #result array reference (via replaceAt), which re-triggers the combine on the next #notify. Still, resetting those fields in the catch block would make the invariant explicit:

Optional hardening
       shouldNotify = previousResult !== newResult
     } catch {
       // If combine throws (e.g. when used with useSuspenseQueries and
       // a query transitions to pending/error state after a reset), we
       // still need to notify so the framework can re-suspend or show
       // an error boundary.
       shouldNotify = true
+      this.#lastResult = undefined
+      this.#lastCombine = undefined
+      this.#combinedResult = undefined
     }

This ensures the next #combineResult call won't short-circuit on stale cache and will always re-run combine.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

useSuspenseQueries combine is called with pending/error results despite types implying data is always defined

1 participant