Conversation
The global console.log/warn/error/dir use Bun's native formatter, which has no knowledge of util.inspect.defaultOptions. Setting options like depth, maxArrayLength, or numericSeparator changed util.inspect and new console.Console(stream) output, but left global console output untouched, so the documented process-wide knob was a silent no-op on the one console most code actually uses. util.inspect.defaultOptions now hands back a Proxy over the sealed defaults object. The first write installs a pair of JS formatters (formatWithOptions for log-style calls, inspect for console.dir) on the VM; once installed, the native console routes argument formatting through them instead of its own printer. The fast native path is kept for the common case where defaultOptions is never touched.
|
Reproduced with: import util from "node:util";
util.inspect.defaultOptions.depth = 0;
util.inspect.defaultOptions.maxArrayLength = 2;
util.inspect.defaultOptions.numericSeparator = true;
console.log({ a: { b: { c: { d: 1 } } } });
console.log([1, 2, 3, 4, 5, 6]);
console.log(1234567);PR #35061 routes the global console through CI: the new |
|
Updated 1:32 AM PT - Jul 22nd, 2026
❌ @robobun, your commit 4bf7b60 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 35061That installs a local version of the PR into your bun-35061 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
Added The other two suggestions are related but not addressed here:
|
WalkthroughChangesGlobal console output now uses VM-backed formatter closures after Inspect defaults and console formatting
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/js/internal/util/inspect.js`:
- Around line 733-736: Condense each new comment to no more than three lines
while preserving its explanation: update the formatter-bridge comment in
src/js/internal/util/inspect.js lines 733-736, the host-function documentation
in src/runtime/node/node_util_binding.rs lines 165-168, the VM-slot
documentation in src/jsc/VirtualMachine.rs lines 326-329, and the fallback-path
explanation in src/jsc/ConsoleObject.rs lines 561-564.
In `@src/jsc/ConsoleObject.rs`:
- Around line 592-599: Update the newline handling in the default-indentation
branch so formatter::write_indent_n is called only when rest contains content
after the matched newline. Do not emit indentation for a terminal newline, while
preserving indentation between newline-separated content segments and the
existing final write_all behavior.
In `@src/jsc/VirtualMachine.rs`:
- Around line 326-333: Clear both console formatter slots, console_util_format
and console_util_dir, in swap_global_for_test_isolation() before replacing
self.global, ensuring every isolation lifecycle exit releases callbacks from the
outgoing realm. Add a regression test that mutates inspect.defaultOptions in one
file and verifies default console formatting in the next file is unaffected.
In `@test/js/node/console/console.test.ts`:
- Around line 104-156: The console tests cover assignment and setter updates but
not the Object.defineProperty mutation path. Add a concurrent test near “setter
form and console.dir” that redefines util.inspect.defaultOptions.depth via
Object.defineProperty and asserts console output reflects depth 0, along with
successful exit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bded88a7-000a-4675-bbab-5a097e7d7dba
📒 Files selected for processing (5)
src/js/internal/util/inspect.jssrc/jsc/ConsoleObject.rssrc/jsc/VirtualMachine.rssrc/runtime/node/node_util_binding.rstest/js/node/console/console.test.ts
…p, cover defineProperty
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Writing to an object that inherits from util.inspect.defaultOptions (Object.create(util.inspect.defaultOptions)) was leaking through to the shared defaults because the set trap dropped the receiver argument.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/node/console/console.test.ts`:
- Around line 168-182: Strengthen the test around the inheriting object setup by
capture the console output before my.depth = 10, then compare the post-write
console.log output against that baseline exactly. Keep the existing sharedDepth
and ownDepth assertions, and ensure the console result assertion cannot pass
when no object is emitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca33dd3a-f45d-4182-bf80-8b48aad15ea6
📒 Files selected for processing (5)
src/js/internal/util/inspect.jssrc/jsc/ConsoleObject.rssrc/jsc/VirtualMachine.rssrc/runtime/node/node_util_binding.rstest/js/node/console/console.test.ts
…pth from --console-depth console.timeLog's extra data args are formatted by a separate ConsoleClient hook that was still using the native formatter; check the same VM slot there. inspectDefaultOptions.depth now starts at the --console-depth / bunfig console.depth value when one is set, so touching an unrelated default (maxArrayLength, etc.) no longer drops the console back to depth 2.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/jsc/ConsoleObject.rs`:
- Around line 6018-6043: Update the formatter path in the console time-log
handling to propagate failures from console_util_format through the same VM
exception/OOM handling used by message_with_type_and_level, rather than
discarding them in nested if let Ok checks. Complete formatting and validate its
result before writing the elapsed-time label or any other output, while
preserving the existing successful formatting and newline/flush behavior.
In `@test/js/node/console/console.test.ts`:
- Around line 153-169: Update the test case around the Bun.spawn call and its
Promise.all to also await proc.stderr.text() alongside stdout and proc.exited,
ensuring the piped stderr stream is drained while preserving the existing stdout
and exit-code assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d27ebe54-c10b-4188-84ba-302a6544eb0d
📒 Files selected for processing (4)
src/js/internal/util/inspect.jssrc/jsc/ConsoleObject.rssrc/runtime/node/node_util_binding.rstest/js/node/console/console.test.ts
Seeding inspectDefaultOptions.depth from --console-depth widened the flag to util.inspect(), util.format, user Console instances, and the REPL defaults, and made util.inspect.defaultOptions.depth diverge from Node's documented 2. Instead, keep the Node default and thread the CLI depth into the fallback closures only, yielding to an explicit defaultOptions.depth write when the user makes one.
There was a problem hiding this comment.
Earlier findings (timeLog routing, --console-depth scope leak) are addressed in 4bf7b60 and no new issues turned up on this revision. Deferring the final call to a maintainer given the design trade-offs — the Proxy-based write detection (accepted isProxy divergence) and the native→util.inspect formatter switch on first mutation are choices worth a human sign-off.
What was reviewed:
--console-depthnow stays scoped to the global-console fallback;util.inspect.defaultOptions.depthreads 2 andutil.inspect()/userConsoleinstances are unaffected (test asserts both).strong::Optionalslots aredeinit'd on both VM teardown paths; per-VM so workers don't share state.to_sliceon the JS-formatted result goes throughto_utf8(), so non-ASCII output is written correctly.- Proxy
settrap forwardsreceiverfor inherited writes — covered by theObject.create(defaultOptions)test.
Extended reasoning...
Overview
The PR wires util.inspect.defaultOptions into the global console by wrapping the sealed defaults in a Proxy whose first write installs two JS formatter closures (formatWithOptionsInternal for log-style, inspect for console.dir) as strong::Optional slots on VirtualMachine. message_with_type_and_level_ and Bun__ConsoleObject__timeLog check those slots and route through the JS formatters when set, otherwise keep the native fast path. Two host functions in node_util_binding.rs bridge --console-depth and the formatter install. ~280 lines across 5 files with 8 new subprocess tests.
Security risks
None. No untrusted-input parsing, auth, or filesystem/network surface. The Proxy handler is __proto__: null and uses captured Reflect primordials; the fallback closures are module-private and only reachable via the native binding.
Level of scrutiny
Medium-high. ConsoleObject.rs is a hot path for every console.log, and the change introduces two design choices a maintainer should ratify: (1) once any default option is mutated, all subsequent global-console output goes through the slower JS util.inspect path instead of the native formatter — reasonable for Node compat but a deliberate perf/fidelity trade; (2) util.inspect.defaultOptions is now observably a Proxy (util.types.isProxy returns true), which the author consciously accepted over per-key accessors. Neither is a bug, but both are the kind of API-shape decision REVIEW.md flags for maintainer agreement.
Other factors
This PR went through three rounds of bot review; each finding (missing timeLog sibling, --console-depth dropped by fallback, then --console-depth leaking into util.inspect defaults) was fixed with a targeted commit and a covering test. All inline threads are resolved. The test suite exercises depth/maxArrayLength/numericSeparator, Object.defineProperty, the setter form, console.dir per-call overrides, console.timeLog, console.group indentation on the fallback, sealed/identity invariants, and inherited-receiver writes. Strong-ref lifecycle is handled in both VM deinit sites. I checked JSValue::to_slice — it converts via to_utf8(), so writing the formatted bytes handles non-Latin1 content. Given the scope and the two design calls above, deferring rather than auto-approving.
This branch has not been deployed
2 participants
Problem
The global
console.log/warn/error/diruse Bun's native formatter, which has no knowledge ofutil.inspect.defaultOptions. Settingdepth,maxArrayLength,numericSeparator, etc. correctly changedutil.inspect,util.format('%O'), andnew console.Console(stream)output, but left globalconsoleoutput untouched, so Node's documented process-wide knob was a silent no-op on the one console most code actually uses.{ a: [Object] }[ 1, 2, ... 4 more items ]1_234_5671234567Cause
message_with_type_and_level_insrc/jsc/ConsoleObject.rsbuilds itsFormatOptionsfrom compiled-in defaults (plus--console-depth) and hands them to the nativeFormatter. Nothing on that path readsinspectDefaultOptions, and the native formatter does not implement most of the option keys anyway.Fix
util.inspect.defaultOptionsnow returns aProxyover the sealed defaults object. The first write installs a pair of JS formatters on the VM (formatWithOptionsfor log-style calls,inspectforconsole.dir); once installed,message_with_type_and_level_routes argument formatting through them instead of the native printer, so every documented option applies. The fast native path is kept for the common case wheredefaultOptionsis never touched. State is per-VM, so workers are isolated.Verification
New tests fail on released Bun (depth/maxArrayLength/numericSeparator ignored) and pass with this change; output now matches Node byte-for-byte for the cases above.
Fixes #12762
[review] gate passed · iteration 1 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 4 passed · 0 rejected · iteration 1
evidence per changed file