error printer: print the properties and cause of an Error thrown through a JSC exception

[robobun]

What

When a test body throws synchronously, or a callback throws an uncaught exception, the printed error is missing everything except its name, message, and stack: own properties, the code line, and the cause chain are all dropped. The same error thrown from an async test (or rejected) prints all of them.

// my.test.js
import { test } from "bun:test";
test("sync throw", () => {
  const err = new Error("outer failure", { cause: new Error("inner cause") });
  err.code = "ERR_FIXTURE";
  err.detail = 42;
  throw err;
});

Before (bun test my.test.js, source preview lines elided; the same code inside a setTimeout callback under bun -e prints the same way):

error: outer failure
      at <anonymous> (/tmp/prdemo/my.test.js:6:9)
(fail) sync throw

After, which is what an async version of the same test already printed:

error: outer failure
 detail: 42,
   code: "ERR_FIXTURE"

      at <anonymous> (/tmp/prdemo/my.test.js:6:9)

error: inner cause
      at <anonymous> (/tmp/prdemo/my.test.js:3:55)
(fail) sync throw

This is what was behind the handoff that led here: a using resource whose dispose threw during a failing test printed only SuppressedError: with no trace of either error. See "Related" below.

Cause

Both entry points (BunTest::on_uncaught_exception after try_take_exception, and report_uncaught_exception) hand run_error_handler the JSC::Exception cell rather than the thrown value. That is deliberate: JSC__Exception__asJSValue returns the cell, and toZigException looks through it to take the stack of the throw site. But print_error_instance_js also passed the cell on to print_error_instance_body, where is_error_instance checks js_type() == ErrorInstance. An Exception cell is not an ErrorInstance, so the whole block that prints the code, the own properties, and the non-enumerable cause (and anything other PRs add to that block) was skipped. Rejections and top-level throws pass the Error itself, which is why only the synchronous and callback cases were affected.

This is not a regression; the Zig version had the same shape.

Fix

print_error_instance_js keeps passing the cell to remap_zig_exception, so the stack, source preview, and divot are unchanged, and passes the Error the exception wraps (JSValue::to_error, the existing helper for exactly this unwrap) to print_error_instance_body. Values that are not an ErrorInstance are passed through as before. Unwrapping everything would turn the current

ResolveMessage: Cannot find module './does-not-exist'
Require stack:
- /tmp/x/my.test.js

for a synchronous require() of a missing module into that plus a second error: Cannot find module ... block from the non-Error fallback, since BuildMessage/ResolveMessage are only intercepted before the body when they arrive bare. How non-Error values thrown through an exception should render (plain objects, internal messages, AggregateError members) is a rendering decision that this PR leaves as it is today; the third test pins the ResolveMessage case.

Exception::value() is renamed to as_js_value(): it returns the cell, not the value, and the old name is how this read as if the body was already getting the Error. Two call sites.

Related

• console,inspect: print SuppressedError .error and .suppressed #36662 adds .error/.suppressed printing for SuppressedError to the same body block. It fixes console.error(se) and a top-level using throw, but on its own a failing test body with a throwing dispose still prints a bare SuppressedError: , because that error arrives through the exception cell. With both changes the test prints SuppressedError:, then error: dispose failed, then the expect(received).toBe(expected) block (verified by building the two together). The same applies to the other open changes to that block (error printer: render non-Error cause values #35172 non-Error causes, error printer: print the AggregateError header, label [cause]/[errors] blocks, and guard the .errors walk #36602 AggregateError members): with this change they also take effect for synchronous test failures and uncaught callback exceptions.
• If console, error printer: render instanceof-Error objects as errors, not object dumps #35723 lands, the ErrorInstance check in print_error_instance_js should become its is_error_like() so function-style Error subclasses get the same treatment.

Verification

test/js/bun/test/stack.test.ts (the file holding the existing own-property printing tests) gains three spawned cases: the bun test failure above, the same error thrown from setTimeout under bun -e, and the synchronous missing-module require() whose output must stay single. The first two fail on the current release build on Linux and Windows and pass with the fix on both; the third passes on both builds (it only guards the ErrorInstance check) and leaves the stack frames and divot out of its snapshot because debug builds add an internal require frame there. Running test/js/bun/test and test/cli/test against this build showed no other output changes; the few failures there were unrelated to the diff (a full disk while they ran, plus two snapshot tests whose mismatches were a [4.08s] timing suffix and missing ANSI codes).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ed9a3896-fbb8-4e47-81b3-ca5f64c0c68c

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and 8e2d467.

📒 Files selected for processing (3)

• src/jsc/Exception.rs
• src/jsc/VirtualMachine.rs
• test/js/bun/test/stack.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed, waiting on CI.

Reproduced on the current release build (Linux, and Windows at the same commit as this branch) with a test body that synchronously throws an Error carrying code, an own property, and a cause: bun test printed only the name, message, and stack; the async version of the same test printed all of them. The same happens for an uncaught exception thrown from a setTimeout callback.

Cause: both paths print from the JSC::Exception cell (needed for the throw-site stack), and print_error_instance_body received the cell instead of the Error it wraps, so its is_error_instance block never ran.

Fix: print_error_instance_js now hands the body the wrapped Error (stack still taken through the cell). Tests in test/js/bun/test/stack.test.ts fail on the release build and pass with the fix on Linux and Windows.

This is the second half of the SuppressedError: report that led here; #36662 is the other half, and the two together make a failing test with a throwing using dispose print both errors.

[claude]

LGTM — small, well-guarded fix that unwraps the JSC::Exception cell before handing it to the error-body printer.

What was reviewed:

• Verified JSValue::to_error unwraps JSC::Exception cells (bindings.cpp:5013) and that js_type() is safe on non-cell values (returns CellType), so throw 42 through an exception cell falls through to the original path.
• Confirmed the ErrorInstance guard preserves existing behavior for ResolveMessage/BuildMessage and plain-object throws — only genuine ErrorInstance values are unwrapped.
• Checked the value() → as_js_value() rename is complete (no remaining Exception::value() callers in src/).

Extended reasoning...

Overview

The PR fixes error printing when an Error reaches the printer wrapped in a JSC::Exception cell (synchronous test throws, uncaught callback exceptions). Previously print_error_instance_body received the cell, whose js_type() is not ErrorInstance, so the block that prints code, own properties, and the cause chain was skipped. The fix is a 4-line unwrap in print_error_instance_js that calls to_error() and passes the wrapped ErrorInstance to the body while still passing the cell to remap_zig_exception (so the throw-site stack is preserved). A method rename (Exception::value → as_js_value) clarifies that it returns the cell, not the wrapped value.

Security risks

None. This is output formatting only — no parsing of untrusted input, no allocation changes, no refcount/GC changes.

Level of scrutiny

Medium-low. The change is 4 substantive lines plus a rename and doc comments. I verified the semantics of JSC__JSValue__toError_ in bindings.cpp (returns the value itself for ErrorInstanceType, exception->value() for JSC::Exception cells, empty otherwise) and that js_type() guards non-cells. The ErrorInstance match-guard means non-Error thrown values (primitives, ResolveMessage, plain objects) fall through unchanged to the existing path — the third test pins this. The change is strictly additive: it prints more detail than before, matching what the async/rejection path already prints.

Other factors

• Three spawned tests added to the existing stack.test.ts (correct location per repo conventions). Tests 1 and 2 exercise both entry points (bun test sync throw, setTimeout callback) and would fail without the fix. Test 3 guards the ResolveMessage non-unwrap case.
• Tests follow harness conventions: tempDir, bunEnv, normalizeBunSnapshot, test.concurrent, drain-then-assert-exit-code, await using for spawned processes.
• The rename covers both call sites (verified via grep); the repo checkout is at the PR HEAD so any missed site would fail cargo check.
• The PR description is unusually thorough and cross-references three related open PRs whose fixes will now also apply to synchronous throws.