From 7d273c056b25a8ad090087d05f0da653afa92fc7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:20:36 +0000 Subject: [PATCH] bun:test: propagate formatter errors out of DiffFormatter DiffFormatter::fmt dropped the JsResult of both JestPrettyFormat::format calls. When the received value threw while it was formatted (for example a RegExp whose toString returns a Symbol), the expected value was formatted with that exception still pending. JSC's exception scope assertion then aborted the process in debug builds. Map the JsError to fmt::Error, like the other Display adapters in the test runner. The matcher then throws its own error with the partial message. matcherHint built its result with format!, which panics on fmt::Error. It now writes into a buffer and returns the pending exception instead. --- src/runtime/test_runner/diff_format.rs | 12 +++-- src/runtime/test_runner/expect.rs | 12 ++++- .../expect-symbol-toPrimitive-crash.test.ts | 53 +++++++++++++++++++ 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/runtime/test_runner/diff_format.rs b/src/runtime/test_runner/diff_format.rs index 46bc4f2be083..43cf4cde6567 100644 --- a/src/runtime/test_runner/diff_format.rs +++ b/src/runtime/test_runner/diff_format.rs @@ -1,7 +1,7 @@ use core::fmt; use bun_core::Output; -use bun_jsc::{JSGlobalObject, JSValue}; +use bun_jsc::{JSGlobalObject, JSValue, js_error_to_write_error}; use super::diff::print_diff::{print_diff_main, DiffConfig}; use super::pretty_format::{FormatOptions, JestPrettyFormat, MessageLevel}; @@ -44,23 +44,25 @@ impl<'a> fmt::Display for DiffFormatter<'a> { flush: false, quote_strings: true, }; - let _ = JestPrettyFormat::format( + JestPrettyFormat::format( MessageLevel::Debug, global_this, core::slice::from_ref(&received), 1, &mut received_buf, fmt_options, - ); // TODO: + ) + .map_err(js_error_to_write_error)?; - let _ = JestPrettyFormat::format( + JestPrettyFormat::format( MessageLevel::Debug, global_this, core::slice::from_ref(&expected), 1, &mut expected_buf, fmt_options, - ); // TODO: + ) + .map_err(js_error_to_write_error)?; } let mut received_slice: &[u8] = received_buf.as_slice(); diff --git a/src/runtime/test_runner/expect.rs b/src/runtime/test_runner/expect.rs index 3daeb9471ffe..c1be1ac91732 100644 --- a/src/runtime/test_runner/expect.rs +++ b/src/runtime/test_runner/expect.rs @@ -2872,7 +2872,17 @@ impl ExpectMatcherUtils { } else { bun_core::pretty_fmt!("(expected)", false) }; - let buf = format!("{head}{not}{matcher_name}{expected_hint}\n\n{diff_formatter}\n"); + use fmt::Write as _; + let mut buf = String::new(); + if write!( + buf, + "{head}{not}{matcher_name}{expected_hint}\n\n{diff_formatter}\n" + ) + .is_err() + && global_this.has_exception() + { + return Err(JsError::Thrown); + } bun_string_jsc::create_utf8_for_js(global_this, buf.as_bytes()) } } diff --git a/test/js/bun/test/expect-symbol-toPrimitive-crash.test.ts b/test/js/bun/test/expect-symbol-toPrimitive-crash.test.ts index 3e75bd522770..775cc16bf7d2 100644 --- a/test/js/bun/test/expect-symbol-toPrimitive-crash.test.ts +++ b/test/js/bun/test/expect-symbol-toPrimitive-crash.test.ts @@ -19,3 +19,56 @@ test("expect does not crash when value has Symbol.toPrimitive returning a Symbol expect(exitCode).toBe(0); }); + +// The diff printed by a failed toEqual/toStrictEqual formats both values. When +// the first one throws, the second must not be formatted with that exception +// still pending. The matcher throws its own error with the partial message. +// matcherHint returns a string to user code, so it surfaces the exception. +test("expect does not crash when value.toString() returns a Symbol while printing a diff", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { expect } from "bun:test"; + const re = /u/i; + re.toString = Symbol; + const results = {}; + const record = (name, fn) => { + try { + fn(); + results[name] = "did not throw"; + } catch (e) { + results[name] = e.constructor.name + ": " + e.message.split("\\n")[0]; + } + }; + record("toStrictEqual", () => expect(re).toStrictEqual({})); + record("toEqual", () => expect(re).toEqual(/x/i)); + record("toMatchObject", () => expect({ a: re }).toMatchObject({ a: 1 })); + expect.extend({ + _hint(received) { + record("matcherHint", () => this.utils.matcherHint("_hint", received, 1)); + return { pass: true, message: () => "" }; + }, + }); + expect(re)._hint(); + console.log(JSON.stringify(results)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ + toStrictEqual: "Error: expect(received).toStrictEqual(expected)", + toEqual: "Error: expect(received).toEqual(expected)", + toMatchObject: "Error: expect(received).toMatchObject(expected)", + matcherHint: "TypeError: Cannot convert a symbol to a string", + }), + stderr: "", + exitCode: 0, + }); +});