Skip to content

Bun__inspect: don't panic when user JS throws during error-message formatting #30980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Bun__inspect: don't panic when user JS throws during error-message formatting #30980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/runtime/api/BunObject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,14 @@ pub fn bun_inspect(global_this: &JSGlobalObject, value: JSValue) -> BunString {
let mut array: Vec<u8> = Vec::new();

let mut formatter = ConsoleObject::Formatter::new(global_this);
if write!(&mut array, "{}", value.to_fmt(&mut formatter)).is_err() {
use core::fmt::Write;
if write!(
bun_core::fmt::VecWriter(&mut array),
"{}",
value.to_fmt(&mut formatter)
)
.is_err()
{
Comment thread
claude[bot] marked this conversation as resolved.
Comment on lines +724 to +730

@claude claude Bot May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 🟣 Heads up (pre-existing): there is one more instance of this same std::io::Write::write_fmt panic in src/runtime/test_runner/expect.rs ExpectMatcherUtils::print_value (expect.rs:2866). writer is a &mut MutableString whose std::io::Write::write() is infallible (MutableString.rs:476-480) and only std::io::Write is in scope (expect.rs:2850), so let _ = write!(writer, "{}", value.to_fmt(&mut formatter)); panics inside write_fmt when ZigFormatter's Display returns fmt::Error — reachable from user JS via this.utils.stringify/printExpected/printReceived in expect.extend matchers. Same one-line core::fmt::Write / VecWriter fix; might be worth folding in alongside the jest.rs site so the Fuzzilli fingerprint is fully eliminated.

Extended reasoning...

Summary

This is a third site (in addition to bun_inspect fixed by this PR and jest.rs format_label already flagged in the earlier comment) with the identical std::io::Write::write_fmt-on-infallible-sink panic. It lives in src/runtime/test_runner/expect.rs inside ExpectMatcherUtils::print_value, the helper that backs this.utils.stringify, this.utils.printExpected, and this.utils.printReceived for custom Jest matchers.

This is pre-existing: the PR does not touch expect.rs, add callers, or change its behavior. Flagging it non-blocking because it is the same root cause / Fuzzilli fingerprint the PR is closing out, and the previous comment only listed the jest.rs sites.

Code path

  • expect.rs:2850 — use std::io::Write as _; (no core::fmt::Write anywhere in the file).
  • expect.rs:2856 — let writer = mutable_string.writer(); where MutableString::writer returns &mut Self (MutableString.rs:58-60), i.e. &mut MutableString.
  • expect.rs:2866 — let _ = write!(writer, "{}", value.to_fmt(&mut formatter)); with a ConsoleObject::Formatter.
  • MutableString implements std::io::Write (MutableString.rs:476-480) with an infallible write() that just extend_from_slices into a Vec<u8> and returns Ok(buf.len()). There is no core::fmt::Write impl for MutableString in the repo.
  • value.to_fmt(&mut formatter) returns a ZigFormatter, whose Display::fmt (ConsoleObject.rs:1922-1953) maps both Tag::get failures and formatter.format failures to Err(core::fmt::Error) via .map_err(|_| core::fmt::Error)? — i.e. it returns fmt::Error when user JS throws during inspection.

Because only std::io::Write is in scope and MutableString only implements std::io::Write, write!(writer, …) resolves to <MutableString as std::io::Write>::write_fmt. Exactly as described in this PR for bun_inspect, std's default_write_fmt panics with "a formatting trait implementation returned an error when the underlying stream did not" when the Display impl errors but the underlying MutableString sink never set an I/O error.

Why nothing prevents it

The let _ = discards a Result it never receives — the panic happens inside write_fmt before it returns. The wrapping print_value_catched (expect.rs:2880-2887) only handles JsResult errors via unwrap_or_else; it cannot catch a Rust panic. The asymmetric-matcher self-print path (expect.rs:2784) hits the same helper.

Step-by-step proof

import { expect, test } from "bun:test";
expect.extend({
  myMatcher(received) {
    return { pass: false, message: () => this.utils.printReceived(received) };
  },
});
const bad = { [Symbol.for("nodejs.util.inspect.custom")]() { throw new Error("boom"); } };
test("x", () => expect(bad).myMatcher());
  1. The matcher fails and Jest evaluates message(), which calls this.utils.printReceived(bad) → Expect_print_received (expect.rs:2905-2911) → print_value_catched → print_value.
  2. At expect.rs:2866, bad.to_fmt(&mut formatter) constructs a ZigFormatter.
  3. write!(writer, "{}", <ZigFormatter>) resolves to <MutableString as std::io::Write>::write_fmt (only std::io::Write is in scope; MutableString has no core::fmt::Write impl).
  4. Inside write_fmt, <ZigFormatter as Display>::fmt runs the console formatter, which invokes bad's inspect.custom. That throws; the formatter maps the JS exception to core::fmt::Error and Display::fmt returns Err(fmt::Error).
  5. std::io::default_write_fmt sees a formatting error with no underlying I/O error (the MutableString sink's write() never fails) and panics with "a formatting trait implementation returned an error when the underlying stream did not".
  6. The let _ = is never reached; print_value_catched's unwrap_or_else never runs; the bun test process crashes.

Impact

A user-supplied object with a throwing custom inspect, passed to a custom matcher that uses this.utils.stringify / printExpected / printReceived (the standard way to build matcher messages), crashes the entire bun test process instead of failing the test. Same crash class and Fuzzilli fingerprint (panic:a formatting trait implementation returned an error when the) that this PR is closing out.

Suggested fix

Same as bun_inspect: bring core::fmt::Write into scope and write through a core::fmt::Write sink (e.g. bun_core::fmt::VecWriter(&mut mutable_string.list), or add a core::fmt::Write impl for MutableString) so the error surfaces as a Result. Then the existing let _ = / print_value_catched fallback degrades gracefully (or you can render a placeholder) instead of panicking.

return BunString::empty();
}
BunString::clone_utf8(&array)
Expand Down
31 changes: 31 additions & 0 deletions test/js/bun/cookie/cookie-expires-validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

describe("Bun.Cookie expires validation", () => {
describe("Date objects", () => {
Expand Down Expand Up @@ -76,6 +77,36 @@ describe("Bun.Cookie expires validation", () => {
}).toThrow();
});

test("throws for objects whose inspect.custom throws", async () => {
// Bun__inspect renders the received value into the error message; a
// throwing inspect.custom must surface as a JS error, not a crash.
// Spawn a subprocess so a regression fails cleanly instead of aborting.
const src = `
const obj = {
[Symbol.for("nodejs.util.inspect.custom")]() {
throw new Error("boom from inspect.custom");
},
};
try {
new Bun.Cookie("name", "value", { expires: obj });
console.log("no-throw");
} catch (e) {
console.log("threw:" + e.message);
}
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
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 }).toMatchObject({
stdout: "threw:boom from inspect.custom",
exitCode: 0,
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

test("invalid strings throw", () => {
expect(() => {
new Bun.Cookie("name", "value", { expires: "tomorrow" });
Expand Down
Loading