From c39183ce1bb390f27f751a8575266cb28a1fd3de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:52:27 +0000 Subject: [PATCH] bun:test: stop building a matcher diff once pretty-printing a value throws DiffFormatter ignored the result of pretty-printing received and expected. When printing the first value threw, the exception stayed pending while the second value was printed, which trips the pending-exception assertions in debug builds. Propagate the failure as fmt::Error instead, so the matcher error is built from what was written so far, like the other matchers. matcherHint rendered the diff with format!, which panics when a Display impl fails; write it into the buffer and return the pending exception instead. --- src/runtime/test_runner/diff_format.rs | 10 ++- src/runtime/test_runner/expect.rs | 6 +- .../bun/test/expect-diff-format-throw.test.ts | 90 +++++++++++++++++++ 3 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 test/js/bun/test/expect-diff-format-throw.test.ts diff --git a/src/runtime/test_runner/diff_format.rs b/src/runtime/test_runner/diff_format.rs index 46bc4f2be083..8b727304a45b 100644 --- a/src/runtime/test_runner/diff_format.rs +++ b/src/runtime/test_runner/diff_format.rs @@ -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(|_| fmt::Error)?; - let _ = JestPrettyFormat::format( + JestPrettyFormat::format( MessageLevel::Debug, global_this, core::slice::from_ref(&expected), 1, &mut expected_buf, fmt_options, - ); // TODO: + ) + .map_err(|_| fmt::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 cf95b89cca37..d5729dff8fc2 100644 --- a/src/runtime/test_runner/expect.rs +++ b/src/runtime/test_runner/expect.rs @@ -2928,7 +2928,11 @@ impl ExpectMatcherUtils { } else { bun_core::pretty_fmt!("(expected)", false) }; - let buf = format!("{head}{not}{matcher_name}{expected_hint}\n\n{diff_formatter}\n"); + let mut buf = format!("{head}{not}{matcher_name}{expected_hint}\n\n"); + use fmt::Write as _; + if writeln!(buf, "{diff_formatter}").is_err() { + return Err(if global_this.has_exception() { JsError::Thrown } else { JsError::OutOfMemory }); + } bun_jsc::bun_string_jsc::create_utf8_for_js(global_this, buf.as_bytes()) } } diff --git a/test/js/bun/test/expect-diff-format-throw.test.ts b/test/js/bun/test/expect-diff-format-throw.test.ts new file mode 100644 index 000000000000..467cc9180777 --- /dev/null +++ b/test/js/bun/test/expect-diff-format-throw.test.ts @@ -0,0 +1,90 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// The failure message of toEqual/toStrictEqual (and matcherHint) is a diff of the pretty-printed +// values. When pretty-printing one of them throws, the exception must not be left pending while +// the other value is formatted. +// +// The pretty-printer reads `$$typeof` to detect React elements, and deepEquals ignores +// non-enumerable properties, so a non-enumerable throwing `$$typeof` getter only fires while the +// failure message is being built. +test.concurrent("matcher failure message when pretty-printing a value throws", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { expect } = Bun.jest(); + function value(a) { + const o = { a }; + Object.defineProperty(o, "$$typeof", { get() { throw new Error("getter threw"); } }); + return o; + } + function message(fn) { + try { fn(); } catch (e) { return e.message; } + return "did not throw"; + } + let utils; + expect.extend({ captureUtils() { utils = this.utils; return { pass: true, message: () => "" }; } }); + expect(0).captureUtils(); + console.log(JSON.stringify([ + message(() => expect(value(1)).toStrictEqual({ a: 2 })), + message(() => expect(value(1)).toEqual({ a: 2 })), + message(() => expect({ a: 1 }).toStrictEqual(value(2))), + message(() => utils.matcherHint("toFoo", value(1), { a: 2 })), + ])); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: + JSON.stringify([ + "expect(received).toStrictEqual(expected)\n\n", + "expect(received).toEqual(expected)\n\n", + "expect(received).toStrictEqual(expected)\n\n", + "getter threw", + ]) + "\n", + stderr: "", + exitCode: 0, + }); +}); + +// Same bug as found by the fuzzer: the matcher fails right where the JS stack ran out, so the +// getter the pretty-printer calls while formatting `received` throws a stack overflow, and +// formatting `expected` then ran with that exception still pending. +test.concurrent("matcher failure message when pretty-printing overflows the stack", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { expect } = Bun.jest(); + const received = { get $$typeof() { return undefined; } }; + const expected = new Uint8ClampedArray(); + let ran = false; + function recurse() { + try { recurse(); } catch {} + if (!ran) { + ran = true; + try { expect(received).toStrictEqual(expected); } catch {} + } + } + recurse(); + console.log("OK"); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "OK\n", stderr: "", exitCode: 0 }); +});