FileSink: return 0 from write() and undefined from flush() after end()

[robobun]

Repro

const writer = Bun.file("/tmp/out.txt").writer();
await writer.write("hello");   // → 5
await writer.end();
writer.write("world");         // → true   (boolean, typed number | Promise<number>)
writer.flush();                // → true

FileSink.write() and flush() after end() return the boolean true, against the declared number | Promise<number> return type.

Cause

FileSink::end() / end_from_js() only set self.done = true on the Err and Pending flush results. On the synchronous Done / Wrote branches (the common case for a regular file) they call writer.end() on the underlying IOWriter but leave self.done false.

A subsequent write() then skips the done early-return and calls writer.write() on the already-ended IOWriter, which returns WriteResult::Done(0). to_result(Done(0)) → Writable::Done → JSValue::TRUE.

Fix

• Set self.done = true on every terminal branch of FileSink::end() and end_from_js().
• Make write() / write_latin1() / write_utf16() return Writable::Owned(0) instead of Writable::Done when the sink is done. Owned(0) converts to the number 0, which is what the HTTP/H3/network sinks already return in the same situation.

With done now set, flush() after end() hits the existing self.done guard in flush_from_js and returns undefined instead of true.

No new exceptions are thrown and post-end writes are still silently ignored. Only the return values change, to match the documented number | Promise<number> type and the other sinks. (An earlier revision of this PR made write-after-end throw ERR_STREAM_WRITE_AFTER_END; that was removed per review as an unnecessary breaking change.)

Verification

New tests in test/js/bun/util/filesink.test.ts:

• write() after end() returns 0 and flush() returns undefined; the post-end bytes do not appear in the file.
• proc.stdin.write() after the subprocess exits returns 0.

Both fail on 1.4.0 (get true).

Related: the Bun.serve streaming-body-error 0\r\n\r\n terminator half of the same fuzzer finding is handled in #32842.

[coderabbitai]

Walkthrough

FileSink now distinguishes user-initiated end() from peer-close, exposes that state through write_after_end_error(), and returns ERR_STREAM_WRITE_AFTER_END only for user-ended sinks. writeFast() now catches synchronous throws from fileSink.write(). Tests cover the updated end, peer-close, and stdin callback behavior.

Changes

FileSink write-after-end enforcement

Layer / File(s)	Summary
JsSinkType write_after_end_error hook src/runtime/webcore/Sink.rs	Adds write_after_end_error(&self) -> Option<ErrorCode> to JsSinkType and inserts an early guard in js_write that throws when the hook returns an ErrorCode.
FileSink ended_by_user state and transitions src/runtime/webcore/FileSink.rs	Adds ended_by_user tracking to FileSink, initializes and resets it, updates end() and end_from_js() state transitions, changes done writes to return Owned(0), and gates write_after_end_error on ended_by_user.
writeFast synchronous throw handling src/js/internal/fs/streams.ts	Wraps fileSink.write(data) in try/catch in writeFast(), scheduling synchronous errors on the next tick or destroying the stream when no callback is present.
FileSink and stdin tests test/js/bun/util/filesink.test.ts	Updates the flush-after-end regression test and adds coverage for write-after-end data integrity, peer-close behavior, and node:child_process stdin callback delivery.

Possibly related PRs

• oven-sh/bun#31135: Adjusts the same end()/end_from_js() flush outcome handling and done state transitions in FileSink.rs.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title matches a real part of the change: FileSink behavior after end().
Description check	✅ Passed	The description covers the repro, cause, fix, and verification, so it satisfies the template content.

---

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

[robobun]

^{Updated 5:05 AM PT - Jun 28th, 2026}

❌ @robobun, your commit a7960e3 has some failures in Build #66207 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 32857

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

bun-32857 --bun

[coderabbitai]

Actionable comments posted: 6

🤖 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/internal/fs/streams.ts`:
- Around line 674-705: The fast-path in writeFast() is bypassing
Writable.prototype.write(), so fast write-after-end failures need to follow the
same normal Writable error handling as src/js/internal/streams/writable.ts.
Update the promise rejection and synchronous throw branches in writeFast() to
route errors through the shared error helper used by Writable (instead of
calling cb(err) and conditionally destroy()), so callback-based writes after
end() still emit the standard error event and mark the stream errored
consistently.

In `@src/runtime/webcore/Sink.rs`:
- Around line 875-879: The write-after-end path in Sink::write_after_end_error
is overriding the canonical ERR_STREAM_WRITE_AFTER_END text with a sink-specific
sentence; update the throw message to use the standard “write after end” payload
expected by ErrorCode.cpp and stream tests. Keep the existing error flow in
Sink.rs, but replace the formatted T::NAME-specific wording so the emitted error
matches the canonical stream error exactly.

In `@test/js/bun/util/filesink.test.ts`:
- Around line 374-378: The test currently pulls in child_process with a runtime
require inside the test body, which should be moved to a module-scope import for
consistency with repo test style. Update the filesink.test.ts fixture so
child_process is imported at the top level and the test case uses that imported
symbol when calling spawn, keeping the test focused on the bunExe() behavior
rather than dynamic require().
- Around line 312-314: The new filesink test currently creates its temporary
path with tmpdirSync, which bypasses the harness cleanup flow. Update the test
in filesink.write-after-end to use tempDir from harness with using dir so the
directory is cleaned up automatically, and keep the rest of the test setup the
same around Bun.file(...).writer().
- Around line 333-336: The post-end assertions in filesink tests are too weak
for the new contract. Update the relevant checks around writer.flush() after
end() and the peer-close write() cases to assert the exact return values from
the existing test helpers, using the concrete symbols involved in these cases
rather than broad negated/type checks. Make the expectations verify undefined
for flush() after end and 0 for peer-close write(), and apply the same stronger
assertions in the other mentioned test block as well.
- Around line 380-399: In the file sink test around the p.stdin close/write
flow, the waiters closed and cbFired can hang because only the success paths are
wired. Update the Promise.withResolvers setup so the p.stdin.on("error"),
p.on("error"), and p.on("exit") handlers reject both the closed and callback
waiters on any failure path. Keep the existing p.stdin.on("close") and write
callback/sync-throw assertions, but ensure all child/process/stdin error cases
propagate immediately instead of timing out.

🪄 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: 7e7d51a4-7d45-4fb2-9336-6b8641687de9

📥 Commits

Reviewing files that changed from the base of the PR and between df92f8f and 157df7e.

📒 Files selected for processing (4)

• src/js/internal/fs/streams.ts
• src/runtime/webcore/FileSink.rs
• src/runtime/webcore/Sink.rs
• test/js/bun/util/filesink.test.ts

[robobun]

CI status: no test failures. On #66207 (the current head, a7960e39) there are zero error-style annotations; the two red lanes are both the darwin-aarch64 agent pool:

• darwin-26-aarch64-test-bun: fails before running any tests on four consecutive builds (#65586, #65607, #65637, #66207), across at least two different agents:

  Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'.
  Refusing to continue with a partial download (would silently fall back to the wrong binary).
  

• darwin-14-aarch64-test-bun: both job instances expired waiting for an agent (never ran).

The only flaky warning on #66207 is napi.test.ts on Windows, which passed on retry and is unrelated.

filesink.test.ts passes on every Windows, Linux, and macOS lane that actually ran it. Diff is down to a single commit per @Jarred-Sumner's review (throw removed; only the true → 0/undefined return-type fix remains).

[Jarred-Sumner]

Changing this in FileSink is an unnecessary breaking change. Don't do that.

[robobun]

@Jarred-Sumner Understood, removed. I force-pushed the branch down to a single commit (a7960e39) that drops the throw, the ended_by_user field, the Sink.rs hook, and all the fs/streams.ts changes.

What is left is just the return-type mismatch I hit along the way: writer.write(x) after end() returns the boolean true (against the declared number | Promise<number>) because end_from_js() never set self.done on the synchronous Done/Wrote paths, so the call fell through to the already-ended IOWriter. With done set correctly it now returns 0 (matching HTTPResponseSink/NetworkSink) and flush() returns undefined. No exceptions, post-end writes are still silently ignored.

If that is also more than you want to change, say so and I'll close this.

[robobun]

The same behavior was reported again independently (write to subprocess.stdin after the child exits, and after .end(), both return the boolean true). The reporter's minimum ask is exactly what is left in this PR: never a boolean, return the number.

I split out the part that does not touch FileSink into #33511: writeFast() replaces Writable.prototype.write on the fast path and so skips its ERR_STREAM_WRITE_AFTER_END / ERR_STREAM_DESTROYED checks, which is why process.stdout.end() followed by process.stdout.write(x) still writes x and reports success. That one is node-compat only, no FileSink change.

This PR is still just the true -> 0 / undefined return-value fix. Happy to close it if you would rather leave FileSink alone entirely.