Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
26 changes: 14 additions & 12 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4761,7 +4761,19 @@
// once the AggregateError branch is taken).
let global_ref = self.global();

// 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 value.is_aggregate_error(global_ref) {

Check warning on line 4776 in src/jsc/VirtualMachine.rs

View check run for this annotation

Claude / Claude Code Review

AggregateError as .cause still omits its member errors

Related gap (pre-existing, not a regression): an `AggregateError` reached via `.cause` still omits its member errors. The `.errors` iteration lives only in `print_errorlike_object`, but the `.cause` recursion at ~line 6092 calls `print_error_instance_js` → `print_error_instance_body` directly, and `.errors` is `DontEnum` so the own-property loop skips it — so `throw new Error("outer", { cause: new AggregateError([new Error("inner")], "agg") })` prints `AggregateError: agg` but never `inner`. Wor
Comment thread
robobun marked this conversation as resolved.
// 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 @@ -4797,6 +4809,7 @@
// SAFETY: `ctx.writer` borrows the caller's stack local,
// live across the synchronous `for_each` call.
let writer = unsafe { &mut *ctx.writer };
let _ = writer.write_all(b"\n");

Check warning on line 4812 in src/jsc/VirtualMachine.rs

View check run for this annotation

Claude / Claude Code Review

Double blank line between BuildMessage members in AggregateError output

The unconditional `writer.write_all(b"\n")` prepend gives one blank line between the header and the first member but *two* blank lines between consecutive `BuildMessage`/`ResolveMessage` members, because those already emit a trailing blank line of their own — the regenerated snapshot in `jsx-template-string-crash.test.ts` captures the double blank. Purely cosmetic (the pre-PR spacing between members was one blank line); could be tightened by tracking whether the previous member already ended in
Comment thread
robobun marked this conversation as resolved.
vm.print_errorlike_object(
next_value,
None,
Expand All @@ -4811,6 +4824,7 @@
formatter: std::ptr::from_mut(formatter),
writer: std::ptr::from_mut(writer),
exception_list: exception_list
.as_deref_mut()
.map(std::ptr::from_mut::<ExceptionList>)
.unwrap_or(core::ptr::null_mut()),
allow_ansi_color,
Expand All @@ -4824,18 +4838,6 @@
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
110 changes: 78 additions & 32 deletions test/js/bun/util/inspect-error.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { describe, expect, jest, test } from "bun:test";
import { bunEnv, bunExe } from "harness";

test("error.cause", () => {
const err = new Error("error 1");
Expand All @@ -9,21 +10,23 @@ test("error.cause", () => {
.replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"),
).toMatchInlineSnapshot(`
"1 | import { describe, expect, jest, test } from "bun:test";
2 |
3 | test("error.cause", () => {
4 | const err = new Error("error 1");
5 | const err2 = new Error("error 2", { cause: err });
2 | import { bunEnv, bunExe } from "harness";
3 |
4 | test("error.cause", () => {
5 | const err = new Error("error 1");
6 | const err2 = new Error("error 2", { cause: err });
^
error: error 2
at <anonymous> ([dir]/inspect-error.test.js:5:20)
at <anonymous> ([dir]/inspect-error.test.js:6:20)

1 | import { describe, expect, jest, test } from "bun:test";
2 |
3 | test("error.cause", () => {
4 | const err = new Error("error 1");
2 | import { bunEnv, bunExe } from "harness";
3 |
4 | test("error.cause", () => {
5 | const err = new Error("error 1");
^
error: error 1
at <anonymous> ([dir]/inspect-error.test.js:4:19)
at <anonymous> ([dir]/inspect-error.test.js:5:19)
"
`);
});
Expand All @@ -35,15 +38,15 @@ test("Error", () => {
.replaceAll("\\", "/")
.replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"),
).toMatchInlineSnapshot(`
"27 | "
28 | \`);
29 | });
30 |
31 | test("Error", () => {
32 | const err = new Error("my message");
"30 | "
31 | \`);
32 | });
33 |
34 | test("Error", () => {
35 | const err = new Error("my message");
^
error: my message
at <anonymous> ([dir]/inspect-error.test.js:32:19)
at <anonymous> ([dir]/inspect-error.test.js:35:19)
"
`);
});
Expand Down Expand Up @@ -73,19 +76,11 @@ note: "duplicateConstDecl" was originally declared here

const normalizeError = str => {
// remove debug-only stack trace frames
// like "at require (:1:21)"
if (str.includes(" (:")) {
const splits = str.split("\n");
for (let i = 0; i < splits.length; i++) {
if (splits[i].includes(" (:")) {
splits.splice(i, 1);
i--;
}
}
return splits.join("\n");
}

return str;
// like "at require (:1:21)" or "at require (51:24)"
return str
.split("\n")
.filter(line => !/^\s+at .+ \(:?\d+:\d+\)$/.test(line))
.join("\n");
};

test("Error inside minified file (no color) ", () => {
Expand All @@ -111,7 +106,7 @@ test("Error inside minified file (no color) ", () => {
error: error inside long minified file!
at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2850)
at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2890)
at <anonymous> ([dir]/inspect-error.test.js:92:7)"
at <anonymous> ([dir]/inspect-error.test.js:87:7)"
`);
}
});
Expand Down Expand Up @@ -140,7 +135,7 @@ test("Error inside minified file (color) ", () => {
error: error inside long minified file!
at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2850)
at <anonymous> ([dir]/inspect-error-fixture.min.js:26:2890)
at <anonymous> ([dir]/inspect-error.test.js:120:7)"
at <anonymous> ([dir]/inspect-error.test.js:115:7)"
`);
}
});
Expand All @@ -154,7 +149,7 @@ test("Inserted originalLine and originalColumn do not appear in node:util.inspec
.replaceAll(import.meta.path.replaceAll("\\", "/"), "[file]"),
).toMatchInlineSnapshot(`
"Error: my message
at <anonymous> ([file]:149:19)"
at <anonymous> ([file]:144:19)"
`);
});

Expand All @@ -175,6 +170,57 @@ describe("observable properties", () => {
}
});

describe("AggregateError", () => {
// Build the aggregate's message (and the expected header) at runtime so the
// source-line preview that the error printer emits cannot contain the
// assertion string by accident.
const mk = `["TOP","AGG","MESSAGE"].join("-")`;
const header = ["AggregateError", ["TOP", "AGG", "MESSAGE"].join("-")].join(": ");

test.concurrent.each([
[
"Bun.inspect",
`process.stderr.write(Bun.inspect(new AggregateError([new Error("m1"), new RangeError("m2")], ${mk})))`,
0,
],
["console.error", `console.error(new AggregateError([new Error("m1"), new RangeError("m2")], ${mk}))`, 0],
["uncaught throw", `throw new AggregateError([new Error("m1"), new RangeError("m2")], ${mk})`, 1],
["unhandled rejection", `Promise.reject(new AggregateError([new Error("m1"), new RangeError("m2")], ${mk}))`, 1],
])("%s prints the aggregate header and each member", async (_, code, wantExit) => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const out = stdout + stderr;
expect(out).toContain(header);
expect(out).toContain("error: m1");
expect(out).toContain("RangeError: m2");
expect(out.indexOf(header)).toBeLessThan(out.indexOf("error: m1"));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
expect(out.indexOf("error: m1")).toBeLessThan(out.indexOf("RangeError: m2"));
expect(exitCode).toBe(wantExit);
});

test.concurrent("unhandled Promise.any rejection prints the aggregate header", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `Promise.any([Promise.reject(new Error("r1")), Promise.reject(new Error("r2"))])`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const out = stdout + stderr;
expect(out).toContain("AggregateError");
expect(out).toContain("error: r1");
expect(out).toContain("error: r2");
expect(out.indexOf("AggregateError")).toBeLessThan(out.indexOf("error: r1"));
expect(out.indexOf("error: r1")).toBeLessThan(out.indexOf("error: r2"));
expect(exitCode).toBe(1);
});
});

test("error.stack throwing an error doesn't lead to a crash", () => {
const err = new Error("my message");
Object.defineProperty(err, "stack", {
Expand Down
10 changes: 8 additions & 2 deletions test/regression/issue/jsx-template-string-crash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ test("JSX lexer should not crash with slice bounds issues", async () => {

expect(exitCode).toBe(1);
expect(normalizeBunSnapshot(stderr.toString().replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(`
"1 | export function x(){return<div a=\`\`/>}
"AggregateError: 2 errors building "<cwd>/[eval]"

1 | export function x(){return<div a=\`\`/>}
^
error: Expected "{" but found "\`"
at <cwd>/[eval]:1:34


1 | export function x(){return<div a=\`\`/>}
^
error: Unterminated string literal
Expand Down Expand Up @@ -57,11 +60,14 @@ test.concurrent("#30959 JSX attribute with invalid '(' value parses cleanly in d
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(normalizeBunSnapshot(stderr.replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(`
"1 | export function x(){return<r L=((}
"AggregateError: 2 errors building "<cwd>/[eval]"

1 | export function x(){return<r L=((}
^
error: Expected "{" but found "("
at <cwd>/[eval]:1:32


1 | export function x(){return<r L=((}
^
error: Unexpected }
Expand Down
Loading