Skip to content

error printer: print an uncaught non-Error value at the console depth - #39637

Open
robobun wants to merge 6 commits into
mainfrom
farm/78bf8981/uncaught-non-error-depth
Open

robobun wants to merge 6 commits into
mainfrom
farm/78bf8981/uncaught-non-error-depth

Conversation

@robobun

@robobun robobun commented Aug 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • An unhandled rejection, uncaught exception, or reportError() value that is not an Error prints at inspect depth 8. console.log uses 2. A happy-dom Event rejected by @monaco-editor/loader made bun test print 145,819 lines (7.2 MB).
  • Cause: both entry points of run_error_handler build their formatter with Formatter::new (src/jsc/VirtualMachine.rs:4806, src/runtime/jsc_hooks.rs:1253), whose max_depth is 8. The non-Error branch of print_error_instance_body (VirtualMachine.rs:6432) formats the value with it.

Fix

  • Both entry points now call Formatter::for_error_handler, which sets max_depth to the console depth. The rule: the error handler prints at the depth console.error(value) prints at, and --console-depth controls both.
  • An Error prints as before: its properties already print at depth 1, and the cause walk does not read max_depth. Two tests pin that, so console: apply depth cap to Map/Set/Array entries and Error cause chains #35288, which makes the cause walk read it, has to decide that on purpose.
  • The non-Error branch now propagates a format error with ?, so the caller clears it. Under --console-depth 0 (unlimited) a very deep value makes the property walk throw a stack overflow RangeError. Discarded, it stayed pending: reportError() threw it into the script, and bun test aborted. Same line as error printer: clear the pending exception when formatting a non-Error uncaught value throws #36921.
  • Verified: test/js/bun/util/reportError.test.ts, 12 new cases. 7 fail on the released bun, and the deep value case fails with the depth change alone. Related suites pass (list in Notes).

Background

  • run_error_handler (VirtualMachine.rs:1387) is the one funnel for every value Bun reports on its own: uncaught exceptions, rejections, reportError(), bun test failures.
  • console_object::Formatter is the native inspector behind console.log. max_depth is the number of nested object levels it prints before it writes [Object ...]. Formatter::new sets it to 8.
  • The console depth is 2, --console-depth N, or bunfig [console] depth. 0 means unlimited. console_depth() now holds that lookup.
Notes

Repro with no dependencies:

// u.test.ts
import { test } from "bun:test";
function build(depth: number, width: number): any {
  if (depth === 0) return { leaf: 1 };
  const o: any = { level: depth };
  for (let j = 0; j < width; j++) o["child" + j] = build(depth - 1, width);
  return o;
}
test("unhandled rejection with a deep non-Error value", async () => {
  Promise.reject(build(12, 2));
  await new Promise(r => setTimeout(r, 10));
});

bun test ./u.test.ts 2>&1 | wc -l prints 2054 lines before this change and 38 after. Bun 1.2.23 prints the same 2054 lines, so this is not a regression.

This bounds the depth, not the size. A value that is wide at depth 2 still prints every property at those levels. That is enough for the happy-dom case: the Event itself is small, and the flood came from the levels below its [Symbol(target)]. A byte budget for this output (#37311 adds one for matchers) is a separate change. Depth parity with console.error is the right rendering with or without it.

bun -e 'reportError(x)', bun -e 'Promise.reject(x)', bun -e 'throw x', and reportError(new AggregateError([x])) all printed depth 8 before. All of them go through run_error_handler. The JSC::Exception entry point (VirtualMachine::print_exception) is changed too, so both halves of the funnel build the same formatter. Today the depth is not observable on that half: the non-Error branch and the AggregateError branch do not run for an exception cell.

Test cases: the 4 entry points above at the default depth, --console-depth 1, --console-depth 3, bunfig depth = 3, --console-depth 0 on a 12 level value, --console-depth 0 on a 20000 level value (stdout shows that the script continued, stderr is not captured because the partial dump is several MB), an uncaught Error with a deep property and three causes, an uncaught AggregateError with three Error members, and a bun test file with a test that rejects with the value plus an unhandled rejection during a test. The 7 that fail on the released bun: the 4 entry points, --console-depth 1, --console-depth 0 on 12 levels, and the bun test file. The 20000 level case passes on the released bun (depth 8 never reaches the stack guard) and fails with the depth change alone. That is what the ? is for. It is also clean under BUN_JSC_validateExceptionChecks=1.

Interaction with open PRs. #35288 makes the cause walk and the AggregateError walk of this printer read formatter.max_depth. Combined with this change, an uncaught error would print two causes and then [Error ...] by default. The cause pin here (root plus three causes) fails in that combination, so whichever lands second decides: keep the console depth for causes, or bound the cause walk on its own, as the property dump already is. #36921 contains the same ? change, so whichever lands second drops one line. #34884 seats the formatter's own stack check in Formatter::new, which for_error_handler inherits. A value whose walk has no JSC guard (a very deep array) is not changed by this PR: print_array never read max_depth.

Node prints an uncaught thrown value with inspect(value, { depth: max(defaultDepth, 5) }), and does not print a non-Error unhandled rejection reason at all (The promise rejected with the reason "#<Object>"). This change keeps Bun's behavior of printing the value. Jest prints a thrown non-Error with pretty-format maxDepth: 3, which is the same three object levels as depth 2 here.

Out of scope: a non-Error value thrown synchronously inside a test body, or inside a timer callback, reaches the printer as a JSC::Exception cell and prints only error plus the stack, with none of the value. #38278 and #37524 change how the printer unwraps that cell for Error instances. Neither unwraps other values. A value that is unwrapped there later prints through the same bounded branch.

docs/runtime/console.mdx gains one sentence about the uncaught value case.

Other suites run with the debug build, all passing: test/js/bun/test/stack.test.ts, test/js/bun/test/bun_test.test.ts, test/js/bun/test/test-test.test.ts, test/regression/issue/20980.test.ts, test/cli/console-depth.test.ts, test/js/web/console/, test/js/bun/console/, test/js/bun/util/inspect.test.js, test/js/bun/util/inspect-error.test.js, test/cli/test/bun-test.test.ts. cargo fmt --all -- --check is clean.


[review] gate passed · iteration 0 · 5 files touched

fails on main (without fix)
ASAN without fix: 7 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/reportError.test.ts
bun test v1.4.0 (4c689909e)

test/js/bun/util/reportError.test.ts:
(pass) reportError [324.40ms]
(pass) native error printer handles lone surrogates in message and stack frame name as U+FFFD [294.55ms]
176 |     ["reportError", `reportError(${value})`],
177 |     ["unhandled rejection", `Promise.reject(${value})`],
178 |     ["uncaught exception", `throw ${value}`],
179 |     ["member of an uncaught AggregateError", `reportError(new AggregateError([${value}]))`],
180 |   ])("%s", async (_, code) => {
181 |     expect(await run(["-e", code])).toEqual({ stdout: "", stderr: atDepth2, exitCode: 1 });
                                          ^
error: expect(received).toEqual(expected)

  {
    "exitCode": 1,
    "stderr": 
  "error
  {
    a: {
      b: {
-       c: [Object ...],
+       c: {
+         d: 1,
+       },
      },
    },
  }
  
  Bun v<bun-version>"
  ,
    "stdout": "",
  }

- Expected  - 1
+ Received  + 3

      at <anonymous> (/workspace/bun/test/js/bun/util/reportError.test.ts:181:37)

... (truncated)

release without fix: 7 FAILED
bun test v1.4.0-canary.1 (4c689909e)

test/js/bun/util/reportError.test.ts:
(pass) reportError [8.28ms]
(pass) native error printer handles lone surrogates in message and stack frame name as U+FFFD [12.30ms]
176 |     ["reportError", `reportError(${value})`],
177 |     ["unhandled rejection", `Promise.reject(${value})`],
178 |     ["uncaught exception", `throw ${value}`],
179 |     ["member of an uncaught AggregateError", `reportError(new AggregateError([${value}]))`],
180 |   ])("%s", async (_, code) => {
181 |     expect(await run(["-e", code])).toEqual({ stdout: "", stderr: atDepth2, exitCode: 1 });
                                          ^
error: expect(received).toEqual(expected)

  {
    "exitCode": 1,
    "stderr": 
  "error
  {
    a: {
      b: {
-       c: [Object ...],
+       c: {
+         d: 1,
+       },
      },
    },
  }
  
  Bun v<bun-version>"
  ,
    "stdout": "",
  }

- Expected  - 1
+ Received  + 3

      at <anonymous> (/workspace/bun/test/js/bun/util/reportError.test.ts:181:37)
176 |     ["reportError", `reportError(${value})`],
177 |     ["unhandled rejection", `Promise.reject(${value})`],
178 |     ["uncaught exception", `throw ${value}`
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/reportError.test.ts
bun test v1.4.0 (4c689909e)

test/js/bun/util/reportError.test.ts:
(pass) reportError [315.36ms]
(pass) native error printer handles lone surrogates in message and stack frame name as U+FFFD [305.74ms]
(pass) an uncaught value that is not an Error is printed at the console depth > reportError [335.56ms]
(pass) an uncaught value that is not an Error is printed at the console depth > unhandled rejection [306.12ms]
(pass) an uncaught value that is not an Error is printed at the console depth > uncaught exception [311.90ms]
(pass) an uncaught value that is not an Error is printed at the console depth > --console-depth 1 prints one level [318.80ms]
(pass) an uncaught value that is not an Error is printed at the console depth > member of an uncaught AggregateError [354.50ms]
(pass) an uncaught value that is not an Error is printed at the console depth > --console-depth 3 prints three levels [275.66ms]
(pass) an uncaught value that is not an Error is printed at the console depth > bunfig console.depth = 3 
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 624ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/39] gen generated_host_exports.rs
generated_host_exports.rs: 92 exports (host=3, lazy=10, generic=79, rust=0); 241 extern-C blocks audited
[2/39] gen cpp.rs (cppbind)
[3/39] gen JS modules (bundle-modules)
Preprocess modules (7884ms)
Bundle modules (76ms)
Postprocesss modules (64ms)
Bundle Functions (680ms)
Generate Code (29ms)

[8.75s] Bundled "src/js" for production
  2628 kb
  198 internal modules
  13 native modules
  92 internal functions across 17 files
[3/30] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

�[1m�[92m   Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
�[1m�[92m   Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
�[1m�[92m   Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
�[1m�[92m   Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
�[1m�[92m   Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
�[1m�[92m
... (truncated)
diff hotspot
docs/runtime/console.mdx             |   2 +
 src/jsc/ConsoleObject.rs             |  24 ++--
 src/jsc/VirtualMachine.rs            |  11 +-
 src/runtime/jsc_hooks.rs             |   4 +-
 test/js/bun/util/reportError.test.ts | 218 ++++++++++++++++++++++++++++++++++-
 5 files changed, 241 insertions(+), 18 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                  reads  edits  tests
docs/runtime/console.mdx                  1      1      0
src/jsc/ConsoleObject.rs                  7      8      0
src/jsc/VirtualMachine.rs                 6      3      0
src/runtime/jsc_hooks.rs                  1      1      0
test/js/bun/util/reportError.test.ts      3      2      0

The uncaught exception and unhandled rejection printer built its
Formatter with Formatter::new, whose max_depth is 8. A thrown or
rejected value that is not an Error is formatted whole with that
formatter, so a wide object graph printed thousands of lines, and
--console-depth had no effect on it.

Both entry points of run_error_handler now build the formatter with
Formatter::for_error_handler, which uses the console depth (2 by
default, --console-depth, bunfig console.depth). The three places that
looked the console depth up share console_depth(). Error instances
are unaffected: their own properties were already printed at depth 1.
@robobun

robobun commented Aug 19, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 10:41 AM PT - Aug 19th, 2026

✅ @robobun, your commit 8f7b2706633bc5469a836255854eb3c32c249abc passed in Build #101319! 🎉


🧪   To try this PR locally:

bunx bun-pr 39637

That installs a local version of the PR into your bun-39637 executable, so you can run:

bun-39637 --bun

@coderabbitai

coderabbitai Bot commented Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f07e115-7019-4509-8b55-371dfb4b068b

📥 Commits

Reviewing files that changed from the base of the PR and between d8794fa and 15099dd.

📒 Files selected for processing (3)
  • src/jsc/ConsoleObject.rs
  • src/jsc/VirtualMachine.rs
  • test/js/bun/util/reportError.test.ts

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.


Walkthrough

The change applies the configured console inspection depth to uncaught non-Error values, unhandled rejections, and related error-reporting paths. Tests cover runtime options, bunfig.toml, direct reporting, deep objects, and bun test failures.

Changes

Console depth reporting

Layer / File(s) Summary
Shared formatter depth handling
src/jsc/ConsoleObject.rs
Adds shared console-depth resolution and a dedicated Formatter::for_error_handler constructor.
Error reporting integration
src/jsc/VirtualMachine.rs, src/runtime/jsc_hooks.rs
Exception-reporting paths use Formatter::for_error_handler and propagate generic formatting errors.
Reporting coverage and documentation
test/js/bun/util/reportError.test.ts, docs/runtime/console.mdx
Tests cover configured depths, unlimited depth, deep objects, existing Error formatting, and multiple uncaught-value paths. Documentation describes the configured-depth behavior.

Possibly related PRs

Suggested reviewers: jarred-sumner, alii

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change to uncaught non-Error value formatting.
Description check ✅ Passed The description explains the problem, fix, scope, verification steps, test results, and related considerations in sufficient detail.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: the fix is pushed and the self-review findings are addressed (15099dd).

Reproduced with the released bun (1.4.0-canary.1+4c689909e) and with 1.2.23:

bun -e 'reportError({ a: { b: { c: { d: 1 } } } })'           # prints d: 1 (depth 8)
bun --console-depth 1 -e 'reportError({ a: { b: { c: 1 } } })' # same output, the flag is ignored
bun test ./u.test.ts 2>&1 | wc -l                              # 2054 lines for the repro in the PR body

With this branch the first command prints c: [Object ...], the flag applies, and the repro test file prints 38 lines.

The self-review found that the depth change alone made --console-depth 0 on a very deep value leave a pending exception (reportError() threw into the script, bun test aborted). The non-Error branch now propagates the format error, and a test covers it.

test/js/bun/util/reportError.test.ts: 7 of the 12 new cases fail on the released bun, all 14 tests in the file pass with the debug build.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — bounded, well-tested change that aligns the error printer's non-Error inspection depth with console.log and makes --console-depth govern both. The inline note on the test helper's undrained stdout is a nit; output there is a single banner line, nowhere near the pipe buffer.

Extended reasoning...

Overview

The PR changes two Formatter::new call sites in the uncaught-error funnel (VirtualMachine::print_exception and the non-Exception branch of jsc_hooks::print_exception) to a new Formatter::for_error_handler constructor whose only difference from new() is max_depth = console_depth(). console_depth() is a new pub(crate) helper that consolidates three identical bun_options_types::context::try_get() lookups (in message_with_type_and_level_, Bun__ConsoleObject__timeLog, and now for_error_handler). One doc sentence and eight new subprocess tests round it out. Net production change is ~15 lines.

Security risks

None. No untrusted input handling changes; this only lowers the default max_depth used when formatting an uncaught non-Error value from 8 to the console depth (2). If anything it reduces attack surface by bounding output volume.

Level of scrutiny

Low-to-medium. The Rust change is mechanical: extract a helper, add a thin constructor, swap two call sites. for_error_handler composes with new() so all other defaults are preserved. The Error-instance path is unaffected because print_error_instance_body already overrides max_depth to 1 for own-properties, and the cause chain doesn't read it (per the PR description and confirmed by the untouched print_externally_remapped_zig_exception path, which passes JSValue::ZERO and never reaches the non-Error dump). The remaining Formatter::new sites I checked (host_fn.rs, ConsoleObject.rs:681/1381/1443, VirtualMachine.rs:5853) are not part of run_error_handler's raw-value branch.

Other factors

The behavior change is user-visible but well-motivated (145K-line / 7.2 MB output on a rejected happy-dom Event), consistent with console.log's existing depth, controllable via the same --console-depth/bunfig knob, and documented in docs/runtime/console.mdx. Jest bounds thrown non-Errors similarly. Tests cover the four entry points (reportError, unhandled rejection, top-level throw, AggregateError member), CLI flag override, bunfig override, and bun test reporting; the author reports 6/8 fail on the released bun, satisfying the fails-for-the-right-reason bar. The stdout-pipe nit is test-harness-only and cannot deadlock at these output sizes.

Comment thread test/js/bun/util/reportError.test.ts Outdated
Comment thread src/jsc/ConsoleObject.rs Outdated
Comment thread src/jsc/ConsoleObject.rs Outdated
Comment thread src/jsc/ConsoleObject.rs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the prior stdout-drain nit and the comment-cop flags are all addressed.

What was reviewed:

  • console_depth() extraction is byte-identical to the two inlined lookups it replaces in message_with_type_and_level_ and Bun__ConsoleObject__timeLog.
  • for_error_handler differs from Formatter::new only in max_depth; the Error branch of print_error_instance_body overwrites max_depth to 1, so Error output is unchanged.
  • Checked remaining Formatter::new callers in the error path — the print_externally_remapped_zig_exception fallback handles a ZigException cell where the non-Error branch does not run, so leaving it at the default is fine.
  • Tests now drain and assert {stdout, stderr, exitCode} for every case, use test.concurrent + tempDir, and cover all four entry points plus CLI/bunfig/bun-test.
Extended reasoning...

Overview

Five files. src/jsc/ConsoleObject.rs gains a console_depth() helper (extracts the existing bun_options_types::context::try_get()...unwrap_or(DEFAULT_CONSOLE_LOG_DEPTH) chain used at two sites) and a Formatter::for_error_handler constructor that calls Self::new then sets max_depth = console_depth(). src/jsc/VirtualMachine.rs:4806 and src/runtime/jsc_hooks.rs:1253 swap Formatter::new for Formatter::for_error_handler (the latter also drops a stale one-line comment). test/js/bun/util/reportError.test.ts adds a describe block with 8 concurrent cases. docs/runtime/console.mdx adds one sentence.

Security risks

None. This changes only the max_depth field on a formatter used to print diagnostic output to stderr. No parsing of untrusted input, no allocation sizing derived from external data, no auth/crypto/permissions.

Level of scrutiny

Low-to-medium. The Rust delta is ~15 net lines and mechanical: one extracted helper (behavior-identical to the two inlined copies it replaces — verified by reading the diff hunks side by side), one named constructor that mutates one field, two one-line call-site swaps. The behavior change is well-bounded: only non-Error values reaching the error printer change, from depth 8 to the console depth. The PR description shows the Error branch already overrides max_depth to 1, and I checked the remaining Formatter::new in print_externally_remapped_zig_exception — it handles a ZigException cell where the non-Error / AggregateError branches do not run, so the default is unobservable there (matching the PR's own note about the exception-cell half of the funnel).

Other factors

My prior review flagged the run helper piping stdout without draining it; commit 3a07323 fixed this — the helper now awaits [stdout, stderr, exited] and every test asserts the combined {stdout, stderr, exitCode} object. The three comment-cop flags on long doc comments were addressed in 63b2310 and d8794fa (both are now one-line doc comments). All review threads are resolved. Tests follow harness conventions (tempDir, bunEnv, normalizeBunSnapshot, test.concurrent), and the author confirmed 6/8 fail on the released bun and all pass on the debug build. The behavior change is well-justified: it aligns uncaught non-Error output with console.error, makes --console-depth control it, and bounds the 145k-line happy-dom pathological case.

…ror output

With --console-depth 0 the property walk of a very deep value throws a
stack overflow RangeError. The non-Error branch discarded the Err and
left the exception pending, so reportError() threw it into the script
and bun test aborted with a pending exception. Propagate it so
print_error_from_maybe_private_data clears it, as it did before the
port (the same line as #36921).

Tests: depth 0 prints every level, the deep value case continues, and
an uncaught Error's properties, causes and AggregateError members print
as before.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the earlier stdout-drain nit and the comment-cop notes are addressed, and the follow-up commit that propagates the format error is sound.

What was reviewed:

  • for_error_handler only changes max_depth; the Error path still overrides to depth 1 and the cause chain is unaffected (pinned by the two "prints as before" tests).
  • The let _ = format(...) → ? change reaches print_error_from_maybe_private_data, which clears the pending JS exception — the --console-depth 0 stack-overflow test exercises exactly that.
  • The remaining Formatter::new fallback at VirtualMachine.rs:5853 is the error_instance == ZERO ZigException path where the non-Error branch never runs, so depth is not observable there.
Extended reasoning...

Overview

The PR bounds the inspect depth used when Bun's error handler prints an uncaught / rejected value that is not an Error. It adds console_depth() (deduplicating three identical option lookups) and Formatter::for_error_handler() (a new() that sets max_depth to the console depth), and swaps the two run_error_handler entry points to use it. It also changes the non-Error branch of print_error_instance_body to propagate a format error via ? instead of discarding it, adds one docs sentence, and adds ~200 lines of tests covering all four entry points, the CLI flag, bunfig, depth 0 (unlimited + stack-guard), and negative pins that Error / AggregateError output is unchanged.

Security risks

None. This is output-formatting depth for diagnostic printing to stderr. No parsing of untrusted input, no auth, no FFI/lifetime changes, no allocation-size arithmetic. The only new code path that touches JS state is the ? propagation, which routes to an existing clear_exception() call in the caller.

Level of scrutiny

Low-to-medium. The native change is ~15 lines across three files and is purely a config-value plumbing change: the two call sites now build the formatter with max_depth = console_depth() instead of the hard-coded 8. The only semantic subtlety — the let _ = → ? swap — I traced to print_error_from_maybe_private_data at VirtualMachine.rs:5329, which already handles CrateError::JSError by calling clear_exception(), matching the added comment. The new stack-overflow test (20000-level object at depth 0) verifies reportError() does not leak that exception into the calling script. I also grepped for other Formatter::new sites feeding print_error_instance_body; the only remaining one (VirtualMachine.rs:5853) is the error_instance == JSValue::ZERO fallback where the depth-reading branch cannot execute, which the PR description already calls out.

Other factors

  • All prior review threads are resolved: my stdout-drain nit was fixed in 3a07323 (the helper now drains and asserts {stdout, stderr, exitCode}), and the three comment-cop flags were addressed by shortening the doc comments to one line each.
  • This is a user-visible default change (depth 8 → 2 for uncaught non-Error values), but strictly in the less-output direction, controllable by the existing --console-depth flag, and aligned with console.error and Jest's maxDepth: 3. The motivating case (7.2 MB / 145k lines from one happy-dom rejection) makes the current default clearly wrong.
  • Test coverage is thorough per REVIEW.md's variant-matrix rule: all four entry points, CLI flag at 0/1/3, bunfig, bun test reporter, and negative pins for Error/AggregateError. Tests use test.concurrent, tempDir, bunEnv, and drain both pipes.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs; the earlier stdout-drain nit and the comment-cop flags are addressed. Because this changes a user-facing default (uncaught non-Error values now print at depth 2 instead of 8) and the PR body flags an intentional interaction with #35288's cause-walk depth, a maintainer sign-off on that choice would still be worthwhile.

What was reviewed:

  • console_depth() is a pure extraction of the two existing inline lookups; for_error_handler only overrides max_depth on top of Formatter::new.
  • The new ? on formatter.format propagates to print_error_from_maybe_private_data, which already matches CrateError::JSError and calls clear_exception() — same handling as the neighboring Tag::get_advanced? / get_own? calls.
  • Checked that the remaining Formatter::new sites (host_fn.rs, VirtualMachine.rs:5853 default_formatter, ConsoleObject.rs) are not error-handler entry points, so leaving them at depth 8 is intentional.
Extended reasoning...

Overview

Five files touched. src/jsc/ConsoleObject.rs adds a console_depth() helper (extracted from two identical inline lookups in message_with_type_and_level_ and Bun__ConsoleObject__timeLog) and a Formatter::for_error_handler constructor that calls Self::new and sets max_depth = console_depth(). src/jsc/VirtualMachine.rs swaps Formatter::new → for_error_handler at the print_exception entry point and changes let _ = formatter.format(...) → formatter.format(...)? in the non-Error branch of print_error_instance_body. src/runtime/jsc_hooks.rs makes the same constructor swap at the other entry point and drops a stale comment. test/js/bun/util/reportError.test.ts gains a 12-case describe block. docs/runtime/console.mdx gains one sentence.

Security risks

None. This changes only the inspect depth used when printing an uncaught value to stderr, and how a formatter error (stack-overflow RangeError from a very deep walk) is propagated. No parsing of untrusted input, no auth, no allocation-size arithmetic. The ? change routes through an existing clear_exception() handler at VirtualMachine.rs:5329-5330, so it does not introduce a new pending-exception path.

Level of scrutiny

Medium. The Rust change is ~15 production lines and mechanically simple, but the effect is a user-facing default change: an uncaught non-Error value that used to print 8 object levels now prints 2 (or whatever --console-depth is set to). The PR body argues this well — parity with console.error, fixes a real 7.2 MB / 145k-line output flood, matches Jest's depth-3 — and pins the Error/cause behavior with two tests so #35288 can't silently combine into a shallower cause chain. That said, choosing "same depth as console.error" over "deeper for debugging" is a product decision a maintainer should confirm, and the PR body itself calls out that whichever of this and #35288 lands second has to make a call.

Other factors

  • All prior feedback is addressed and the threads are resolved: the run helper now drains stdout and every case asserts { stdout, stderr, exitCode }; the two doc comments flagged by comment-cop are now one line each.
  • I traced the ? propagation: format returns JsResult<()>, which ? converts through print_error_instance_body → print_error_instance_js → print_error_from_maybe_private_data, which matches CrateError::JSError and calls global().clear_exception(). This is the same path the neighboring Tag::get_advanced(...)? and get_own(...)? already use, so the change is consistent and safe.
  • I checked the remaining Formatter::new call sites: host_fn.rs:174 (debug-format helper), VirtualMachine.rs:5853 (default_formatter for the ZigException-only overload, not on the run_error_handler path), and the ConsoleObject-internal sites. None are run_error_handler entry points, so leaving them at depth 8 matches the PR's stated scope.
  • Test coverage is thorough (all four entry points, CLI flag, bunfig, unlimited depth at 12 and 20000 levels, Error/AggregateError pins, bun test reporter path) and the PR states 7/12 fail on the released bun and pass with the debug build, plus a clean BUN_JSC_validateExceptionChecks=1 run.
  • The 20000-level test uses stderr: "ignore" deliberately (the partial dump is several MB) and asserts on stdout + signalCode === null + exit code, which is the correct hang-guard shape for that case.

Jarred-Sumner pushed a commit that referenced this pull request Sep 17, 2026
…ins (#35288)

### Problem
- `console.log` of a nested `Map`, `Set`, `Array`, `MessageEvent`, Error
`cause` chain, or `AggregateError` prints every level. A 1000-deep `Map`
prints 2 MB. Deeper chains throw `RangeError: Maximum call stack size
exceeded` out of `console.log`. A nested `AggregateError` chain
overflows the native stack.
- Cause: only `print_object` (`src/jsc/ConsoleObject.rs`) compares
`depth` to `max_depth`. The other container printers never compare it.
The `cause` loop and `agg_iter` (`src/jsc/VirtualMachine.rs`) do not
track depth.

### Fix
- Each container printer returns `[Array ...]`, `[Map ...]`, `[Set
...]`, `[MapIterator ...]`, or `[MessageEvent ...]` past the cap, like
`[Object ...]`, through one `print_depth_exceeded_marker`. An empty
container still prints `[]`, `Map {}`, or `Set {}`.
- The `cause` loop and `agg_iter` bump `depth` and print `[Error ...]`
past the cap. `agg_iter` also checks the native stack guard
(`stack_check`). The error property dump narrows `max_depth`, so it
keeps the caller's cap in `outer_max_depth` for these walks.
- `Bun.inspect.table` passes its `depth` option as the cell start depth,
against a fixed `max_depth` of 5. `TablePrinter::set_start_depth` now
clamps it, so a `depth` above 5 prints cells like the default.
- Verified: `inspect.test.js` (13 new cases, 12 fail on the released
bun), `bun-inspect-table.test.ts` (2 new), and the related console
suites.

### Background
- `Formatter` (`ConsoleObject.rs`) is the native walker behind
`console.log` and `Bun.inspect`. `max_depth` comes from
`--console-depth`, `Bun.inspect(x, {depth})`, or the default of 2.
- `print_error_instance_body` (`VirtualMachine.rs`) prints an Error, not
`Formatter`. It dumps the error's properties one level deep and walks
`cause` and `AggregateError.errors` itself.
- `{depth: Infinity}` sets `max_depth` to `u16::MAX`, and `depth`
saturates there. A depth comparison alone cannot stop that walk.

<details><summary>Notes</summary>

Sizes before and after, on this branch:

| input | before | after | node |
|-|-|-|-|
| 100-deep `cause` chain | ~20 KB | 711 B | 575 B |
| 1000-deep `Map` | ~2 MB | 167 B | 60 B |
| 1000-deep `Set` | ~2 MB | 152 B | 40 B |
| 100-deep `Array` | ~20 KB | 129 B | 20 B |
| 1000-deep `MessageEvent` | ~3 MB | 261 B | n/a |
| 3000 causes, 12000 Maps, 20000 AggregateErrors | `RangeError` or
SIGSEGV | truncates | truncates |

Tables. A container nested inside a cell now prints as a marker, like a
nested plain object already did: `{ x: [Object ...] }`, `[ [Array ...]
]`, `Map(1) { 1: [Map ...] }`. Node's `console.table` prints the same
shape (`[ [Array] ]`, `Map(1) { 1 => [Map] }`). For a `depth` option
above 5, the released bun starts every cell past the cap: a plain object
cell prints ` [Object ...]` and the new gates would have done the same
to Array, Map, and Set cells. The clamp makes such a depth print cells
like the default, object cells included. `{ depth: 0 }` keeps its
meaning (`console-table.test.ts` uses it to mirror `console.table`). The
option is not redefined as a `max_depth`: #34241 did that and was closed
without a merge.

AggregateError at the cap. When the members are one level past the cap,
the AggregateError prints like any other error first (name, message,
stack), then one `[Error ...]` per member. Without that,
`Bun.inspect(agg, { depth: 0 })` printed only the markers. Node prints
the same shape: the header, then `[errors]: [Array]`. The normal output
is unchanged: members only, as #39633 left it.

Errors inside Error properties. With `err.details = { inner }`,
`err.list = [inner]`, or an `AggregateError` inside a property, the
cause and the members of the nested error follow the caller's depth:
`depth: Infinity` prints them all, `depth: 2` prints `[Error ...]`.
Plain object properties keep the one-level cap.

Cause depth per entry point, measured with a 50-deep chain.
`console.log`: Node prints root + 2 causes, this branch prints root + 2
causes. Uncaught `throw`: Node prints root + 5 causes, this branch
prints root + 8 (the error handler's `Formatter::new` depth), released
bun prints all 50. #39637 changes the error handler's formatter to the
console depth. Combined with this PR as written, an uncaught error would
print root + 2 causes, and #39637's three-cause test would fail. The
cause loop reads the depth cap on purpose so `{depth}` and
`--console-depth` control it. Whichever PR lands second has to pick: the
console depth for causes, or #39637 applying its depth only on the
non-Error branch.

`test/js/web/console/console-log.expected.txt` changes for
`[[[[Array(1000).fill(4)]]]]`, which now stops at the fourth level like
Node. The removed text (a long array wrapped at indent 10, then `... 900
more items`) is asserted again by `long arrays get cutoff at a nested
indent` in `console-log.test.ts`, through `Bun.inspect(x, { depth: 5
})`.

`stack_check` is the formatter's native stack guard. Container printers
reach it in `print_as_prelude`. The error walks do not, so `agg_iter`
calls it directly.

Suites run on the merged branch: `inspect.test.js`,
`inspect-error.test.js`, `reportError.test.ts`, `console-log.test.ts`,
`console-depth.test.ts`, `bun-inspect.test.ts`,
`bun-inspect-table.test.ts`, `console-table.test.ts`,
`build-error.test.ts` (244 pass, 1 skip that is also skipped on main).

JSX and Proxy have the same unbounded recursion. #29709 is open with
that fix, so it is left out here.

Review history: the `AggregateError` gate, its `stack_check`, the
`print_event` gate, the relative `max_depth`, the state restore before
`?` in the cause loop, the empty-container order, `outer_max_depth`, the
table clamp, and the AggregateError header at the cap each came from
review and each has a test.

The branch merges main as of 2026-09-16. The only textual conflict was
in `inspect.test.js`, where both sides appended a describe block.

</details>

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 3 · platform-specific test(s) that do not
run on this machine, deferring to CI, which covers all platforms:
test/js/bun/util/inspect.test.js

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@robobun

robobun commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator Author

A note on overlap with #43238 (not a change request for the rest of this PR).

#43238 touches the same non-Error branch of print_error_instance_body. A maintainer asked there that #43238 own what a value throws while it is printed: the branch clears the exception itself and still prints the frames of the throw. It leaves the two formatter.format(..) lines as they are on main, so this PR still merges cleanly with it.

If #43238 lands first, the hunk here that turns those two lines into ? is no longer needed. It would also return before the frames are printed, and the test an object whose toString throws keeps the frames of the throw in test/js/bun/test/stack.test.ts fails with it. The exception this PR fixes is already cleared by #43238 on the bare path too (reportError(v), an unhandled rejection), because it is the same code. The tests of this PR are worth a re-run on top of #43238 at that point: the report of a hostile value gains a newline there.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant