Skip to content

fix(napi): populate Error::cause from ThreadsafeFunction callee-handled callbacks#3162

Merged
Brooooooklyn merged 6 commits into
napi-rs:mainfrom
hadronzoo:fix/threadsafe-function-error-cause
Apr 3, 2026
Merged

fix(napi): populate Error::cause from ThreadsafeFunction callee-handled callbacks#3162
Brooooooklyn merged 6 commits into
napi-rs:mainfrom
hadronzoo:fix/threadsafe-function-error-cause

Conversation

@hadronzoo

@hadronzoo hadronzoo commented Mar 24, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Fixes #3161.

When a callee-handled ThreadsafeFunction callback throws, call_js_cb constructs the Error directly with cause: None. This means any cause set on the JavaScript error is silently dropped.

Root cause

call_js_cb builds an Error struct directly:

Err(Error {
  maybe_raw: error_reference,
  maybe_env: raw_env,
  cause: None,  // <-- always None
  status: Status::from(raw_status),
  reason,
})

Both From<Unknown> impls already call extract_error_cause to populate this field, but that path is never reached in the TSF callback.

Fix

Make extract_error_cause pub(crate) and call it from call_js_cb, consistent with how From<Unknown> already handles cause extraction.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling in asynchronous callbacks to properly capture and preserve the underlying error cause from JavaScript exceptions.

@hadronzoo
fix: populate Error::cause from ThreadsafeFunction callee-handled cal…
889fd4e 
…lbacks

When a callee-handled ThreadsafeFunction callback throws, call_js_cb
builds an Error directly with cause: None. Add a call to
extract_error_cause (already used by both From<Unknown> impls) so the
cause chain is preserved.
@coderabbitai

coderabbitai Bot commented Mar 24, 2026 •
edited
Loading

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Exposes extract_error_cause as pub(crate) and uses it in the ThreadsafeFunction callback error path to populate Error::cause from a cleared JavaScript exception instead of leaving it None.

Changes

Cohort / File(s) Summary
Error Cause Exposure
crates/napi/src/error.rs		 Changed extract_error_cause visibility from private to pub(crate) so it can be reused across the crate (no logic changes).
Error Cause Integration
crates/napi/src/threadsafe_function.rs		 Imported and used extract_error_cause in the WithCallback exception path; replaced hardcoded cause: None with extracted cause from the cleared JS exception and added an unsafe block with a safety comment around Unknown::from_raw_unchecked(...).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Brooooooklyn

Poem

🐰 I dug through stack and trace,
Found a hidden cause in place,
Threads now pass the truth along,
A tiny hop, a clearer song. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR successfully implements the fix for issue #3161 by making extract_error_cause pub(crate) and calling it in call_js_cb to populate Error::cause, directly matching the suggested approach.
Out of Scope Changes check ✅ Passed All changes are scoped to addressing issue #3161: making extract_error_cause public for reuse and applying it in the ThreadsafeFunction callback error path.
Title check ✅ Passed The title accurately describes the main change: making Error::cause be populated from ThreadsafeFunction callee-handled callbacks, which is the central fix in this changeset.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Brooooooklyn

Brooooooklyn commented Apr 1, 2026

Copy link
Copy Markdown
Member

@hadronzoo you need to run cargo fmt

@hadronzoo

hadronzoo commented Apr 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

My mistake. Done.

@Brooooooklyn Brooooooklyn changed the title fix: populate Error::cause from ThreadsafeFunction callee-handled callbacks fix(napi): populate Error::cause from ThreadsafeFunction callee-handled callbacks Apr 3, 2026
@Brooooooklyn Brooooooklyn merged commit f10d6ce into napi-rs:main Apr 3, 2026
73 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 3, 2026
Merged
@coderabbitai coderabbitai Bot mentioned this pull request May 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

napi::Error::cause is not populated when a ThreadsafeFunction callback throws

2 participants

@hadronzoo @Brooooooklyn