Skip to content

process: implement process._rawDebug#30937

Open
robobun wants to merge 3 commits into
mainfrom
farm/50751022/fix-rawdebug
Open

process: implement process._rawDebug#30937
robobun wants to merge 3 commits into
mainfrom
farm/50751022/fix-rawdebug

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented May 17, 2026

Fixes #30934

Repro

// repro.cjs
process.on('beforeExit', () => process._rawDebug('hi'));
$ node repro.cjs      # stderr
hi
$ bun repro.cjs       # (nothing)

Cause

process._rawDebug was wired to Process_stubEmptyFunction in the process property table in src/jsc/bindings/BunProcess.cpp — it returned undefined and never wrote anything. The beforeExit handler in the issue fires correctly, but its call into _rawDebug dropped on the floor.

Node's process._rawDebug(...args) is an undocumented-but-depended-on API: util.format(...args) written synchronously to fd 2. Used by Node's own test suite, by pino/thread-stream, and by libraries that need to log from contexts where the event loop or the normal stderr stream isn't reliable (during shutdown, from workers, etc).

Fix

  • New getRawDebug builtin in src/js/builtins/ProcessObjectInternals.ts: returns a closure that does fs.writeSync(2, util.format(...args) + '\n').
  • New constructRawDebug PropertyCallback in BunProcess.cpp invokes that builtin once, caches the result (matching Node's identity: process._rawDebug === process._rawDebug).
  • Flip the _rawDebug row in processObjectTable from Process_stubEmptyFunction to constructRawDebug PropertyCallback.

Matches Node byte-for-byte on the cases the test exercises — plain args, printf-style %d/%s/%j, nested objects/arrays, empty call — and is synchronous (writes complete before subsequent microtasks run).

Verification

git stash push -- src/
bun bd test test/js/node/process/process.test.js -t 'process._rawDebug'
  → 0 pass, 4 fail  (no output written; stub)

git stash pop
bun bd test test/js/node/process/process.test.js -t 'process._rawDebug'
  → 4 pass, 0 fail

Also removed _rawDebug from the undefinedStubs list in the same file since it's no longer a stub. Existing process-onBeforeExit tests still pass.

process._rawDebug was stubbed to a no-op (Process_stubEmptyFunction).
Node writes util.format(...args) + '\n' synchronously to fd 2 from
this path; libraries like pino/thread-stream and Node's own test
suite depend on it. Calling it from a beforeExit handler (or anywhere
else) silently dropped output.

Fixes #30934
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 17, 2026

Updated 1:27 PM PT - May 17th, 2026

@robobun, your commit 6bc28e3715901342582eb2024da2a9ff6c47bdfc passed in Build #55553! 🎉


🧪   To try this PR locally:

bunx bun-pr 30937

That installs a local version of the PR into your bun-30937 executable, so you can run:

bun-30937 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 666c72f2-a2db-4394-abca-95d5c72faecf

📥 Commits

Reviewing files that changed from the base of the PR and between bcc83d3 and 6bc28e3.

📒 Files selected for processing (1)
  • test/js/node/process/process.test.js

Walkthrough

Implements process._rawDebug: a TypeScript logger that formats args with util.format and synchronously writes to stderr, wired through a C++ binding and validated by spawned-process tests.

Changes

process._rawDebug feature

Layer / File(s) Summary
TypeScript logger implementation
src/js/builtins/ProcessObjectInternals.ts
getRawDebug() exports a _rawDebug function that formats arguments using node:util and synchronously writes the result plus a newline to file descriptor 2 via node:fs.writeSync.
C++ binding and property wiring
src/jsc/bindings/BunProcess.cpp
constructRawDebug performs a profiled call to the internal code generator, clears and reports exceptions to the event loop, and the Process property table maps _rawDebug to this new callback instead of the empty stub.
Test coverage for _rawDebug
test/js/node/process/process.test.js
Removes _rawDebug from undefined stubs and adds a dedicated test suite that spawns Bun child processes to verify stderr writes, printf-style argument formatting, newline behavior (including empty-arg calls), and invocation during shutdown via beforeExit.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: implementing the process._rawDebug function.
Description check ✅ Passed The PR description thoroughly covers both required sections from the template: what the PR does (implementation details and reasoning) and how it was verified (test results and manual verification steps).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/process/process.test.js`:
- Around line 735-792: Add a regression test that asserts the stable identity of
process._rawDebug by spawning a child Bun process which evaluates and prints
(e.g. via console.log) the boolean expression process._rawDebug ===
process._rawDebug, then assert the child stdout equals "true\n" and exitCode is
0; place this new it(...) inside the existing "describe('process._rawDebug',
...)" block alongside the other tests so it verifies the identity contract that
motivated the PropertyCallback wiring.
🪄 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: eecd6686-2135-4020-b329-5aa3dcddc447

📥 Commits

Reviewing files that changed from the base of the PR and between 172afa5 and e12b09f.

📒 Files selected for processing (3)
  • src/js/builtins/ProcessObjectInternals.ts
  • src/jsc/bindings/BunProcess.cpp
  • test/js/node/process/process.test.js

Comment thread test/js/node/process/process.test.js
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
test/js/node/process/process.test.js (1)

735-784: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a regression test for _rawDebug stable identity.

The new block validates output semantics, but it still doesn’t cover the identity contract this PR introduces (process._rawDebug === process._rawDebug).

Suggested test addition
 describe("process._rawDebug", () => {
+  it("has stable identity", async () => {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "-e", `process.exit(process._rawDebug === process._rawDebug ? 0 : 1);`],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+    expect(stderr).toBe("");
+    expect(exitCode).toBe(0);
+  });
+
   it("writes util.format output + newline to stderr synchronously", async () => {
🤖 Prompt for 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.

In `@test/js/node/process/process.test.js` around lines 735 - 784, Add a
regression test in the existing describe("process._rawDebug") block that
verifies the stable identity invariant by spawning a process that evaluates and
prints the strict equality of process._rawDebug to itself (e.g. run bunExe with
-e "console.log(process._rawDebug === process._rawDebug)"), capture the
appropriate stdio (stdout or stderr consistent with other tests), and assert the
output is "true\n" and exitCode is 0; place this new it(...) alongside the other
tests so it exercises the same spawning pattern and uses the same bunExe(),
bunEnv, and proc.exited symbols.
🤖 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.

Duplicate comments:
In `@test/js/node/process/process.test.js`:
- Around line 735-784: Add a regression test in the existing
describe("process._rawDebug") block that verifies the stable identity invariant
by spawning a process that evaluates and prints the strict equality of
process._rawDebug to itself (e.g. run bunExe with -e
"console.log(process._rawDebug === process._rawDebug)"), capture the appropriate
stdio (stdout or stderr consistent with other tests), and assert the output is
"true\n" and exitCode is 0; place this new it(...) alongside the other tests so
it exercises the same spawning pattern and uses the same bunExe(), bunEnv, and
proc.exited symbols.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba7118bb-383c-42d7-b862-d29d42c1638e

📥 Commits

Reviewing files that changed from the base of the PR and between e12b09f and bcc83d3.

📒 Files selected for processing (1)
  • test/js/node/process/process.test.js

Addresses coderabbitai review feedback: verify that the PropertyCallback
wiring caches the lazily-constructed function so process._rawDebug ===
process._rawDebug, matching Node.
Comment on lines +471 to +473
return function _rawDebug(...args) {
writeSync(2, format.$apply(null, args) + "\n");
};
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.

🟡 Minor Node-compat divergence: Node's _rawDebug is backed by native FPrintF(stderr, ...), which silently no-ops when fd 2 is closed or the pipe is broken — but fs.writeSync(2, ...) will throw EBADF/EPIPE in those cases. Since this is meant as a last-resort logger for shutdown/crash paths where stderr may already be in a degraded state, consider wrapping the writeSync in try { ... } catch {} so a failed debug write can't mask the original error or change exit behavior.

Extended reasoning...

What the bug is

Node implements process._rawDebug in lib/internal/process/per_thread.js as a thin wrapper around the native binding _rawDebug (RawDebug() in src/node_process_methods.cc), which calls FPrintF(stderr, ...) followed by fflush(stderr). C stdio writes to a closed or broken FILE* set the stream's error indicator and return a negative value, but never propagate a JavaScript exception — the call simply does nothing visible from JS.

Bun's new implementation at src/js/builtins/ProcessObjectInternals.ts:471-473 uses fs.writeSync(2, ...):

return function _rawDebug(...args) {
  writeSync(2, format.$apply(null, args) + "\n");
};

fs.writeSync throws on any underlying write(2) error. So if fd 2 has been closed (EBADF) or stderr is a pipe whose reader has exited (EPIPE, since SIGPIPE is ignored), _rawDebug will throw where Node would silently continue.

Code path that triggers it

  1. Something puts stderr into a degraded state — e.g. user code calls fs.closeSync(2), or the process was launched as bun script.js 2>&1 | head and head exits after reading enough lines.
  2. A crash/shutdown path (or library like pino/thread-stream, per the PR description) calls process._rawDebug('diagnostic message').
  3. writeSync(2, ...) invokes write(2), which returns -1 with errno = EBADF or EPIPE.
  4. Bun's writeSync converts that into a thrown JS error, which propagates out of _rawDebug.

Why nothing prevents it

There is no try/catch around the writeSync call, and the constructRawDebug C++ wrapper in BunProcess.cpp only catches exceptions thrown while constructing the closure (the one-time getRawDebug() call), not exceptions thrown when the returned _rawDebug function is later invoked.

Step-by-step proof

const fs = require('fs');
fs.closeSync(2);
process._rawDebug('x');
console.log('survived');
  • Node: FPrintF(stderr, "x\n") writes to a closed fd, fprintf returns an error indicator, nothing is thrown into JS. 'survived' is printed to stdout. Exit code 0.
  • Bun (this PR): writeSync(2, "x\n")write(2, ...) = -1, errno=EBADF → throws Error: EBADF: bad file descriptor, write. The uncaught exception sets exit code 1 and 'survived' is never printed.

The EPIPE variant is more realistic in practice: bun -e "for(let i=0;i<1e5;i++) process._rawDebug(i)" 2>&1 | head -1 — once head exits, the next writeSync(2, ...) throws EPIPE in Bun but is silently dropped in Node.

Impact

The PR's own comment describes _rawDebug as something that "works even while the event loop is blocked or during shutdown" — i.e., it's a last-resort logger meant to be called from already-failing code paths. If it throws, it can:

  • Mask the original error being reported (the EBADF/EPIPE becomes the visible failure instead of whatever was being logged).
  • Change exit codes (an otherwise-clean shutdown that tries to log from beforeExit/exit now exits non-zero).
  • Re-enter uncaughtException handlers in surprising ways.

That said, this is a rare edge case: most programs never close fd 2, and stderr's reader dying mid-process is uncommon. The previous Bun behavior was a no-op stub, so this PR is still a strict improvement. And _rawDebug is an undocumented API.

Fix

One-line change — swallow write errors to match Node's C-stdio semantics:

return function _rawDebug(...args) {
  try { writeSync(2, format.$apply(null, args) + "\n"); } catch {}
};

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

process.beforeExit event is not emitted during normal shutdown

1 participant