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..6ff7f781b88e 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,15 +7251,12 @@ 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); @@ -7226,7 +7274,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", () => {