Skip to content
Open
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
10 changes: 5 additions & 5 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,13 +502,13 @@ fn message_with_type_and_level_(
let mut table_printer = TablePrinter::init(global, level, tabular_data, properties)?;
table_printer.value_formatter.indent += u32::from(default_indent);

if enable_colors {
let _ = table_printer.print_table::<true>(writer);
let printed = if enable_colors {
table_printer.print_table::<true>(writer)
} else {
let _ = table_printer.print_table::<false>(writer);
}
table_printer.print_table::<false>(writer)
};
let _ = writer.flush();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return Ok(());
return printed;
}
}

Expand Down
57 changes: 57 additions & 0 deletions test/js/bun/console/console-table.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,3 +363,60 @@
expect({ stdout, stderr, exitCode }).toEqual({ stdout: box("1") + "calls=1\n", stderr: "", exitCode: 0 });
});
});

// Reading a cell runs user code (getters, Proxy traps, custom inspect). An
// exception thrown there must surface from console.table itself, exactly as
// it does from Bun.inspect.table, rather than being swallowed by the printer.
describe("console.table propagates exceptions thrown while reading cells", () => {

Check warning on line 370 in test/js/bun/console/console-table.test.ts

View check run for this annotation

Claude / Claude Code Review

New tests pass with and without the Rust fix

As the PR description notes, all four new tests pass with and without the Rust change — the `Err(Thrown)` case already surfaced via the pending exception, and the only JS-observable delta (`Err(OutOfMemory)` propagation) isn't exercised. The tests are still useful as regression coverage for the throw contract, and the OOM path isn't practically triggerable from JS, so this is just a note that reverting `src/jsc/ConsoleObject.rs:505-511` would leave the suite green.
Comment thread
robobun marked this conversation as resolved.
test("a throwing getter on a row", () => {
const boom = new Error("getter boom");
const row = {};
Object.defineProperty(row, "x", {
get() {
throw boom;
},
enumerable: true,
});
expect(() => console.table([row])).toThrow(boom);
});

test("a throwing Proxy trap on the tabular data", () => {
const boom = new Error("proxy boom");
const data = new Proxy(
{ a: 1 },
{
ownKeys() {
throw boom;
},
},
);
expect(() => console.table(data)).toThrow(boom);
});

test("a throwing custom inspect in a cell", () => {
const boom = new Error("inspect boom");
const cell = {
[Bun.inspect.custom]() {
throw boom;
},
};
expect(() => console.table([{ x: cell }])).toThrow(boom);
});

test("nothing is printed and the error is uncaught in a script", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`console.table([{ get x() { throw new Error("table getter boom"); } }]);
console.log("unreachable");`,
],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("");
expect(stderr).toContain("table getter boom");
expect(exitCode).toBe(1);
});
});