Skip to content

fix: console.trace should go to stderr - #20020

Closed
ma-ts wants to merge 8 commits into
oven-sh:mainfrom
ma-ts:fix/19952
Closed

ma-ts wants to merge 8 commits into
oven-sh:mainfrom
ma-ts:fix/19952

Conversation

@ma-ts

@ma-ts ma-ts commented May 29, 2025

Copy link
Copy Markdown

What does this PR do?

This PR fixes issue #19952 by making console.trace() go to stderr. Furthermore, it improves node.js compatibility by adding the Trace: statement before the printing of the given variables.

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

Added automated tests to validate this, also manually validated the results:

echo "console.trace('hello');" | node - >/dev/null
Trace: hello
    at [stdin]:1:9
    at runScriptInThisContext (node:internal/vm:209:10)
    at node:internal/process/execution:449:12
    at [stdin]-wrapper:6:24
    at runScriptInContext (node:internal/process/execution:447:60)
    at evalFunction (node:internal/process/execution:87:30)
    at evalScript (node:internal/process/execution:99:3)
    at node:internal/main/eval_stdin:53:5
    at Socket.<anonymous> (node:internal/process/execution:200:5)
    at Socket.emit (node:events:530:35)
echo "console.trace('hello');" | bun - >/dev/null

echo "console.trace('hello');" | bun-debug - >/dev/null
bun-debug(43560,0x1f0475f00) malloc: nano zone abandoned due to inability to reserve vm space.
Trace: hello
at /Users/pnlmw585/dev/bun-tests/[stdin]:1:9
at loadAndEvaluateModule (7:44)
at asyncFunctionResume (9:85)
at promiseReactionJobWithoutPromiseUnwrapAsyncContext (14:20)
at promiseReactionJob (31:60)



<!-- **For code changes, please include automated tests**. Feel free to uncomment the line below -->

<!-- I wrote automated tests -->

<!-- If JavaScript/TypeScript modules or builtins changed:

- [x] I included a test for the new code, or existing tests cover it
- [] I ran my tests locally and they pass (`bun-debug test test-file-name.test`)

-->

<!-- If Zig files changed:

- [ ] I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
- [x] I included a test for the new code, or an existing test covers it
- [ ] JSValue used outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
- [ ] I wrote TypeScript/JavaScript tests and they pass locally (`bun-debug test test-file-name.test`)
-->

<!-- If new methods, getters, or setters were added to a publicly exposed class:

- [ ] I added TypeScript types for the new methods, getters, or setters
-->

<!-- If dependencies in tests changed:

- [ ] I made sure that specific versions of dependencies are used instead of ranged or tagged versions
-->

<!-- If a new builtin ESM/CJS module was added:

- [ ] I updated Aliases in `module_loader.zig` to include the new module
- [ ] I added a test that imports the module
- [ ] I added a test that require() the module
-->

@190n 190n linked an issue May 29, 2025 that may be closed by this pull request
190n
190n previously approved these changes May 29, 2025

@190n 190n 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, let's see if the tests pass.

Comment on lines +66 to +75
test("console.trace should output to stderr", async () => {
const [out, outValue] = writable();
const [err, errValue] = writable();
const c = new Console({ stdout: out, stderr: err });
c.trace("hello world!");
out.end();
err.end();
expect(await outValue()).toBe("");
expect(await errValue()).toStartWith("Trace: hello world!\n");
});

@RiskyMH RiskyMH May 31, 2025 •

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.

https://buildkite.com/bun/bun/builds/17607#annotation-test/js/node/console/console.test.ts

bun test v1.2.16-canary.1 (82a91eea)
 
✓ console.Console > global instanceof Console
✓ console.Console > new Console instanceof Console [1.99ms]
 
✓ console.Console > it can write to a stream [0.83ms]
✓ console.Console > can enable colors [0.94ms]
✓ console.Console > stderr and stdout are separate [0.48ms]
 
69 |     const c = new Console({ stdout: out, stderr: err });
70 |     c.trace("hello world!");
71 |     out.end();
72 |     err.end();
73 |     expect(await outValue()).toBe("");
74 |     expect(await errValue()).toStartWith("Trace: hello world!\n");
                                  ^
error: expect(received).toStartWith(expected)
 
Expected to start with: "Trace: hello world!\n"
Received: "Error\n    at <anonymous> (/opt/homebrew/etc/buildkite-agent/builds/macOS-14-aarch64-1/bun/bun/test/js/node/console/console.test.ts:70:7)\n    at <anonymous> (/opt/homebrew/etc/buildkite-agent/builds/macOS-14-aarch64-1/bun/bun/test/js/node/console/console.test.ts:66:49)\n"
 
      at <anonymous> (/opt/homebrew/etc/buildkite-agent/builds/macOS-14-aarch64-1/bun/bun/test/js/node/console/console.test.ts:74:30)
✗ console.Console > console.trace should output to stderr [0.79ms]
✓ console._stdout [0.34ms]
✓ console._stderr [0.43ms]
 
 
 7 pass
 1 fail
 12 expect() calls
Ran 8 tests across 1 files. [23.00ms]

@zackradisic

Copy link
Copy Markdown
Contributor

Thanks @ma-ts for the fix!

I went ahead and fixed the failing console.test.ts test as this goes through a separate codepath unrelated to the code in ConsoleObject.zig file.

Will merge once CI runs!

@robobun

robobun commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR, and sorry it sat for so long. The stderr routing is now being fixed as part of #37128, which reworks console output as a whole. This branch patches src/bun.js/ConsoleObject.zig, which no longer exists (the console now lives in src/jsc/ConsoleObject.rs), so it cannot be rebased as is. The Trace: header, which #37128 does not add, is carried by #32638 against the current source. Closing this one in favor of those two.

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.

console.trace() goes to stdout instead of stderr

5 participants