Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
94 changes: 68 additions & 26 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4469,6 +4469,7 @@ impl VirtualMachine {
allow_side_effects: bool,
) {
let mut formatter = crate::console_object::Formatter::new(self.global());
formatter.stack_check = bun_core::util::StackCheck::init();
let colors = bun_core::Output::enable_ansi_colors_stderr();
self.print_errorlike_object(
exception.value(),
Expand Down Expand Up @@ -4784,7 +4785,54 @@ impl VirtualMachine {
// once the AggregateError branch is taken).
let global_ref = self.global();

if value.is_aggregate_error(global_ref) {
let is_aggregate = value.is_aggregate_error(global_ref);
if is_aggregate {
use crate::console_object::formatter::visited;

if !formatter.stack_check.is_safe_to_recurse() {
let marker = if allow_ansi_color {
bun_core::pretty_fmt!("<r><cyan>[AggregateError: nesting too deep]<r>\n", true)
} else {
bun_core::pretty_fmt!("<r><cyan>[AggregateError: nesting too deep]<r>\n", false)
};
let _ = writer.write_all(marker.as_bytes());
return;
}

if formatter.map_node.is_none() {
let mut node = NonNull::new(visited::Pool::get_node())
.expect("ObjectPool::get_node always returns a valid heap node");
let data = visited::node_data_mut(&mut node);
data.clear();
formatter.map = core::mem::take(data);
formatter.map_node = Some(node);
}
let entry = formatter.map.get_or_put(value).expect("unreachable");
if entry.found_existing {
let marker = if allow_ansi_color {
bun_core::pretty_fmt!("<r><cyan>[Circular]<r>\n", true)
} else {
bun_core::pretty_fmt!("<r><cyan>[Circular]<r>\n", false)
};
let _ = writer.write_all(marker.as_bytes());
return;
}
// Fall through: print this AggregateError's own header before its children.
}

// Note: reborrow so the add-to-error-list tail can still see it after
// `print_error_from_maybe_private_data`.
Comment thread
robobun marked this conversation as resolved.
let mut exception_list = exception_list;
let was_internal = self.print_error_from_maybe_private_data(
value,
exception_list.as_deref_mut(),
formatter,
writer,
allow_ansi_color,
allow_side_effects,
);

if is_aggregate {
// Note: `JSValue::for_each` takes a C-ABI fn
// pointer + erased ctx, so thread the captures through a struct.
// The C trampoline erases lifetimes via `*mut c_void`; round-trip
Expand Down Expand Up @@ -4830,35 +4878,29 @@ impl VirtualMachine {
ctx.allow_side_effects,
);
}
let mut ctx = AggCtx {
formatter: std::ptr::from_mut(formatter),
writer: std::ptr::from_mut(writer),
exception_list: exception_list
.map(std::ptr::from_mut::<ExceptionList>)
.unwrap_or(core::ptr::null_mut()),
allow_ansi_color,
allow_side_effects,
};
// `getErrorsProperty` is
// `getDirect` (own data prop, nothrow); `for_each` may throw, in
// which case the error is swallowed.
// `getDirect`: empty / GetterSetter when `.errors` is deleted or an accessor.
let errors = value.get_errors_property(global_ref);
let _ = errors.for_each(global_ref, (&raw mut ctx).cast(), agg_iter);
if errors.is_object() {
let mut ctx = AggCtx {
formatter: std::ptr::from_mut(formatter),
writer: std::ptr::from_mut(writer),
exception_list: exception_list
.map(std::ptr::from_mut::<ExceptionList>)
.unwrap_or(core::ptr::null_mut()),
allow_ansi_color,
allow_side_effects,
};
if errors
.for_each(global_ref, (&raw mut ctx).cast(), agg_iter)
.is_err()
{
self.global().clear_exception();
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}
let _ = formatter.map.remove(&value);
return;
}

// Note: reborrow so the add-to-error-list tail can still see it after
// `print_error_from_maybe_private_data`.
let mut exception_list = exception_list;
let was_internal = self.print_error_from_maybe_private_data(
value,
exception_list.as_deref_mut(),
formatter,
writer,
allow_ansi_color,
allow_side_effects,
);

if was_internal {
if let Some(exception_) = exception {
let mut holder = crate::zig_exception::Holder::init();
Expand Down
1 change: 1 addition & 0 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1194,6 +1194,7 @@ fn print_exception(
vm_ref.print_exception(exception, exception_list, writer, true);
} else {
let mut formatter = bun_jsc::console_object::Formatter::new(global);
formatter.stack_check = bun_core::util::StackCheck::init();
// `Formatter::new` already
// defaults `error_display_level` to `Full` (ConsoleObject.rs:1176).
let colors = bun_core::Output::enable_ansi_colors_stderr();
Expand Down
189 changes: 189 additions & 0 deletions test/js/bun/util/inspect-error-cycle.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
// Error-graph cycle / deep-chain segfaults in the native error printer.
// The AggregateError `errors` recursion had no stack check and no visited
// set, so self/mutual cycles and very deep nesting hit the stack guard page
// (silent SIGSEGV) via `print_errorlike_object` -> `for_each` -> `agg_iter`.
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

type Shape = { name: string; build: string };
type Sink = { name: string; wrap: (b: string) => string; allowFail: boolean };

const shapes: Shape[] = [
{
name: "self-cycle",
build: `const ae = new AggregateError([], "self"); ae.errors = [ae]; const e = ae;`,
},
{
name: "mutual-cycle",
build: `const a = new AggregateError([], "A"); const b = new AggregateError([], "B"); a.errors = [b]; b.errors = [a]; const e = a;`,
},
{
name: "deleted-errors",
build: `const ae = new AggregateError([new Error("x")], "del"); delete ae.errors; const e = ae;`,
},
{
name: "accessor-errors",
build: `const ae = new AggregateError([], "acc"); Object.defineProperty(ae, "errors", { get() { throw new Error("boom"); } }); const e = ae;`,
},
{
name: "non-iterable-errors",
build: `const ae = new AggregateError([], "ni"); ae.errors = 42; const e = ae;`,
},
{
name: "mixed-agg-cause",
build: `const a = new AggregateError([], "A"); const c = new Error("C"); a.errors = [c]; c.cause = a; const e = a;`,
},
];

const sinks: Sink[] = [
{ name: "console.log", wrap: b => `${b} console.log(e);`, allowFail: false },
{ name: "console.error", wrap: b => `${b} console.error(e);`, allowFail: false },
{ name: "Bun.inspect", wrap: b => `${b} Bun.inspect(e);`, allowFail: false },
{ name: "uncaught-throw", wrap: b => `${b} throw e;`, allowFail: true },
{
name: "unhandled-reject",
wrap: b => `${b} Promise.reject(e); await 1;`,
allowFail: true,
},
{
name: "uncaughtException-handler",
wrap: b => `process.on("uncaughtException", err => { console.error(err); process.exit(0); }); ${b} throw e;`,
allowFail: false,
},
];

describe.concurrent("error-graph cycles do not crash the printer", () => {
for (const shape of shapes) {
for (const sink of sinks) {
const cell = `${shape.name} x ${sink.name}`;
test(cell, async () => {
const code = sink.wrap(shape.build);
await using proc = Bun.spawn({
cmd: [bunExe(), "--no-install", "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
if (proc.signalCode) {
throw new Error(
`crashed with ${proc.signalCode}\nstdout: ${stdout.slice(0, 300)}\nstderr: ${stderr.slice(0, 300)}`,
);
}
if (sink.allowFail) {
expect(exitCode).toBeLessThan(128);
} else {
if (exitCode !== 0) {
throw new Error(`exit ${exitCode}\nstdout: ${stdout.slice(0, 300)}\nstderr: ${stderr.slice(0, 300)}`);
}
expect(exitCode).toBe(0);
}
});
}
}
});

// Depth tests kept out of the concurrent matrix: each prints hundreds of
// headers before the stack guard fires, which is seconds under debug+ASAN.

Check warning on line 87 in test/js/bun/util/inspect-error-cycle.test.ts

View check run for this annotation

Claude / Claude Code Review

Depth-test describe block runs serially instead of concurrently

The depth-test block uses plain `describe` while its 8 tests are independent subprocess spawns that each take "seconds under debug+ASAN" — that's an argument for `describe.concurrent`, not against it (the sibling blocks at L55 and L125 already are, and L144's "deep nesting renders depth marker" does the identical 3000-level work inside a concurrent block). If the intent is to avoid 8 simultaneous deep-recursion ASAN processes, the comment should say that; otherwise this should be `describe.concu
Comment thread
robobun marked this conversation as resolved.
Outdated
// Release bun segfaults at ~2000 levels.
describe("error-graph depth does not crash the printer", () => {
const deepAgg = `let x = new AggregateError([], "leaf"); for (let i = 0; i < 3000; i++) x = new AggregateError([x], "n" + i); const e = x;`;
const deepCause = `let x = new Error("leaf"); for (let i = 0; i < 3000; i++) x = new Error("n" + i, { cause: x }); const e = x;`;

for (const sink of sinks) {
test(`deep-aggregate x ${sink.name}`, async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "--no-install", "-e", sink.wrap(deepAgg)],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(proc.signalCode).toBeFalsy();
if (sink.allowFail) expect(exitCode).toBeLessThan(128);
else expect(exitCode).toBe(0);
});
}

// The cause-chain depth guard exists but was inert on the uncaught /
// rejection path because the formatter's stack_check was never seated.
for (const sink of sinks.filter(s => s.allowFail)) {
test(`deep-cause x ${sink.name}`, async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "--no-install", "-e", sink.wrap(deepCause)],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(proc.signalCode).toBeFalsy();
expect(exitCode).toBeLessThan(128);
});
}
});

describe.concurrent("AggregateError printer output", () => {
test("self-cycle renders [Circular] and includes the header", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const ae = new AggregateError([], "outer"); ae.errors = [ae]; process.stdout.write(Bun.inspect(ae));`,
],
env: { ...bunEnv, NO_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toContain("[Circular]");
expect(stdout).toContain("outer");
expect(exitCode).toBe(0);
});

test("deep nesting renders depth marker", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`let x = new AggregateError([], "leaf"); for (let i = 0; i < 3000; i++) x = new AggregateError([x], ""); process.stdout.write(Bun.inspect(x));`,
],
env: { ...bunEnv, NO_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toContain("[AggregateError: nesting too deep]");
expect(exitCode).toBe(0);
});

test("uncaught AggregateError prints its own message", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `throw new AggregateError([new Error("inner")], "outer message");`],
env: { ...bunEnv, NO_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("outer message");
expect(stderr).toContain("inner");
expect(exitCode).toBe(1);
});

test("deleted errors property prints header", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const ae = new AggregateError([new Error("x")], "msg"); delete ae.errors; process.stdout.write(Bun.inspect(ae));`,
],
env: { ...bunEnv, NO_COLOR: "1" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toContain("msg");
expect(exitCode).toBe(0);
});
});
Loading