bun:test: don't abort when formatting a test.each title value throws

[robobun]

Problem

A test.each / describe.each title interpolating a value via $path (or %p) aborts the test runner when formatting that value throws, e.g. a throwing [Symbol.for("nodejs.util.inspect.custom")]:

import { test } from "bun:test";
test.each([{ a: { b: { [Symbol.for("nodejs.util.inspect.custom")]() { throw 1 } } } }])("case $a.b", () => {});

panic: a formatting trait implementation returned an error when the underlying stream did not

Cause

format_label in src/runtime/test_runner/jest.rs formats non-string interpolated values with write!(&mut list, "{}", value.to_fmt(&mut formatter)) through std::io::Write. ZigFormatter's Display impl collapses the pending JS exception to fmt::Error, and std::io::Write::write_fmt panics when the Display impl errors while the sink (Vec<u8>) did not. The %p branch in the same function has the identical bug.

Fix

Add Formatter::format_value, a fallible single-value entry point that propagates the JsError (thrown exception, termination, OOM) instead of collapsing it to fmt::Error, and use it at both format_label sites. The thrown exception now surfaces as a test error and bun test exits 1 instead of crashing.

The Formatter::format_value hunk is byte-identical to the one in #36912 so the two merge cleanly in either order.

Sibling sites with the same write!-through-Display pattern that are intentionally not touched here because they already have their own PRs: ExpectMatcherUtils::print_value in expect.rs (#36912) and bun_inspect in BunObject.rs (#30980).

Verification

New test in test/cli/test/bun-test.test.ts covers test.each and describe.each for both the $path and %p sites, one fixture file per variant with a distinct error message (the declaration throw aborts module evaluation, so a single file can only exercise the first one). It aborts with the panic on a build without the fix and passes with it. The full bun-test.test.ts file passes.

---

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/test/bun-test.test.ts

[coderabbitai]

Walkthrough

Changes

The formatter now writes values through a fallible API. Test runner formatting propagates JavaScript errors instead of panicking. CLI tests cover throwing custom inspect formatters in interpolation and %p output.

Formatter error propagation

Layer / File(s)	Summary
Add fallible formatter output src/jsc/ConsoleObject.rs	ZigFormatter::write_to writes stored values to a bun_io::Write sink, restores state, and prevents reuse after consumption.
Propagate errors in test output src/runtime/test_runner/jest.rs, test/cli/test/bun-test.test.ts	Label and %p formatting now propagates formatter errors. Tests verify custom formatter failures and exit code 1.

Possibly related PRs

• oven-sh/bun#36912: Both PRs add fallible formatter entry points and propagate custom-inspect errors, but target different callers.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly describes the primary fix for formatting errors in test.each titles.
Description check	✅ Passed	The description explains the problem, cause, fix, affected paths, and verification results with sufficient detail.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/cli/test/bun-test.test.ts`:
- Around line 1221-1234: Expand the throwing custom-formatter coverage around
the existing test.each fixture to include describe.each, using distinct error
messages for the $a.b and %p formatter paths. Capture and assert each execution
result separately so both formatter variants and the sibling describe.each API
are independently verified.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23677408-4078-48c0-bd12-52a492517104

📥 Commits

Reviewing files that changed from the base of the PR and between ace8f42 and 648ea0d.

📒 Files selected for processing (3)

• src/jsc/ConsoleObject.rs
• src/runtime/test_runner/jest.rs
• test/cli/test/bun-test.test.ts

[claude]

LGTM — both prior points are addressed: the test now covers test.each/describe.each × $path/%p in separate fixture files with distinct asserted messages, and the sibling write!-through-Display sites are named as deferred to #36912 / #30980.

What was reviewed:

• Formatter::format_value mirrors the existing ZigFormatter::fmt seat/clear of remaining_values and resets it on the error path too; Vec<u8> already implements bun_io::Write (bun_core/util.rs:1869).
• Both format_label call sites ?-propagate into an existing JsResult return; the remaining write!(...).unwrap() in %# formats a usize and cannot enter JS.
• stack_check.update() seats the recursion limit that make_formatter leaves at default(); the old to_fmt path did not, so this is strictly no worse.

Extended reasoning...

Overview

Adds Formatter::format_value (fallible single-value entry point returning JsResult<()>) in src/jsc/ConsoleObject.rs, and swaps the two write!(&mut list, "{}", value.to_fmt(...)).unwrap() sites in format_label (src/runtime/test_runner/jest.rs:713 and :790) to formatter.format_value::<false>(value, &mut list)?. A new test in test/cli/test/bun-test.test.ts runs four one-line fixture files (one per test.each/describe.each × $path/%p combination), each throwing a distinct message from [Symbol.for("nodejs.util.inspect.custom")], and asserts all four appear in stderr with exit code 1.

Security risks

None. This changes error propagation inside the test-title formatter — no new user-controlled input surface, no auth/crypto/permissions.

Level of scrutiny

Medium: touches native Formatter state (remaining_values is a RawSlice into a stack local). I checked that format_value copies the existing ZigFormatter::fmt pattern (ConsoleObject.rs:1823–1836): the raw slice is seated to &one, the fallible work is captured into result, and remaining_values is reset to EMPTY before returning — so no dangling raw slice on the error path. Vec<u8> implements bun_io::Write via the blanket at bun_core/util.rs:1869, so &mut list unsizes to &mut dyn bun_io::Write without a wrapper.

Other factors

This is a re-review after 2da2ba3/8647470/7356fa2. My two earlier comments — (1) the %p case was unreachable behind the $path throw in a single fixture, and (2) the same panic class exists at expect.rs/BunObject.rs — are both addressed: the test is split into four independently-evaluated files with distinct asserted messages, and the PR description now names the sibling sites as intentionally deferred to #36912 and #30980 (with the format_value hunk kept byte-identical to #36912 so they merge cleanly). CodeRabbit and the comment-cop bot are also resolved. No outstanding reviewer comments.

[claude]

LGTM — the panic is now a propagated JsError, and the earlier feedback (separate fixture per variant, sibling sites named in the description, shared format_value helper) is addressed.

What was reviewed:

• format_value mirrors ZigFormatter::fmt (same Tag::get → format → reset remaining_values sequence) but returns JsResult; remaining_values is cleared on every path since the body uses .and_then rather than ?.
• Both format_label call sites ($path and %p) now ?-propagate to the host fn, and each of the four test/describe × path/%p variants is asserted independently in its own fixture file.
• Vec<u8> implements bun_io::Write so &mut list satisfies the new signature; stack_check.update() only tightens the recursion guard relative to the old path.

Extended reasoning...

Overview

Adds Formatter::format_value in src/jsc/ConsoleObject.rs — a fallible single-value entry point that returns JsResult<()> instead of collapsing a JS exception to fmt::Error — and switches both format_label interpolation sites in src/runtime/test_runner/jest.rs ($path and %p) from write!(&mut list, "{}", value.to_fmt(...)).unwrap() to formatter.format_value::<false>(value, &mut list)?. A new test in test/cli/test/bun-test.test.ts covers test.each/describe.each × $path/%p in four separate fixture files with distinct error messages.

Security risks

None. The change turns a process abort into a caught JS error surfaced to stderr with exit code 1. No new inputs are parsed, no auth/crypto/permissions surface is touched.

Level of scrutiny

Moderate — native runtime code touching the console formatter, but the diff is ~30 production lines and structurally identical to the existing ZigFormatter Display path (same remaining_values seat-and-reset dance, same Tag::get → format sequence). The only behavioral addition is stack_check.update(), which enables the recursion guard where the old to_fmt path left it disabled — a strict improvement. The .and_then chain guarantees remaining_values is reset to EMPTY before return on both success and error, so the stack-local RawSlice never dangles.

Other factors

All prior review feedback has been addressed: my earlier note that the %p fixture was unreachable led to the four-file split with per-variant assertions (2da2ba3); the sibling write!-into-infallible-sink sites (ExpectMatcherUtils::print_value #36912, bun_inspect #30980) are now named as intentionally deferred in the PR description, and the format_value hunk is kept byte-identical to #36912 so the two merge without conflict. The comment-cop bot flags on the doc comment were resolved (it is API guidance, not a workaround justification). The bug hunting system found no issues on the current revision.

[robobun]

Verified a few more %p / $path shapes against this branch (main 1bfcb4e + this diff), since the same outer write! site was reachable through several inner formatter paths. Each of these aborts with panic: a formatting trait implementation returned an error when the underlying stream did not (exit 134) on canary b66764f, and with this change reports the thrown value under "Unhandled error between tests" and exits 1:

• %p with a throwing [Symbol.for("nodejs.util.inspect.custom")], including throw 1 (non-Error)
• %p with a boxed String whose toString throws: top level, nested in an object, and as a Map key
• describe.each(...)("suite %p") with the boxed String
• $m where m is a Map keyed by the boxed String

import { test } from "bun:test";
const s = new String("x"); s.toString = () => { throw new Error("boom"); };
test.each([[new Map([[s, 1]])]])("case %p", () => {});