query icon indicating copy to clipboard operation
query copied to clipboard

fix(chore): report errors of mutation callbacks asynchronously

Open SimonSimCity opened this issue 3 months ago • 6 comments

Fixes #9664, by catching the error of onSuccess, onError and onSettled callbacks passed to the mutate function and reporting it on a separate execution context.

This change will catch all errors and, by passing it to Promise.reject(e), move it to a new execution context, which we explicitly want to ignore (hence the void keyword).

By ignoring the error on the newly created execution context, this error is reported by https://developer.mozilla.org/en-US/docs/Web/API/Window/unhandledrejection_event, where tools like Sentry can pick it up.

Raising of an unhandledRejection event is crucial to help the developer to be informed about their function misbehaving, and the surrounding try-catch will help securing this libraries code, despite of the fact that something failed in the users codebase.

Not to be confused with https://github.com/TanStack/query/pull/9676, which handles a slightly different problem.

Summary by CodeRabbit

  • Bug Fixes

    • Lifecycle callback exceptions (success/error/settled) no longer disrupt mutation processing or observer notifications; such errors are isolated and surfaced asynchronously to avoid app crashes.
  • Tests

    • Added tests verifying that thrown errors from lifecycle callbacks are reported without preventing observer updates or multiple notification deliveries.

SimonSimCity avatar Sep 22 '25 14:09 SimonSimCity

Walkthrough

Wraps mutation lifecycle callbacks (onSuccess/onError/onSettled) in try/catch and converts thrown errors into rejected Promises via void Promise.reject(err). Adds tests verifying thrown callback errors are reported as unhandled rejections and that subscribers are still notified twice.

Changes

Cohort / File(s) Summary
Mutation observer logic
packages/query-core/src/mutationObserver.ts
Wrap onSuccess/onError/onSettled calls in try/catch; if a callback throws, call void Promise.reject(err) to surface the error without interrupting the mutation notification flow; preserve subscription notifications.
Tests for erroneous callbacks
packages/query-core/src/__tests__/mutationObserver.test.tsx
Add tests that install a global unhandledRejection listener, trigger mutations whose lifecycle callbacks throw, assert two unhandled rejections are emitted and that the observer still notifies subscribers twice; includes listener setup/teardown.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User Code
  participant MO as MutationObserver
  participant MF as mutationFn
  participant CB as Callbacks (onSuccess/onError/onSettled)
  participant LS as Subscribers
  participant PR as Promise.reject

  U->>MO: mutate(variables, { onSuccess/onError, onSettled })
  MO->>MF: execute mutationFn
  alt mutation succeeds
    MF-->>MO: result
    MO->>CB: try { onSuccess(result) }
    alt onSuccess throws
      CB-->>MO: throw e
      MO->>PR: void Promise.reject(e)
    end
    MO->>CB: try { onSettled(result, null) }
    alt onSettled throws
      CB-->>MO: throw e2
      MO->>PR: void Promise.reject(e2)
    end
  else mutation errors
    MF-->>MO: throw error
    MO->>CB: try { onError(error) }
    alt onError throws
      CB-->>MO: throw e
      MO->>PR: void Promise.reject(e)
    end
    MO->>CB: try { onSettled(undefined, error) }
    alt onSettled throws
      CB-->>MO: throw e2
      MO->>PR: void Promise.reject(e2)
    end
  end
  MO->>LS: notify subscribers (state updates)
  MO->>LS: notify subscribers (settled)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit saw callbacks tumble and spree,
I caught each fall with a careful plea.
Rejected promises whisper the error,
Subscribers still get their double stirrer.
Hop on—state settles safe and free. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title succinctly describes the primary change: moving errors thrown by mutation callbacks to an asynchronous context for reporting. It directly reflects the mutationObserver.ts changes that wrap callback invocations and use void Promise.reject(e) to report callback errors without altering public APIs. The title is concise and relevant to the main change.
Linked Issues Check ✅ Passed The implementation matches the objectives of issue #9664 by catching errors in onSuccess/onError/onSettled and re-emitting them asynchronously via Promise.reject(e) so the library's settle/invalidation logic still runs; onSettled is invoked on both success and error paths and callback errors are surfaced as unhandled rejections. Unit tests added in mutationObserver.test.tsx assert that callback errors become unhandledRejection events while subscribers are still notified, which prevents a thrown callback from interrupting the settle flow that would leave isPending stuck. Based on the code changes and tests, the PR satisfies the coding-related requirements described in the linked issue.
Out of Scope Changes Check ✅ Passed The changes are limited to mutationObserver.ts and its tests, with no modifications to exported/public signatures or other packages, and the behavior changes are targeted solely at error handling for mutation callbacks as described in the PR objectives. No unrelated files or functionality appear to have been changed. Therefore there are no apparent out-of-scope changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • [ ] 📝 Generate Docstrings
🧪 Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10e81025dce76e8b03c38898c8bace04115c8bd8 and 7fbb7acb0696e6bba3f6586d2aca0e350b2fa5ba.

📒 Files selected for processing (1)
  • packages/query-core/src/__tests__/mutationObserver.test.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/query-core/src/tests/mutationObserver.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Test
  • GitHub Check: Preview

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.

coderabbitai[bot] avatar Sep 22 '25 14:09 coderabbitai[bot]

View your CI Pipeline Execution ↗ for commit 7556f45618e478dfc54b3715479a231e9e41b58b

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 2m 53s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 18s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-26 09:52:08 UTC

nx-cloud[bot] avatar Sep 22 '25 14:09 nx-cloud[bot]

More templates

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@9675
@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@9675
@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@9675
@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@9675
@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@9675
@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@9675
@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@9675
@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@9675
@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@9675
@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@9675
@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@9675
@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@9675
@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@9675
@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@9675
@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@9675
@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@9675
@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@9675
@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@9675
@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@9675
@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@9675

commit: 7556f45

pkg-pr-new[bot] avatar Sep 22 '25 14:09 pkg-pr-new[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 60.70%. Comparing base (cd29063) to head (7556f45). :warning: Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #9675       +/-   ##
===========================================
+ Coverage   46.38%   60.70%   +14.32%     
===========================================
  Files         214      143       -71     
  Lines        8488     5734     -2754     
  Branches     1920     1544      -376     
===========================================
- Hits         3937     3481      -456     
+ Misses       4108     1953     -2155     
+ Partials      443      300      -143     
Components Coverage Δ
@tanstack/angular-query-experimental 93.85% <ø> (ø)
@tanstack/eslint-plugin-query ∅ <ø> (∅)
@tanstack/query-async-storage-persister 43.85% <ø> (ø)
@tanstack/query-broadcast-client-experimental 24.39% <ø> (ø)
@tanstack/query-codemods ∅ <ø> (∅)
@tanstack/query-core 97.49% <100.00%> (+0.01%) :arrow_up:
@tanstack/query-devtools 3.48% <ø> (ø)
@tanstack/query-persist-client-core 79.60% <ø> (ø)
@tanstack/query-sync-storage-persister 84.61% <ø> (ø)
@tanstack/query-test-utils ∅ <ø> (∅)
@tanstack/react-query 96.00% <ø> (ø)
@tanstack/react-query-devtools 10.00% <ø> (ø)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (ø)
@tanstack/solid-query 78.06% <ø> (ø)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client 100.00% <ø> (ø)
@tanstack/svelte-query 87.58% <ø> (ø)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client 100.00% <ø> (ø)
@tanstack/vue-query 71.10% <ø> (ø)
@tanstack/vue-query-devtools ∅ <ø> (∅)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Sep 22 '25 15:09 codecov[bot]

⚠️ No Changeset found

Latest commit: 7556f45618e478dfc54b3715479a231e9e41b58b

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

changeset-bot[bot] avatar Sep 26 '25 09:09 changeset-bot[bot]

If there's anything that would need a discussion or if you want further insight on why I wrote something the way I did, please ping me. I just want to help having this merged, because I see not having this in as a way of disguising errors in my codebase that I want to be aware of.

SimonSimCity avatar Dec 09 '25 11:12 SimonSimCity