Skip to content

console: route global console through process.stdout/stderr, native table index column, _rawDebug (+4 tests) - #35380

Closed
cirospaciari wants to merge 2 commits into
claude/callback-throw-uncaughtfrom
claude/node-v26-sweep-b
Closed

console: route global console through process.stdout/stderr, native table index column, _rawDebug (+4 tests)#35380
cirospaciari wants to merge 2 commits into
claude/callback-throw-uncaughtfrom
claude/node-v26-sweep-b

Conversation

@cirospaciari

@cirospaciari cirospaciari commented Jul 24, 2026

Copy link
Copy Markdown
Member

Stacked on #34660 — review that first.

What this does

Two things, both in the console output path.

1. The global console now writes through process.stdout / process.stderr. Bun's global console formats natively and writes straight to its own buffered writer, so replacing process.stdout.write had no effect on console.log — the write went to the fd regardless. Node performs every console write through the stream, and Node's own test harness (test/common/hijackstdio.js) is built entirely on that. This was the single blocker shared by a group of upstream console tests.

2. console.table's native printer labels its index column. It used a blank space where Node writes (index), or (iteration index) for a Map, a Set, or one of their iterators. The column is also left-justified now, like every other cell and like Node's cli_table.js.

Plus two smaller fixes found while working the same path:

  • process._rawDebug was a no-op stub (Process_stubEmptyFunction). It now formats its arguments and writes to fd 2 directly, bypassing process.stderr the way Node does, so it still reports when that stream is broken or replaced.
  • console.assert(false, msg) printed just msg. Node prefixes Assertion failed: .

How the routing works, and what it costs

console.log is a hot path and the native printer exists for speed, so the fast path had to stay free.

  • A process-wide flag flips the first time a process.stdout or process.stderr object is constructed. A program that never touches either — the common case, and the one the native printer exists for — pays exactly one relaxed atomic load per console call and then takes the pre-existing code path unchanged.
  • Once a stream exists, the check is a getDirect walk up its prototype chain compared against the write recorded when the stream was built. getDirect never invokes an accessor, a Proxy trap or any other user code, so the console write path still needs no throw scope (the Rust caller could not discharge one — see Process__emitMessageEvent). Anything exotic in the chain yields no direct slot, compares unequal, and routes through the ordinary JS path, which is the safe direction.
  • Only when the comparison fails does the message get buffered and handed to the replacement, in a single write call, matching Node's kWriteToConsole. The native writer is flushed first so the two sinks cannot interleave out of order.

Measured with perf stat -e instructions:u, taking the slope between N=100k and N=200k console.log calls so process startup cancels out. Five interleaved runs per configuration, same binary directory, debug+ASAN build (bun bd), stdout to /dev/null:

configuration instructions / console.log
baseline (this branch's parent) 248,932
this branch, process.stdout never touched 251,523
this branch, process.stdout touched, write not replaced 251,496

The two "this branch" rows are indistinguishable (0.01% apart), which is the number that matters for the gate: the getDirect walk is below the noise floor. The ~1% against baseline is not the gate — the fast path adds one atomic load — it is the refactor that moved the message body into a write_message helper, which is a real call at -O0 and inlines under optimization. Caveat stated plainly: a debug+ASAN build spends ~249k instructions per console.log, so this harness cannot resolve anything smaller than about 1%; I did not build an LTO release binary to tighten it.

Tests

Vendored verbatim from nodejs/node at tag v26.3.0:

file why it passes now
parallel/test-console-count.js console.count writes through the replaced process.stdout.write
parallel/test-process-raw-debug.js process._rawDebug is implemented

Both fail on this branch's parent. test-process-raw-debug.js reports as a silent pass under the campaign's append-a-throw canary, and that is a false positive: the file's top-level switch ends in return child() / return parent(), so an appended statement is unreachable in both branches. Verified real two other ways — it exits 1 on the parent build and exits 1 with a throw injected inside child().

New Bun-side tests in test/js/node/console/console.test.ts, both failing on the parent build:

  • the global console writes through a replaced process.stdout.write / process.stderr.write, across log, error, warn and count
  • console.assert prefixes Assertion failed:

Also in this diff

  • test/js/bun/console/__snapshots__/*.snap and the inline expectations in console-table.test.ts are regenerated for the new index column. Bun.inspect.table shares the native printer, so its output changes too; docs/runtime/utils.mdx is updated to what the code now prints. This is a deliberate change to a public Bun API's output — flagging it explicitly in case you would rather Bun.inspect.table keep the blank header, in which case the printer needs a flag.
  • Per campaign policy, three tests vendored in the first commit of this PR are dropped: test-eslint-prefer-abort-signal-abort.js (skips permanently — tools/eslint is not vendored in Bun) and the two Amaro-gated compile-cache TypeScript tests (process.config.variables.node_use_amaro can never be true in Bun). All three passed only by skipping.

Verified

  • Each vendored file run one process per file the way CI runs them (--config=bunfig.node-test.toml, BUN_DEBUG_QUIET_LOGS=1, node:test files under bun test). Campaign preflight reports PREFLIGHT OK.
  • All 93 already-vendored test-console-*, test-process-*, test-util-inspect*, test-util-format*, test-stdout-*, test-stderr-* and test-repl-* files re-run individually: 93 pass, 0 fail.
  • bun bd test test/js/node/console/ test/js/bun/console/: 85 pass, 1 skip, 0 fail.
  • test/js/node/process/ has 7 pre-existing failures; identical on the parent build.
  • test/expectations.txt untouched.

Still blocked, for the record

The routing fix does not by itself land the rest of the hijackstdio-based tests. Each now gets past the interception and fails on something else:

  • test-console-diagnostics-channels.js — the hijack callback now fires; needs console.log/info/debug/warn/error to publish to diagnostics_channel before formatting.
  • test-console.js — needs console._times exposed as a Map, and console.log's object formatting to match util.inspect exactly (Bun prints {\n slashes: "\\\\",\n} where Node prints { slashes: '\\\\' }).
  • test-console-stdio-setters.js — needs the native console to read console._stdout / console._stderr rather than process.stdout / process.stderr. Bun already exposes those as custom accessors on the console object; the native path does not consult them.
  • test-common.js — unrelated to console: Bun's adapted test/common/index.js does not implement Node's leaked-global detection, so the leakedGlobal.js fixture child exits 0.
  • test-internal-errors.js (--expose-internals plus internal/errors.E) and test-repl-tab-complete-buffer.js (repl.start) are unrelated to this path.

console.time / timeEnd / timeLog still write natively. That path does not use the console's writer at all — it goes through Output::print_error — and it writes to stderr where Node writes to stdout. Both want fixing together, separately from this change.

test-compile-cache-permission-allowed.js
test-compile-cache-typescript-commonjs.js
test-compile-cache-typescript-strip-sourcemaps.js
test-eslint-prefer-abort-signal-abort.js

Verbatim copies of the upstream v26.3.0 files. Each was run one process
per file with bunfig.node-test.toml and exits 0 with no runtime changes.
Three fixes to Console#table (the `node:console` Console class, not the
native global console.table):

- Cells are left-justified. Node changed cli_table.js from centering to
  left-justifying, so every table Bun printed had the wrong padding.
- Map and Set iterators are previewed instead of consumed as opaque
  iterables, so `console.table(map.entries())` renders separate Key and
  Values columns like Node. internal/util/inspect already had a
  previewEntries helper; it is now exported and wired up.
- `isBuffer` was destructured off the `node:buffer` module namespace,
  where it does not exist, so any table containing a nested object threw
  "isBuffer is not a function".

Vendors the upstream v26.3.0 test-console-table.js, which now passes, and
updates console-table-iterators.test.ts to the layout real Node prints.
@robobun

robobun commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator
Updated 9:37 PM PT - Jul 23rd, 2026

@cirospaciari, your commit e7718d3 is still building in Build #79287, but has 1 failures so far (All Failures):

  • 📦 Binary size — 4 over 0.50 MB
  • targetthis build canary: main #79256
    sizeΔ
    bun-darwin-aarch6457.83 MB57.42 MB+421.5 KB
    bun-darwin-x6463.19 MB62.79 MB+406.4 KB
    bun-linux-aarch6470.99 MB70.30 MB+707.4 KB
    bun-linux-x6472.50 MB71.80 MB+721.6 KB
    bun-linux-aarch64-musl64.51 MB64.20 MB+320.0 KB
    bun-linux-x64-musl66.62 MB66.29 MB+336.0 KB
    bun-linux-aarch64-android78.34 MB77.78 MB+577.1 KB
    bun-linux-x64-android80.51 MB79.98 MB+544.3 KB
    bun-freebsd-x6482.76 MB82.43 MB+336.1 KB
    bun-freebsd-aarch6484.53 MB84.14 MB+400.1 KB
    bun-windows-x6479.94 MB79.54 MB+410.0 KB
    bun-windows-aarch6470.56 MB70.19 MB+373.5 KB

    Add [skip size check] to the commit message if this increase is intentional.

@github-actions

Copy link
Copy Markdown
Contributor

Found 1 issue this PR may fix:

  1. Bun formats differently the index column when using table from node:console #32614 - Reports that Bun's node:console table() formats the index column differently from Node (centered/compact vs left-aligned); the renderRow left-justification fix directly addresses this

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #32614

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. console: left-align Console#table cells to match Node #33453 - Fixes the same left-alignment bug in renderRow in ConsoleObject.ts and updates the same snapshot test file
  2. node:console: fix Console#table throwing on object cells, Maps and Sets #33452 - Fixes the same isBuffer import bug and Map/Set handling in ConsoleObject.ts

🤖 Generated with Claude Code

@cirospaciari

Copy link
Copy Markdown
Member Author

Superseded by #35391, which combines this branch with the other five stacked on claude/callback-throw-uncaught so there is one review surface instead of six. Every source change and every vendored test from this branch is carried over unchanged; #35391's description lists the merge conflicts that needed a decision and links back here for the rationale.

The branch claude/node-v26-sweep-b is not deleted.

@cirospaciari cirospaciari changed the title Fix Console#table layout and vendor 5 Node v26.3.0 tests Route global console output through process.stdout/stderr; fix console.table index column Jul 24, 2026
@cirospaciari cirospaciari changed the title Route global console output through process.stdout/stderr; fix console.table index column console: route global console through process.stdout/stderr, native table index column, _rawDebug (+4 tests) Jul 24, 2026
@cirospaciari

Copy link
Copy Markdown
Member Author

Follow-up on the consolidation: parallel/test-compile-cache-permission-allowed.js is removed on #35391, with the reasoning recorded in that PR's description.

Short version: it was a false pass, not a regression. On this branch Bun ignored --permission and both --allow-fs-* flags, so the child ran with no sandbox and the compile-cache assertions were the only thing exercised — the file asserted "Bun passes a permission-model test" while Bun has no permission model. #35375 making --permission exit 1 as unsupported did not break it, it exposed that it never covered anything.

Not permanent: a permission-model branch is in flight on the same base (flag surface, process.permission, fs/net/child enforcement). Once that lands this file becomes genuinely passable and should be re-added by whoever owns that work.

Your other three tests — test-console-count.js, test-console-table.js, test-process-raw-debug.js — are on #35391 and passing.

@cirospaciari

Copy link
Copy Markdown
Member Author

🤖 From the combined PR #35391: parallel/test-process-raw-debug.js, which your branch vendored, fails on both Windows lanes:

AssertionError: Expected values to be strictly equal:
+ actual   'I can still debug!\n'
- expected 'I can still debug!\r\n'
  at test-process-raw-debug.js:50

The test asserts os.EOL, so on Windows it wants CRLF; process._rawDebug emits a bare LF. Linux lanes pass. Either _rawDebug needs to use the platform line ending on Windows, or the file is not portable and should come out.

Nothing else from your branch is red — test-console-count.js, test-console-table.js and the Console#table fixes are all green. Also: the console→process.stdout routing was suspected of causing a broad set of failures across bake/dev, fuzzy-wuzzy and in-process-cron. It was not. Those were all the async_hooks executeCallImpl validator abort, and they went green once that was fixed. Your routing change is exonerated.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants