From f2c583d456fc02a3d527c65435c0e3760395747d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 27 Sep 2026 18:35:45 +0000 Subject: [PATCH 1/2] error printer: one render per error, [Circular] under the key that closes the cycle An Error that reaches itself through two of its own properties crashed the process when it was printed: const e = new Error('cyc'); e.cause = e; e.errors = [e]; throw e; The set of values that are being rendered lived with two of the callers that start the render of an error. The console formatter recorded an error and removed it again before it called the printer. The loop that prints the cause recorded the next error. The uncaught entries recorded nothing. Every render of an error passes print_error_instance_js. It now writes [Circular] for an error that a caller further up is rendering. The body records the error before the first render it nests, and print_error_instance_js removes the record when the body returns. An error that nests no render is never recorded. The console formatter no longer records errors. It keeps its stack check for them. An Error-valued own property that closes a cycle is printed in place, under its key. That includes a cause that is not enumerable. A member of an AggregateError that is being rendered prints [Circular] on its own line. The walk over the members is not changed in any other way. --- src/jsc/ConsoleObject.rs | 38 ++--- src/jsc/VirtualMachine.rs | 82 ++++++++-- test/js/bun/util/inspect-error.test.js | 147 ++++++++++++++++++ .../worker_threads/worker_threads.test.ts | 28 ++++ 4 files changed, 256 insertions(+), 39 deletions(-) diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index a7aed777cf33..c73ba392a657 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -1910,6 +1910,10 @@ pub mod formatter { | Tag::Event ) } + + pub(crate) fn is_recorded_by_its_printer(self) -> bool { + matches!(self, Tag::Error) + } } /// Only `CustomFormattedObject` carries a payload. @@ -3226,6 +3230,11 @@ pub mod formatter { let _ = self.map.remove(&value); } + #[inline] + pub(crate) fn visited_contains(&self, value: JSValue) -> bool { + self.map_node.is_some() && self.map.contains(&value) + } + /// Circular-reference / stack-overflow / visited-map prelude for /// `print_as`. Outlined so its locals (the pool node, the /// `get_or_put` result, the `[Circular]` write path) live in a leaf @@ -3240,7 +3249,7 @@ pub mod formatter { &mut self, writer_: &mut dyn bun_io::Write, value: JSValue, - can_circ: bool, + format: Tag, remove_before_recurse: &mut bool, ) -> JsResult { if self.failed { @@ -3249,7 +3258,7 @@ pub mod formatter { if self.global_this.has_exception() { return Err(jsc::JsError::Thrown); } - if !can_circ { + if !format.can_have_circular_references() { return Ok(true); } @@ -3261,6 +3270,10 @@ pub mod formatter { return Ok(false); } + if format.is_recorded_by_its_printer() { + return Ok(true); + } + if !self.visited_insert(value) { if writer_ .write_all(pfmt!("[Circular]", C).as_bytes()) @@ -3289,7 +3302,7 @@ pub mod formatter { if !self.print_as_prelude::( writer_, value, - format.can_have_circular_references(), + format, &mut remove_before_recurse, )? { return Ok(()); @@ -3861,25 +3874,6 @@ pub mod formatter { writer_: &mut dyn bun_io::Write, value: JSValue, ) -> JsResult<()> { - // Temporarily remove from the visited map to allow - // printErrorlikeObject to process it. The circular reference - // check is already done in print_as, so we know it's safe. - let was_in_map = if self.map_node.is_some() { - self.map.remove(&value).is_some() - } else { - false - }; - let map_restore_ptr: *mut visited::Map = &raw mut self.map; - scopeguard::defer! { - // SAFETY: `self.map` outlives this guard; no other borrow is - // live at the drop point. - unsafe { - if was_in_map { - let _ = (*map_restore_ptr).insert(value, ()); - } - } - } - let mut adapter = DynWriteAdapter::new(&mut *writer_); // SAFETY: per-thread VM. let vm = VirtualMachine::get().as_mut(); diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 058c9773a0a3..9491f1df32e9 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -5944,7 +5944,20 @@ impl VirtualMachine { let writer = unsafe { &mut *ctx.writer }; ctx.printed_member = true; formatter.depth = formatter.depth.saturating_add(1); - if formatter.depth > formatter.error_chain_max_depth() + if next_value.is_cell() + && next_value.js_type() == crate::JSType::ErrorInstance + && formatter.visited_contains(next_value) + { + let _ = if ctx.allow_ansi_color { + writer.write_all( + bun_core::pretty_fmt!("[Circular]\n", true).as_bytes(), + ) + } else { + writer.write_all( + bun_core::pretty_fmt!("[Circular]\n", false).as_bytes(), + ) + }; + } else if formatter.depth > formatter.error_chain_max_depth() || !formatter.stack_check.is_safe_to_recurse() { let _ = if ctx.allow_ansi_color { @@ -6611,6 +6624,7 @@ impl VirtualMachine { ) -> crate::CrateResult<()> { let mut default_formatter = crate::console_object::Formatter::new(self.global()); let f = formatter.unwrap_or(&mut default_formatter); + let mut recorded = false; self.print_error_instance_body( zig_exception, JSValue::ZERO, @@ -6619,6 +6633,7 @@ impl VirtualMachine { writer, allow_ansi_color, allow_side_effects, + &mut recorded, ) // `defer default_formatter.deinit()` → Drop. } @@ -6667,6 +6682,18 @@ impl VirtualMachine { return Ok(()); } + if error_instance.is_cell() + && error_instance.js_type() == crate::JSType::ErrorInstance + && formatter.visited_contains(error_instance) + { + writer.write_all(if allow_ansi_color { + bun_core::pretty_fmt!("[Circular]", true).as_bytes() + } else { + bun_core::pretty_fmt!("[Circular]", false).as_bytes() + })?; + return Ok(()); + } + // Note: `Holder` is ~4 KB (32 ZigStackFrames + 6 source lines + // ZigException). It sits next to the large runtime-dispatched body, so // box it to keep the per-level recursion frame small enough for the @@ -6690,6 +6717,7 @@ impl VirtualMachine { ); error_instance.ensure_still_alive(); + let mut recorded = false; let result = self.print_error_instance_body( // SAFETY: see above. unsafe { &mut *exception }, @@ -6700,7 +6728,11 @@ impl VirtualMachine { writer, allow_ansi_color, allow_side_effects, + &mut recorded, ); + if recorded { + formatter.visited_remove(error_instance); + } drop(source_code_slice); exception_holder.deinit(self); @@ -6722,6 +6754,7 @@ impl VirtualMachine { writer: &mut bun_core::io::Writer, allow_ansi_color: bool, allow_side_effects: bool, + recorded: &mut bool, ) -> crate::CrateResult<()> { use crate::JSType; use crate::console_object::formatter::TagOptions; @@ -7065,7 +7098,9 @@ impl VirtualMachine { } let kind = value.js_type(); - if kind == JSType::ErrorInstance && !prev_had_errors { + let circular = kind == JSType::ErrorInstance + && (value == error_instance || formatter.visited_contains(value)); + if kind == JSType::ErrorInstance && !prev_had_errors && !circular { if field.eq_ascii(b"cause") { saw_cause = true; } @@ -7076,6 +7111,12 @@ impl VirtualMachine { || value.is_primitive() || kind.is_string_like() { + if circular && field.eq_ascii(b"cause") { + saw_cause = true; + } + if !*recorded && !value.is_primitive() { + *recorded = formatter.visited_insert(error_instance); + } let prev_disable_inspect_custom = formatter.disable_inspect_custom; let prev_quote_strings = formatter.quote_strings; let prev_max_depth = formatter.max_depth; @@ -7159,20 +7200,30 @@ impl VirtualMachine { )?; } - if !is_first_property { - writer.write_all(b"\n")?; - } - // "cause" is not enumerable, so the above loop won't see it. if !saw_cause { let key = bun_core::String::static_("cause"); if let Some(cause) = error_instance.get_own(global_ref, &key)? { if cause.is_cell() && cause.js_type() == JSType::ErrorInstance { - cause.protect(); - errors_to_append.push(cause); + if cause == error_instance || formatter.visited_contains(cause) { + let pad_left = longest_name.saturating_sub(b"cause".len()); + is_first_property = false; + splat_space(writer, pad_left as u64)?; + pretty_write!( + writer, + " cause: [Circular],\n" + )?; + } else { + cause.protect(); + errors_to_append.push(cause); + } } } } + + if !is_first_property { + writer.write_all(b"\n")?; + } } else if error_instance != JSValue::ZERO { // If you do `reportError([1,2,3])` we should still show something. let tag = Tag::get_advanced( @@ -7200,19 +7251,17 @@ impl VirtualMachine { )?; } + if !*recorded && !errors_to_append.is_empty() { + *recorded = formatter.visited_insert(error_instance); + } + let mut exception_list = exception_list; for &err in &errors_to_append { - // Circular-ref guard for cause chains. - if !formatter.visited_insert(err) { - writer.write_all(b"\n")?; - pretty_write!(writer, "[Circular]")?; - continue; - } - writer.write_all(b"\n")?; let prev_depth = formatter.depth; formatter.depth = formatter.depth.saturating_add(1); - let over_cap = formatter.depth > formatter.error_chain_max_depth(); + let circular = formatter.visited_contains(err); + let over_cap = !circular && formatter.depth > formatter.error_chain_max_depth(); let result: crate::CrateResult<()> = if over_cap { pretty_write!(writer, "[Error ...]").map_err(Into::into) } else { @@ -7226,7 +7275,6 @@ impl VirtualMachine { ) }; formatter.depth = prev_depth; - formatter.visited_remove(err); result?; } diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index 355ea17b2683..4fb2e2dca37f 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/test/js/bun/util/inspect-error.test.js @@ -510,3 +510,150 @@ describe.concurrent("AggregateError whose errors cannot be walked", () => { expect(exitCode).toBe(1); }); }); + +// The printer renders an error once and prints `[Circular]` where the error +// comes back, under the key that holds it. Each entry of the printer must do +// the same: the uncaught entries, the console, `Bun.inspect` and `bun test`. +describe.concurrent("an error that reaches itself", () => { + const shapes = { + "through an own property": { + source: 'const e = new Error("x"); e.self = e;', + lines: ["error: x", " self: [Circular],"], + }, + "through cause and through errors": { + source: 'const e = new Error("cyc"); e.cause = e; e.errors = [e];', + lines: ["error: cyc", " cause: [Circular],", " errors: [", " [Circular]", "],"], + }, + "through an error inside an array property": { + source: 'const e = new Error("a"), b = new Error("b"); e.x = [b]; b.y = b;', + lines: ["error: a", " x: [", "error: b", " y: [Circular],", "],"], + }, + "through the cause of its cause": { + source: 'const e = new Error("a"), b = new Error("b"); e.cause = b; b.cause = e;', + lines: ["error: a", "error: b", " cause: [Circular],"], + }, + // The constructor makes `cause` a property that is not enumerable. + "through a cause from the constructor": { + source: 'const e = new Error("x", { cause: 0 }); e.cause = e;', + lines: ["error: x", " cause: [Circular],"], + }, + "through the cause of a cause from the constructor": { + source: 'const e = new Error("a", { cause: 0 }), b = new Error("b", { cause: e }); e.cause = b;', + lines: ["error: a", "error: b", " cause: [Circular],"], + }, + }; + const expected = Object.fromEntries(Object.entries(shapes).map(([name, { lines }]) => [name, lines])); + + // Drops the source preview, the frames and the version banner of an + // uncaught error: what is left is one line per rendered error, its + // properties and the markers. + function rendered(text) { + return text + .split("\n") + .filter(line => !/^\s*\d+ \| /.test(line) && !/^\s*\^\s*$/.test(line) && !/^\s+at /.test(line)) + .filter(line => line.trim() !== "" && !line.startsWith("Bun v")); + } + + async function run(cmd, { files, env } = {}) { + using dir = tempDir("inspect-error-cycle", files ?? {}); + await using proc = Bun.spawn({ + cmd: [bunExe(), ...cmd], + cwd: String(dir), + env: { ...bunEnv, ...env }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + // These entries return, so one process prints every shape. + const entries = { + "console.log": { log: "console.log", print: "console.log(e)", stream: "stdout", exitCode: 0 }, + "console.error": { log: "console.error", print: "console.error(e)", stream: "stderr", exitCode: 0 }, + "Bun.inspect": { log: "console.log", print: "console.log(Bun.inspect(e))", stream: "stdout", exitCode: 0 }, + "reportError": { log: "console.error", print: "reportError(e)", stream: "stderr", exitCode: 1 }, + }; + for (const [entry, { log, print, stream, exitCode }] of Object.entries(entries)) { + test(entry, async () => { + const source = Object.entries(shapes) + .map(([name, shape]) => `{ ${log}(${JSON.stringify("shape: " + name)}); ${shape.source} ${print}; }`) + .join("\n"); + const result = await run(["-e", source]); + const seen = {}; + let lines; + for (const line of rendered(result[stream])) { + if (line.startsWith("shape: ")) seen[line.slice("shape: ".length)] = lines = []; + else lines.push(line); + } + expect({ seen, exitCode: result.exitCode }).toEqual({ seen: expected, exitCode }); + }); + } + + // These entries end the process. + for (const [name, { source, lines }] of Object.entries(shapes)) { + test(`throw: ${name}`, async () => { + const { stderr, exitCode } = await run(["-e", `${source} throw e;`]); + expect({ lines: rendered(stderr), exitCode }).toEqual({ lines, exitCode: 1 }); + }); + } + + test("Promise.reject", async () => { + const { source, lines } = shapes["through cause and through errors"]; + const { stderr, exitCode } = await run(["-e", `${source} Promise.reject(e);`]); + expect({ lines: rendered(stderr), exitCode }).toEqual({ lines, exitCode: 1 }); + }); + + test("bun test: a test that rejects with it is printed once and the next test runs", async () => { + const { stderr, exitCode } = await run(["test", "./cycle.test.js"], { + files: { + "cycle.test.js": ` + import { test } from "bun:test"; + test("rejects", async () => { + ${shapes["through cause and through errors"].source} + throw e; + }); + test("next", () => {}); + `, + }, + }); + expect(rendered(stderr).filter(line => /^\s*(error: |cause: |errors: |\[Circular\]|\],)/.test(line))).toEqual( + shapes["through cause and through errors"].lines, + ); + expect(stderr).toContain(" 1 pass"); + expect(stderr).toContain(" 1 fail"); + expect(exitCode).toBe(1); + }); + + test("throw: as a member of an AggregateError that it holds", async () => { + const { stderr, exitCode } = await run([ + "-e", + 'const e = new Error("x"); e.list = [new AggregateError([e, new Error("y")], "agg")]; throw e;', + ]); + expect({ lines: rendered(stderr), exitCode }).toEqual({ + lines: ["error: x", " list: [", " [Circular]", "error: y", "],"], + exitCode: 1, + }); + }); + + test("throw: one GitHub annotation", async () => { + const { stderr, exitCode } = await run(["-e", `${shapes["through cause and through errors"].source} throw e;`], { + env: { GITHUB_ACTIONS: "true" }, + }); + expect(stderr.split("\n").filter(line => line.startsWith("::error")).length).toBe(1); + expect(exitCode).toBe(1); + }); + + test("an error that is printed twice without a cycle is rendered in full each time", async () => { + const { stdout, exitCode } = await run([ + "-e", + 'const e = new Error("twice"); e.meta = {}; console.log([e, e]); console.log(e, e); console.log({ a: e, b: { c: e } });', + ]); + const lines = rendered(stdout); + expect({ + renders: lines.filter(line => line.trim() === "error: twice").length, + circular: lines.filter(line => line.includes("[Circular]")), + exitCode, + }).toEqual({ renders: 6, circular: [], exitCode: 0 }); + }); +}); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 809f6376c75c..4eb0acb1a404 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -715,6 +715,34 @@ describe("error event", () => { expect(err).toBeInstanceOf(Error); expect(err.message).toMatch(/MessagePort \[EventTarget\] \{.*\}/s); }); + + // The worker renders its uncaught error to text before it reports the error. + test("is fired with an error that reaches itself", async () => { + using dir = tempDir("worker-error-cycle", { + "parent.cjs": ` + const { Worker } = require("node:worker_threads"); + const seen = []; + const worker = new Worker("const e = new Error('cyc'); e.cause = e; e.errors = [e]; throw e;", { eval: true }); + worker.on("error", error => seen.push(error === null ? "null" : error.name + ": " + error.message)); + worker.on("exit", code => { + seen.push("exit " + code); + console.log(JSON.stringify(seen)); + }); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "parent.cjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ + stdout: JSON.stringify(["Error: cyc", "exit 1"]), + exitCode: 0, + }); + }); }); describe("getHeapSnapshot", () => { From 9638e773baf4a0dc7b1253d00df04e5772caf3b6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:26:08 +0000 Subject: [PATCH 2/2] error printer: the cause loop needs no check for an error that is being rendered The loop over the own properties prints such an error in place, so it never reaches the queue. --- src/jsc/VirtualMachine.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 9491f1df32e9..6ff7f781b88e 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -7260,8 +7260,7 @@ impl VirtualMachine { writer.write_all(b"\n")?; let prev_depth = formatter.depth; formatter.depth = formatter.depth.saturating_add(1); - let circular = formatter.visited_contains(err); - let over_cap = !circular && formatter.depth > formatter.error_chain_max_depth(); + let over_cap = formatter.depth > formatter.error_chain_max_depth(); let result: crate::CrateResult<()> = if over_cap { pretty_write!(writer, "[Error ...]").map_err(Into::into) } else {