child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED

[robobun]

Repro

import { spawn } from "node:child_process";
const sleep = (ms) => new Promise((r) => setTimeout(r, ms));

const c = spawn("sh", ["-c", "exec 0<&-; sleep 1.5"], { stdio: ["pipe", "ignore", "ignore"] });
await sleep(400);
let errEv = "NO-ERROR-EVENT";
c.stdin.on("error", (e) => (errEv = e.code));
let cb1, cb2;
c.stdin.write(Buffer.alloc(65536, 0x41), (e) => (cb1 = e ? e.code : "success"));
await sleep(300);
console.log({ cb1, errEv, destroyed: c.stdin.destroyed, writable: c.stdin.writable });
const r2 = c.stdin.write("more-bytes", (e) => (cb2 = e ? e.code : "success"));
await sleep(300);
console.log({ r2, cb2 });
c.kill("SIGKILL");

	node v26.3.0	bun 1.4.0	bun (this PR)
cb1 / errEv	EPIPE / EPIPE	EPIPE / no event	EPIPE / EPIPE
destroyed / writable	true / false	false / true	true / false
r2 / cb2	false / ERR_STREAM_DESTROYED	true / success	false / ERR_STREAM_DESTROYED

A producer that doesn't attach a callback to every single write never learns the pipe broke; every write after the first failure vanishes into a dead pipe while reporting success.

Cause

child.stdin is a WriteStream on the FileSink fast path (writableFromFileSink). Its .write() override writeFast in src/js/internal/fs/streams.ts bypasses the Writable state machine and, on sink rejection, called cb(err) but only this.destroy(err) when no callback was provided. Node's onwriteError calls the callback and errorOrDestroy(stream, er) unconditionally; the callback is additive, not a replacement for the 'error' event.

Because the stream was never destroyed, the next writeFast call went straight to the (now-ended) sink again, which returned synchronously, and the success callback fired.

Fix

• writeFast / underscoreWriteFast: route every sink write error through errorOrDestroy() regardless of whether a callback was supplied.
• writeFast: bail to Writable.prototype.write when the state is destroyed (in addition to ending), so a write after the first failure surfaces ERR_STREAM_DESTROYED instead of reaching the sink.
• Use two-arg then(onFulfilled, onRejected) so a throw from the fulfillment handler isn't caught as a write failure.

Relation to #33485

#33485 fixes the same !hasCallback guard in writeFast for the process.stdout-on-hung-up-tty case. It does not add the state.destroyed bail, so with that change alone the second write to a child's closed stdin still returns true with a success callback (verified against its branch). This PR is a superset on the streams.ts side and adds child_process-specific coverage; either can be closed in favor of the other once merged.

Verification

bun bd test test/js/node/child_process/child_process.test.ts -t "stdin write failure"

Fails (timeout waiting for 'error') on 1.4.0, passes with the fix. Related suites all green: fs.test.ts -t WriteStream, tty.test.ts, regression/issue/1632.test.ts, process-stdio.test.ts, child-process-stdio.test.js, and node test/parallel test-file-write-stream*, test-fs-write-stream-*, test-child-process-std*, test-process-external-stdio-close*, test-console-log-stdio-broken-dest.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/child_process/child_process.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6ecf1af-c231-44cc-aa6e-3ed7fac36b09

📥 Commits

Reviewing files that changed from the base of the PR and between be77b65 and 465228d.

📒 Files selected for processing (2)

• src/js/internal/fs/streams.ts
• test/js/node/child_process/child_process.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 3:33 PM PT - Jul 15th, 2026}

❌ @robobun, your commit 465228d has 2 failures in Build #73385 (All Failures):

• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/parallel/test-net-connect-memleak.js - code 1 on 🐧 3.23 x64
• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34268

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

bun-34268 --bun

[robobun]

Status: reproduced with USE_SYSTEM_BUN=1 bun test test/js/node/child_process/child_process.test.ts -t "stdin write failure" (times out waiting for 'error'); passes with bun bd test.

CI build 73385 finished. The new test passed on every lane. test/js/node/child_process/child_process.test.ts is not in the failure set.

Remaining red is unrelated to this JS-only streams change:

• test-worker-message-port-transfer-terminate.js on x64-asan: JSC C++ exception-scope assertion during worker terminate, tracked as ASAN CI: JSC assertion in JSObject::getOwnPropertyDescriptor during worker terminate (test-worker-message-port-transfer-terminate) #34095.
• test-net-connect-memleak.js on alpine x64/x64-baseline: pre-existing GC-timing flake.
• 30205.test.ts napi LeakSanitizer, jsc-stress, fetch-backpressure, fetch.stream, bun-run-dir, spawn.test.ts, test-repl-close.js: all retried green or known-flaky on main.

Diff is ready for review.

[github-actions]

Found 2 issues this PR may fix:

1. process EPIPE error event not emitted #7251 - EPIPE error event not emitted on process.stdout when piped to head — same bug class where FileSink fast-path WriteStream doesn't emit 'error' on write failure
2. FileSink.write incoherencies #12194 - FileSink.write incoherencies when writing to Bun.spawn stdin pipe — PR's changes to writeFast/underscoreWriteFast error handling directly address inconsistent error propagation from the sink

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

Fixes #7251
Fixes #12194

🤖 Generated with Claude Code

[robobun]

Neither of the suggested issues is closed by this change:

• process EPIPE error event not emitted #7251's repro uses console.log, which goes through the native console writer and never touches the WriteStream fast path edited here. console: surface EPIPE from console.log on process.stdout #30635 tracks that half.
• FileSink.write incoherencies #12194 is about the raw FileSink.write() return-value semantics on Bun.spawn (promise vs number, value exceeding input size). This PR only changes the node:fs WriteStream wrapper around the sink, not the sink itself.

[github-actions]

This PR may be a duplicate of:

1. stdio: emit 'error' on a failed write even when a write callback is passed #33485 - Fixes the same writeFast/underscoreWriteFast functions in src/js/internal/fs/streams.ts to emit 'error' on a failed write even when a write callback is passed
2. fs: fail WriteStream fast-path write() with ERR_STREAM_DESTROYED after destroy #34267 - Adds the same state.destroyed check in writeFast to fail subsequent writes with ERR_STREAM_DESTROYED after stream destruction

🤖 Generated with Claude Code

[robobun]

Re the duplicate detector:

• stdio: emit 'error' on a failed write even when a write callback is passed #33485 fixes the !hasCallback guard in writeFast (pty EIO case) but not the state.destroyed bail, so the second write on a dead child.stdin still reports success there.
• fs: fail WriteStream fast-path write() with ERR_STREAM_DESTROYED after destroy #34267 adds the state.destroyed bail but not the unconditional errorOrDestroy, so the first EPIPE write still doesn't emit 'error' or destroy the stream when a callback is passed.

This PR is the union of both on the streams.ts side (and also covers underscoreWriteFast), so it handles the full sequence in the report: failing write with a callback → 'error' emitted + stream destroyed → next write → ERR_STREAM_DESTROYED. Either of the other two can be closed once this lands, or the other way around with the missing half folded in.

[claude]

I didn't find any bugs, but this changes error semantics on the shared FileSink fast path used by process.stdout/process.stderr and tty.WriteStream (not just child_process stdin), and the PR explicitly asks for a coordination call vs #33485 — worth a human look.

Checked: errorOrDestroy usage matches the existing pattern at streams.ts:311 and Node's onwriteError (callback + destroy are additive). The state.destroyed bail correctly routes post-failure writes to Writable.prototype.write for ERR_STREAM_DESTROYED. The two-arg .then() switch prevents a throwing 'drain' listener from being misclassified as a write failure. Test awaits real conditions (stdout readiness handshake, error event) with failure paths wired to reject, and cleanup is in finally.

Extended reasoning...

Overview

The PR modifies src/js/internal/fs/streams.ts — specifically the writeFast and underscoreWriteFast functions on the FileSink fast path — so that a sink write error always routes through errorOrDestroy() (matching Node's onwriteError), rather than only when no callback was supplied. It also adds state.destroyed to the bail-to-Writable.prototype.write guard so subsequent writes surface ERR_STREAM_DESTROYED, and switches .then().catch() to two-arg .then() so a throw from the fulfillment handler (e.g. a user 'drain' listener) is not misinterpreted as a write failure. A new test in child_process.test.ts covers the EPIPE-on-closed-stdin case end to end.

Security risks

None. This is stream error-propagation plumbing; no auth, crypto, or untrusted-input parsing is involved.

Level of scrutiny

Moderate-to-high. While the diff is small (~30 lines net) and the mechanism is well-argued against Node's onwriteError, the FileSink fast path is shared by process.stdout/process.stderr (ProcessObjectInternals.ts:55), tty.WriteStream (tty.ts:109), and child_process stdin (child_process.ts:1207). Changing when these streams get destroyed on write error is user-observable well beyond the PR title's scope. The author did run the relevant suites (tty.test.ts, process-stdio.test.ts, test-console-log-stdio-broken-dest, etc.), which is reassuring, but a maintainer should confirm the process.stdout/tty behavioral shift is intended.

Other factors

The PR description explicitly flags overlap with #33485 ("either can be closed in favor of the other once merged") — that is a maintainer coordination decision, not something I should resolve by auto-approving. CI (#73385) was still building at review time. The new test is well-constructed: it awaits a stdout readiness handshake instead of sleeping, wires error/exit to reject, uses Promise.withResolvers for the callback/event assertions, and kills the child in finally.

[robobun]

On the shared-path concern: the three consumers diverge exactly where they should.

• process.stdout/stderr on a pipe/socket are created with autoClose: false (so autoDestroy is false). errorOrDestroy takes the non-destroying branch: it sets errored and emits 'error' but leaves state.destroyed false, so the next write still takes the fast path. Verified by running stdio: emit 'error' on a failed write even when a write callback is passed #33485's tty hangup tests against this branch: both pass (["cb:EIO","error:EIO"], and unhandled → exit 1).
• process.stdout/stderr file-backed have autoDestroy flipped back to true and the _destroy → _undestroy() override, so destroy(err) runs and immediately undestroys; state.destroyed is false again before the next write.
• child.stdin has autoDestroy: true and the stock WriteStream.prototype._destroy, so errorOrDestroy → destroy(err) sticks and the next write routes to ERR_STREAM_DESTROYED.

Also ran #34267's write-after-close/write-after-exit tests against this branch: both pass. So this change satisfies the test coverage from all three PRs.