console: write timeEnd/timeLog to stdout instead of stderr - #32423
console: write timeEnd/timeLog to stdout instead of stderr#32423robobun wants to merge 3 commits into
Conversation
Node.js routes console.timeEnd() and console.timeLog() output through console.log, which writes to stdout. Bun was writing to stderr, so redirecting stdout did not capture the timing output. Switch the elapsed-time print, the label print, and the extra-argument formatter in timeLog to their stdout equivalents. Fixes #12031
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
Changesconsole.timeEnd/timeLog stdout redirect
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Updated 3:33 PM PT - Jun 16th, 2026
❌ @robobun, your commit 76c5882 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32423That installs a local version of the PR into your bun-32423 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — mechanical swap from stderr to stdout helpers, matches Node behavior, with regression tests.
Extended reasoning...
Overview
This PR fixes #12031 by routing console.timeEnd() and console.timeLog() output to stdout instead of stderr, matching Node.js behavior. The change touches two files:
src/jsc/ConsoleObject.rs(~10 lines): swapsOutput::print_elapsed→print_elapsed_stdout,print_error/print_errorln→print/println,error_writer()→writer(), andenable_ansi_colors_stderr()→enable_ansi_colors_stdout(). Each replacement is the direct stdout counterpart of the original helper, all of which already exist inbun_core::outputandConsoleObject.test/js/web/console/console-timeLog.test.ts: flips existing assertions from stderr to stdout and adds two new regression tests verifyingtimeEnd/timeLogwrite to stdout with stderr empty.
Security risks
None. This only changes which file descriptor receives diagnostic timing output. No user input handling, auth, crypto, or permission logic is involved.
Level of scrutiny
Low. The change is a one-to-one substitution of well-established, symmetric output helpers — I verified each stdout counterpart exists and behaves as the mirror of the stderr version (print_elapsed_stdout uses pretty! vs pretty_error!; print/println target Destination::Stdout; ConsoleObject::writer() is the stdout buffered writer). console.count() in the same file already follows this stdout pattern, so this brings timeEnd/timeLog in line with it.
Other factors
- No CODEOWNERS entries cover the modified files.
- The bug hunting system found no issues.
- The PR includes before/after verification (
USE_SYSTEM_BUN=1fails,bun bdpasses) and the test updates assert both that stdout receives the output and that stderr is empty, which would catch any partial regression. - The author explicitly scoped out the larger "route through
console.logoverride" question, which is reasonable for this fix.
|
Diff is ready and tests pass locally ( CI builds 62914 and 62926 both failed on infrastructure before any build job started: every lane reports |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/web/console/console-timeLog.test.ts`:
- Line 64: The test "should log to console correctly" should be marked as
concurrent to align with the other tests in the file and improve test suite
parallelism. Change the test declaration from it("should log to console
correctly", async () => { to it.concurrent("should log to console correctly",
async () => { by adding .concurrent to the it function call.
🪄 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: cf375811-f67e-4e03-bc50-7557a1044a19
📒 Files selected for processing (2)
src/jsc/ConsoleObject.rstest/js/web/console/console-timeLog.test.ts
There was a problem hiding this comment.
LGTM — mechanical stderr→stdout swap for console.timeEnd/timeLog with matching test updates.
Extended reasoning...
Overview
This PR fixes #12031 by changing console.timeEnd() and console.timeLog() to write to stdout instead of stderr, matching Node.js behavior. The change is in src/jsc/ConsoleObject.rs (two functions, ~6 lines each) and swaps stderr-targeting helpers for their existing stdout equivalents: print_elapsed → print_elapsed_stdout, print_error/print_errorln → print/println, error_writer() → writer(), and enable_ansi_colors_stderr() → enable_ansi_colors_stdout(). All target functions already exist in bun_core::output and on ConsoleObject. Tests in console-timeLog.test.ts are updated to assert output on stdout with stderr empty, plus two new regression tests linked to the issue.
Security risks
None. This only changes which file descriptor timing output is written to. No auth, crypto, parsing, or untrusted-input handling is touched.
Level of scrutiny
Low. The change is mechanical (1:1 substitution of stderr helpers with stdout helpers), localized to two adjacent functions, and follows the same pattern already used by console.count() in the same file. The PR description verified before/after test results.
Other factors
- No prior human reviewer comments to address.
- Test coverage is good: existing tests were flipped to assert stdout/stderr correctly, and new tests cover the default label, named labels, and
timeLogwith extra args. - I confirmed
print_elapsed_stdout,enable_ansi_colors_stdout, andConsoleObject::writer()all exist with the expected signatures. - The PR explicitly scopes out the larger question of routing through a JS-overridable
console.log, which is reasonable.
There was a problem hiding this comment.
LGTM — mechanical stderr→stdout swap for console.timeEnd/timeLog to match Node, with tests.
Extended reasoning...
Overview
This PR changes Bun__ConsoleObject__timeEnd and Bun__ConsoleObject__timeLog in src/jsc/ConsoleObject.rs to write timing output to stdout instead of stderr, matching Node.js behavior (fixes #12031). The source change is 8 lines: each stderr helper is swapped for its existing stdout counterpart (print_elapsed → print_elapsed_stdout, print_errorln/print_error → println/print, error_writer() → writer(), enable_ansi_colors_stderr() → enable_ansi_colors_stdout()). I verified all target functions already exist in bun_core::output and on ConsoleObject with the expected stdout semantics. The test file is updated to assert stderr is empty and stdout contains the timing lines, and adds three new spawn-based tests covering the default label and timeLog with extra args.
Security risks
None. This only changes which file descriptor receives formatted timing strings; no input parsing, auth, or privilege boundaries are touched.
Level of scrutiny
Low. The change is a 1:1 mechanical substitution between symmetric helper pairs that already exist, mirrors how console.count() in the same file already routes to stdout, and is covered by 5 subprocess tests that pass on the new build and fail on system Bun (demonstrating the behavior change). The one bot nitpick (mark the integration test it.concurrent) was addressed in 76c5882.
Other factors
No CODEOWNERS entries cover these paths, no outstanding human review comments, and the only CI failures noted in the thread were infrastructure (agent image not found) rather than test failures. The PR description explicitly scopes out the larger question of routing through a JS-overridable console.log, which is reasonable for this fix.
|
Superseded by #34240, which also switches to Node's |
1 participant
Fixes #12031.
Repro
Node routes
console.timeEnd()andconsole.timeLog()throughconsole.log, so the timing output goes to stdout. Bun wrote it to stderr, which meant redirecting stdout did not capture the timing output and it leaked past pipes.Cause
Bun__ConsoleObject__timeEndandBun__ConsoleObject__timeLoginsrc/jsc/ConsoleObject.rsused the stderr helpers:Output::print_elapsed(which targets stderr viapretty_error!),Output::print_error/print_errorln, the console'serror_writer(), andenable_ansi_colors_stderr().console.count()in the same file already goes to stdout.Fix
Switch to the stdout equivalents:
print_elapsed_stdout,print/println,writer(), andenable_ansi_colors_stdout().print_elapsed_stdoutalready existed inbun_core::output.Verification
test/js/web/console/console-timeLog.test.tsupdated to assert output lands on stdout with stderr empty, coveringtimeEnd(default, empty, and named labels) andtimeLog(with and without extra args).USE_SYSTEM_BUN=1 bun test test/js/web/console/console-timeLog.test.ts→ 5 failbun bd test test/js/web/console/console-timeLog.test.ts→ 5 passThe reporter also noted that Node routes through
console.logitself (so overridingconsole.logintercepts the output). Bun's global console is native and does not call back intoconsole.log; that is a larger design question left out of scope here.