bun:test: stop building a matcher diff once pretty-printing a value throws

[robobun]

Problem

Fuzzing hit this assertion in a debug build while a failing toStrictEqual was building its error message:

ASSERTION FAILED: Unexpected exception observed on thread ...
Error Exception: Maximum call stack size exceeded.
!exception() || m_vm.hasPendingTerminationException()
JavaScriptCore/ExceptionScope.h(63) : void JSC::ExceptionScope::assertNoExceptionExceptTermination()

The fuzzer's program recursed until the JS stack ran out and then called Bun.jest().expect(x).toStrictEqual(new Uint8ClampedArray()) at the bottom. It is flaky in that form because pretty-printing the fuzzer's x only calls back into JS under leftover state from earlier programs. Any value whose pretty-printing throws reproduces it deterministically, for example a throwing $$typeof getter (the printer reads $$typeof to detect React elements):

const { expect } = Bun.jest();
const o = { a: 1 };
Object.defineProperty(o, "$$typeof", { get() { throw new Error("boom"); } });
expect(o).toStrictEqual({ a: 2 }); // debug build: assertion above; release: throws "boom"

Cause

DiffFormatter::fmt (src/runtime/test_runner/diff_format.rs) pretty-prints received and expected into two buffers and ignored both results (let _ = ...; // TODO:, carried over from the Zig version). When printing received throws, the exception is still pending while expected is printed, and the first JSC call made for it (JSC__JSValue__getOwn for the $$typeof check, ASSERT_NO_PENDING_EXCEPTION) fires the assertion. In release builds it only worked by accident: the second print fails early on the stale exception and throw_value refuses to replace a pending exception, so the getter's error escaped instead of the matcher's.

Fix

The two ? in diff_format.rs are the fix. A failed print now returns fmt::Error from Display, the same thing the ZigFormatter adapter used by the other matchers does, and nothing touches JSC afterwards. create_error_instance already handles that case: it clears the exception and throws the matcher error built from what was written so far, so toEqual / toStrictEqual and the other diff-based matchers now behave like toBe does when printing a value throws (expect(received).toStrictEqual(expected) with an empty body, instead of the getter's error).

ExpectMatcherUtils.matcherHint rendered the same DiffFormatter with format!, which would now panic when the Display impl fails. It writes into the buffer instead and returns the pending exception (or OutOfMemory when the printer failed without one), which keeps its current behavior of surfacing the error thrown while printing; previously it returned a string with the exception still pending, which is the same assertion in debug builds.

utils.stringify / printReceived / printExpected have a related panic that is already fixed separately in #36912, so they are not touched here.

Verification

test/js/bun/test/expect-diff-format-throw.test.ts:

• The first test covers toStrictEqual and toEqual with a throwing received, toStrictEqual with a throwing expected, and matcherHint. On the unfixed build the first two cases abort the debug build with the assertion above, and on release 1.4.0 all four cases report "getter threw" instead of the matcher message, so it fails on both build types without the fix.
• The second test is the fuzzer's shape: the matcher fails at the bottom of the stack, so the getter the printer calls throws a stack overflow. It aborts the unfixed debug build with the exact assertion from the report and passes with the fix.

Also ran expect.test.js, expect-extend.test.js, jest-extended.test.js, mock-fn.test.js, the expect-*-crash tests, expect-label, test-test.test.ts and the snapshot tests against the debug build. pretty-format-overflow.test.ts segfaults its child process on a local debug+ASAN build before and after this change (the jest pretty-printer has no stack check; reported separately), which is unrelated to this fix.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 17 minutes

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: 0ce04ace-7dad-4631-97de-7430c656fdd2

📥 Commits

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

📒 Files selected for processing (3)

• src/runtime/test_runner/diff_format.rs
• src/runtime/test_runner/expect.rs
• test/js/bun/test/expect-diff-format-throw.test.ts

---

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

[robobun]

^{Updated 5:57 AM PT - Aug 11th, 2026}

@robobun, your commit c39183c is building: #92263

[robobun]

CodeRabbit's automatic review got rate limited here rather than skipped on purpose; it should pick the PR up again on its own or on the next push. Nothing in the diff has changed since it was opened, CI is running.

[github-actions]

This PR may be a duplicate of:

1. bun:test: propagate throwing Proxy traps through strict deepEquals and diff rendering #34649 - Makes the same two JestPrettyFormat::format calls in DiffFormatter::fmt return Err(fmt::Error) and rewrites the same ExpectMatcherUtils::matcherHint format! line, bundled with an extra bindings.cpp deepEquals Proxy fix.
2. test: clear pending JS exception when diff formatting throws #29784 - Same root cause and same two DiffFormatter::fmt call sites, but clears the pending exception instead of aborting the diff; adds a near-identically named test file.
3. Clear pending JSC exception in DiffFormatter catch blocks #28538 - Same clear-pending-exception remedy on the same two DiffFormatter::fmt call sites, reached via a throwing RegExp.toString repro.

🤖 Generated with Claude Code

[claude]

LGTM — targeted fix that stops DiffFormatter from continuing into JSC after JestPrettyFormat throws.

What was reviewed:

• Traced every DiffFormatter caller: the string-only sites (snapshots, zig__renderDiff) never reach the changed path; the JSValue sites all go through throw! → create_error_instance, which already clears the pending exception on fmt::Error (JSGlobalObject.rs:744-748).
• matcherHint's format! → writeln! swap avoids the panic the new ? would introduce; the has_exception() → Thrown/OutOfMemory branch matches the pattern used elsewhere in this file.
• Tests follow harness conventions (concurrent subprocess spawn, both pipes drained, combined-object assertion) and cover both the deterministic getter case and the fuzzer's stack-overflow shape.

Extended reasoning...

Overview

Three files: diff_format.rs replaces two let _ = ...; // TODO: with .map_err(|_| fmt::Error)? so the Display impl bails out when pretty-printing throws instead of continuing with a pending JSC exception; expect.rs swaps format! for writeln! in ExpectMatcherUtils::matcherHint so the newly-fallible Display doesn't panic; and a new test file exercises toStrictEqual/toEqual/matcherHint with a throwing $$typeof getter plus the fuzzer's stack-overflow-at-matcher-call shape.

Security risks

None. This is the test runner's failure-message rendering path — no untrusted input parsing, no auth/crypto/filesystem, and the only external effect is which error object escapes to the user's catch.

Level of scrutiny

Low-to-medium. The Rust change is ~6 lines and mechanical (propagate an error that was explicitly TODO'd). The one non-obvious risk — that other DiffFormatter consumers might now panic on the new fmt::Error — was checked: the snapshot/C++-bridge callers use received_string/expected_string and return before the changed code, and every JSValue caller (toEqual, toStrictEqual, toMatchObject, toHaveProperty, toHaveBeen*CalledWith, toHave*ReturnedWith, toBe string branch) routes through throw_rendered → JSGlobalObject::throw → create_error_instance, whose write!().is_err() branch already clears the exception and returns the partial buffer. matcherHint was the only format! caller and is fixed in this PR.

Other factors

The PR description is unusually thorough (root cause, why release builds accidentally worked, why the fuzzer repro was flaky, full list of related test files re-run). Tests use test.concurrent, drain stdout/stderr/exited together, and assert exact strings — the first test's assertions distinguish the fixed behavior (matcher header with empty body) from the unfixed release behavior ("getter threw"), so it fails on both build types without the fix as claimed. No CODEOWNERS entry covers src/runtime/test_runner/. No prior reviewer comments to address.

[robobun]

Closing this one. The duplicate finder is right: the two JestPrettyFormat::format results in DiffFormatter::fmt are already covered by open PRs, and #34649 already asked for a maintainer to pick between them, so a fourth variant of the same two lines does not help.

For whoever lands this, how the open PRs differ:

• Clear pending JSC exception in DiffFormatter catch blocks #28538 and test: clear pending JS exception when diff formatting throws #29784 clear the pending exception inside DiffFormatter and keep rendering the diff.
• bun:test: propagate throwing Proxy traps through strict deepEquals and diff rendering #34649 returns fmt::Error from DiffFormatter (what this PR did too) and rewrites matcherHint, which otherwise panics in format! once Display can fail. It also changes create_error_instance to rethrow the user's error instead of clearing it, and bundles a deepEquals Proxy change.

Two things from this PR that may be useful there: the same assertion keeps coming out of fuzzing (it shows up whenever the matcher fails at the bottom of the JS stack, because the getter the pretty-printer calls for $$typeof then throws a stack overflow), and a non-enumerable throwing $$typeof getter is a convenient repro since deepEquals skips it and only the message rendering hits it:

const { expect } = Bun.jest();
const o = { a: 1 };
Object.defineProperty(o, "$$typeof", { get() { throw new Error("boom"); } });
expect(o).toStrictEqual({ a: 2 }); // debug build: assertNoExceptionExceptTermination in JSC__JSValue__getOwn