node:worker_threads: rebind console output sink instead of replacing the global console - #34347
node:worker_threads: rebind console output sink instead of replacing the global console#34347robobun wants to merge 6 commits into
Conversation
…the global console Inside a node:worker_threads worker, the global console was being replaced wholesale with a node:console instance so that console.log output would flow through the port-backed process.stdout/stderr. That swap loses Bun's documented console APIs (console.write, console[Symbol.asyncIterator], console.Console) and silently switches formatting from Bun's inspector to Node's util.inspect, so the same value printed differently on the worker and the main thread of one process. Rebind the native ConsoleObject's output sink instead: when a worker sets up its port-backed stdio, it now calls a native setter that stashes the Writable streams on the per-VM ConsoleObject. message_with_type_and_level and console.count format into a buffer and hand the bytes to stream.write() when an override is present, so Bun's formatter and console surface survive while output still reaches worker.stdout/stderr. As a side effect of extracting the format body, console.error() with no arguments now writes its newline to stderr instead of stdout, matching Node.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughChangesWorker-thread stdio now routes the existing Bun console through JS stream overrides. Worker console routing
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 6:10 AM PT - Jul 16th, 2026
❌ @autofix-ci[bot], your commit f07dd4c has 2 failures in
🧪 To try this PR locally: bunx bun-pr 34347That installs a local version of the PR into your bun-34347 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
… clippy safety-comment placement
…lorless; clarify worker stdio 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: 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/js/node/worker_threads.ts`:
- Around line 478-479: Update the stream override logic around setOutputStreams
so each argument uses the captured stdout or stderr stream only when that
corresponding value exists; otherwise preserve the native process stream/fd path
by leaving the uncaptured side unoverridden. Avoid gating both streams on the
combined stdout || stderr condition.
In `@test/js/node/worker_threads/worker-console-bun-api.test.ts`:
- Around line 76-78: Update the worker stream completion logic around the stdout
and stderr handlers so captured output is reported only after both streams have
emitted their end events. Track completion for each stream, then perform the
existing process.stdout.write with the combined out and err values once both are
finished.
🪄 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: 8a921279-bfcd-4e73-a830-f387022429f3
📒 Files selected for processing (7)
src/codegen/generate-js2native.tssrc/js/node/worker_threads.tssrc/jsc/ConsoleObject.rssrc/jsc/web_worker.rssrc/runtime/dispatch_js2native.rssrc/runtime/server/RequestContext.rstest/js/node/worker_threads/worker-console-bun-api.test.ts
… both worker streams in test
There was a problem hiding this comment.
No bugs found in this revision, but this is a substantive native change — new Strong-rooted JS stream handles on the per-VM ConsoleObject, JS re-entry from console paths that previously only wrote fds, and worker-shutdown ordering for releasing those handles — so it's worth a human look.
What was reviewed:
- Strong-handle lifecycle:
stdout_override/stderr_overrideare released inclear_output_streams()fromweb_worker.rsshutdown step 2, beforeteardownJSCVM— ordering looks correct. - JS re-entry:
count/timeEnd/timeLogand the override branch ofmessage_with_type_and_level_scope thevm_console_mutborrow before callingwrite_to_js_stream; the snapshottedJSValuestays rooted by the still-held Strong across formatting. write_message_bodyextraction: the empty-argsMessageType::Lognewline now useswriter(soconsole.error()with no args goes to stderr);write_tracenow honors the caller'senable_colors; the sole external caller inRequestContext.rswas updated.- All prior review threads (timeEnd/timeLog capture, trace ANSI, per-stream override gating, both-stream-end wait in the test, trace stdout routing) confirmed applied in the current diff.
Extended reasoning...
Overview
The PR replaces globalThis.console = new Console(...) in node:worker_threads workers with a native output-sink rebinding on the per-VM ConsoleObject. It touches:
src/jsc/ConsoleObject.rs(~260 lines): two newStrongOptionalfields, aset_output_streamsjs2native entry point, awrite_to_js_streamhelper that invokes.write()on a JS Writable, extraction ofwrite_message_bodyfrommessage_with_type_and_level_, and override routing added tocount/timeEnd/timeLog.src/jsc/web_worker.rs: releases the Strong handles in shutdown step 2, before JSC VM teardown.src/js/node/worker_threads.ts: calls the new native setter instead of swapping the global console.src/codegen/generate-js2native.ts+src/runtime/dispatch_js2native.rs: wiring for the new$newRustFunctioncall site.src/runtime/server/RequestContext.rs: updated for thewrite_tracesignature change.- New test file exercising API surface, formatting parity, and
worker.stdout/stderrcapture includingcount/timeLog/timeEnd.
Security risks
None identified. No auth/crypto/permissions surface. The new set_output_streams binding is only reachable from the worker_threads builtin ($newRustFunction), not user code. write_to_js_stream does a property get + call on process.stdout/stderr (builtin-constructed port-backed Writables), not on arbitrary user objects.
Level of scrutiny
This warrants human review. It is not a mechanical change: it adds GC-rooted state to a per-VM object, introduces JS re-entry from console output paths that previously wrote fds directly, extracts and reshapes the body of message_with_type_and_level_ (a hot path hit by every console.log), and adds a step to worker shutdown ordering. The PR has already been through three rounds of review feedback (timeEnd/timeLog leaking to fd, write_trace ANSI, per-stream override gating, test stream-end race, trace stream routing), each of which was a real gap — which is evidence the change is subtle enough to merit a careful maintainer pass on the final shape.
Other factors
- The bug-hunting system found nothing on this final revision.
- All inline review threads are resolved and I verified each fix landed in the diff (5348bde routes
timeEnd/timeLogthroughstderr_override; e85b532 threadsenable_colorsintowrite_trace; e9a400f gates each override on its own capture flag and waits for both streamendevents in the test). - The
let _ = write_to_js_stream(...)pattern incount/timeEnd/timeLogdiscards aJsResult, so a throwing.write()would leave a pending exception on a void-returning host call — in practice the port-backed Writable'swritedoesn't throw, so I'm noting it rather than flagging it as a bug. - Tests pass on both debug/ASAN and release per the PR's evidence block; the new test asserts capture of
log/error/count/timeLog/timeEndand formatting parity with the main thread.
|
CI build #73859 is complete. The new test The two remaining red tests are unrelated to this diff:
Ready for review. |
|
Closing: #37128 reworks the stdio sinks and lists this PR as superseded (it takes the same approach: If something here turns out not to be covered once #37128 lands, this can be reopened. |
1 participant
What
Inside a
node:worker_threadsworker,console.writeandconsole[Symbol.asyncIterator](both documented Bun APIs) were gone, and console formatting silently switched to Node'sutil.inspectstyle while the main thread of the same process still printed Bun's.Bun.inspect()inside the worker was unchanged, proving it is the console binding and not a VM-wide inspect change. The webWorkerglobal was unaffected; onlynode:worker_threadsdiverged.Cause
setupWorkerStdio()insrc/js/node/worker_threads.tsdid:The guard is effectively unconditional (the Worker constructor always creates the stdout/stderr channels), and the swap replaces Bun's native console with a
node:consoleinstance, losing the Bun-specific surface and switching toutil.formatWithOptionsformatting. Introduced by #31216.Fix
Rebind the native
ConsoleObject's output sink instead of replacing the global.node:worker_threadsnow calls a native setter that stashes the port-backedprocess.stdout/process.stderrasStronghandles on the per-VMConsoleObject. When an override is present,message_with_type_and_level(coveringlog/info/debug/warn/error/dir/dirxml/table/trace/assert/group*) andconsole.countformat into a buffer with Bun's formatter and hand the bytes tostream.write(), so the Bun console surface and formatting are preserved while output still flows throughworker.stdout/worker.stderr.The overrides are released before JSC VM teardown in the worker shutdown path so the
Stronghandles never outlive the heap.As a side effect of extracting the format body,
console.error()with no arguments now writes its newline to stderr instead of stdout, matching Node.Verification
Fails on
main(console APIsundefined,'k' => 'v'formatting captured), passes with this change. Node'stest-worker-stdio*.jsandtest-worker-console-listeners.jscontinue to pass.[review] gate passed · iteration 0 · 6 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file