From 77066e2c12ade707ad5d6869f767293f3de51416 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 10:40:38 +0000 Subject: [PATCH 01/27] console.write: handle every failed write when given several arguments Every write to a pipe that is already broken fails on the spot, with a rejected Promise of its own. console.write() kept only the last Promise a write returned, so with several arguments a script that awaited it and caught the error still died of the unhandled rejection of an earlier argument's write. It now returns a Promise over every distinct one. The comments on the two POSIX-only tests say why they are: on Windows a write to a pipe whose reader is alive is accepted whole and returns its byte count, so there is no pending Promise for them to be about. --- src/js/builtins/ConsoleObject.ts | 21 ++++++++---- test/js/bun/console/console-write.test.ts | 42 +++++++++++++++++++++++ test/js/bun/util/filesink.test.ts | 3 ++ 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index b1eff1c6366c..78f7486ed48e 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -129,23 +129,30 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined }, in $putByIdDirectPrivate(this, "writer", writer); } - // A backed-up writer (FileSink) returns a Promise instead of a count: one Promise, the same for every write made - // while it is backed up, of the bytes those writes added. The caller gets a Promise of the total then, because - // awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung up) arrives. + // A write() returns a Promise instead of a count in two cases. A backed-up writer (FileSink) returns its one + // outstanding Promise, the same for every write made while it is backed up, of the bytes those writes added. A + // write that fails on the spot (the pipe is already broken) returns a rejected Promise of its own. The caller + // gets a Promise of the total over every distinct one: awaiting it waits for the drain and is where a write + // error (EPIPE) arrives, and none is left for an unhandled rejection. var wrote = 0; - var pending: Promise | undefined; + var pending: Promise[] | undefined; const count = $argumentCount(); var i = 0; do { const result = writer.write(arguments[i]); if (typeof result === "number") wrote += result; - else pending = result; + else if (pending === undefined) pending = [result]; + // The shared Promise comes back from consecutive writes, so the last one is the only possible repeat. + else if (pending[pending.length - 1] !== result) pending.push(result); } while (++i < count); writer.flush(true); if (pending === undefined) return wrote; - if (wrote === 0) return pending; - return pending.then(n => wrote + n); + if (pending.length === 1 && wrote === 0) return pending[0]; + return Promise.all(pending).then(counts => { + for (const n of counts) wrote += n; + return wrote; + }); } // This is the `console.Console` constructor. It is mostly copied from Node. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 035df8e9a2e7..f8e2845a50ce 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -103,6 +103,9 @@ console.error(JSON.stringify({ one, two })); // The Promise a backed-up console.write() returns is where its write error arrives: a script that // awaits it can handle a reader that hung up, instead of dying with an unhandled rejection. +// +// Not on Windows: a write to a pipe whose reader is alive is accepted whole there and returns its byte +// count, so whether this script sees a Promise at all depends on whether the reader is already gone. test.skipIf(isWindows).each([ ["one argument", ""], ["several arguments", ', "tail"'], @@ -134,3 +137,42 @@ try { expect(stderr).toBe("caught EPIPE\n"); expect(exitCode).toBe(0); }); + +// Every write to a pipe that is already broken fails on the spot, with a rejected Promise of its own. +// console.write() kept only the last one, so a script that awaited it and caught the error still died of the +// unhandled rejection of an earlier argument's write. +// +// The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and the writes +// are large enough to go to the pipe at once instead of being buffered. +test("an awaited console.write with several arguments handles every failed write", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +process.on("unhandledRejection", e => { + console.error("unhandledRejection " + e?.code); +}); +require("node:fs").readSync(0, Buffer.alloc(1)); +try { + await console.write(Buffer.alloc(1024 * 1024, "a").toString(), Buffer.alloc(1024 * 1024, "b").toString()); + console.error("resolved"); +} catch (e) { + console.error("caught " + e.code); +} +`, + ], + env: bunEnv, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + + await proc.stdout.cancel(); + proc.stdin.write("x"); + await proc.stdin.end(); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toBe("caught EPIPE\n"); + expect(exitCode).toBe(0); +}); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index bcc36b370c53..a72ac5422b17 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1153,6 +1153,9 @@ it("start() with invalid options throws instead of silently ignoring them", asyn // // The child blocks in a synchronous read of stdin between its two writes, so no event-loop turn can tell the sink // about the hang-up first: the second write() is the one that finds out, with the first still pending. +// +// Not on Windows: a write to a pipe whose reader is alive is accepted whole there and returns its byte count, so +// there is no pending promise for a later write to fail beside. it.skipIf(isWindows)("a write() that fails while another is pending rejects the pending promise once", async () => { await using proc = Bun.spawn({ cmd: [ From fbf7c3a6e18d282f7c73c6cc7b1ac793789cd93b Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 10:47:55 +0000 Subject: [PATCH 02/27] console.write: only a Promise is pending; intrinsics for the array and the chain The sink's write() has three results: a count, a Promise, and `true` from a sink that is finished. Anything that was not a number was treated as a Promise, so `true` would have been added to the total as 1. `$arrayPush`, `.$then` and an index loop instead of Array.prototype.push, Promise.prototype.then and the array iterator, which a script can replace, as BundlerPlugin.ts does for the same shape. --- src/js/builtins/ConsoleObject.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index 78f7486ed48e..67543186924a 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -129,11 +129,11 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined }, in $putByIdDirectPrivate(this, "writer", writer); } - // A write() returns a Promise instead of a count in two cases. A backed-up writer (FileSink) returns its one + // The sink's write() returns a count, or a Promise of one in two cases. A backed-up sink returns its one // outstanding Promise, the same for every write made while it is backed up, of the bytes those writes added. A // write that fails on the spot (the pipe is already broken) returns a rejected Promise of its own. The caller - // gets a Promise of the total over every distinct one: awaiting it waits for the drain and is where a write - // error (EPIPE) arrives, and none is left for an unhandled rejection. + // gets a Promise over every distinct one: awaiting it waits for the drain and is where a write error (EPIPE) + // arrives, with none left over for an unhandled rejection. (A finished sink returns `true`: nothing written.) var wrote = 0; var pending: Promise[] | undefined; const count = $argumentCount(); @@ -141,16 +141,18 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined }, in do { const result = writer.write(arguments[i]); if (typeof result === "number") wrote += result; - else if (pending === undefined) pending = [result]; - // The shared Promise comes back from consecutive writes, so the last one is the only possible repeat. - else if (pending[pending.length - 1] !== result) pending.push(result); + else if ($isPromise(result)) { + if (pending === undefined) pending = [result]; + // The shared Promise comes back from consecutive writes, so the last one is the only possible repeat. + else if (pending[pending.length - 1] !== result) $arrayPush(pending, result); + } } while (++i < count); writer.flush(true); if (pending === undefined) return wrote; if (pending.length === 1 && wrote === 0) return pending[0]; - return Promise.all(pending).then(counts => { - for (const n of counts) wrote += n; + return Promise.all(pending).$then(counts => { + for (var j = 0; j < counts.length; j++) wrote += counts[j]; return wrote; }); } From 7072113fdab550fc47c81bd3a947093cbce88746 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 10:53:53 +0000 Subject: [PATCH 03/27] console.write: sum the pending Promises without Promise.all `Promise.all` is a property a script can replace. The Promises are chained with `.$then` instead, and every one after the first is marked handled: its reaction is attached only once the earlier ones have fulfilled, and if one of those rejects, that is the error the caller gets. The single-Promise case falls out of the same code. --- src/js/builtins/ConsoleObject.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index 67543186924a..035f717ae9df 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -150,11 +150,16 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined }, in writer.flush(true); if (pending === undefined) return wrote; - if (pending.length === 1 && wrote === 0) return pending[0]; - return Promise.all(pending).$then(counts => { - for (var j = 0; j < counts.length; j++) wrote += counts[j]; - return wrote; - }); + // Summed by hand: `Promise.all` is a property a script can replace. Every Promise after the first is marked + // handled, because its reaction is only attached once the ones before it have fulfilled; if one of those + // rejects instead, that is the error the caller gets, and this one's is not reported a second time. + var total = pending[0]; + for (var j = 1; j < pending.length; j++) { + const next = pending[j]; + $pokePromiseAsHandled(next); + total = total.$then(sum => next.$then(n => sum + n)); + } + return wrote === 0 ? total : total.$then(n => wrote + n); } // This is the `console.Console` constructor. It is mostly copied from Node. From 5c4dae0a9974fdab26570e2c37c3f08564bdd66f Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:03:30 +0000 Subject: [PATCH 04/27] console.write: write every argument natively The builtin called the sink's write() once per argument and had to work out, from the JS values it got back, which Promises were the same one and which were rejections of their own. Inside the sink those are still values: a failed write is an error, a backed-up write is the sink's one pending operation. JSSink::js_write_all writes every argument, flushes once and returns one result. The first failed write ends the call with that error. The counts (a chunk written before the sink backed up, or a small one buffered beside the pending operation) are credited to that operation, so its Promise resolves to the call's total. js_write's argument handling and js_flush's body are now write_value() and flush_value(), shared with it. The builtin makes the writer as before and passes its arguments on. --- src/codegen/generate-js2native.ts | 1 + src/js/builtins/BunBuiltinNames.h | 1 + src/js/builtins/ConsoleObject.ts | 43 +++--------- src/runtime/webcore/FileSink.rs | 6 ++ src/runtime/webcore/Sink.rs | 108 ++++++++++++++++++++++++------ 5 files changed, 106 insertions(+), 53 deletions(-) diff --git a/src/codegen/generate-js2native.ts b/src/codegen/generate-js2native.ts index 8e7168f1f745..9a59572fa8ed 100644 --- a/src/codegen/generate-js2native.ts +++ b/src/codegen/generate-js2native.ts @@ -46,6 +46,7 @@ const sourceFiles = readdirRecursiveWithExclusionsAndExtensionsSync( const rustIdentifierPaths: Record = { "bun.rs": "bun.rs", "ipc.rs": "runtime/ipc_host.rs", + "FileSink.rs": "runtime/webcore/FileSink.rs", "Counters.rs": "jsc/Counters.rs", "FrameworkRouter.rs": "runtime/bake/FrameworkRouter.rs", "JSBundler.rs": "runtime/api/JSBundler.rs", diff --git a/src/js/builtins/BunBuiltinNames.h b/src/js/builtins/BunBuiltinNames.h index 2974058b0b6e..bc6d5762b2fa 100644 --- a/src/js/builtins/BunBuiltinNames.h +++ b/src/js/builtins/BunBuiltinNames.h @@ -197,6 +197,7 @@ using namespace JSC; macro(writable) \ macro(writableType) \ macro(write) \ + macro(writeAll) \ macro(writer) \ macro(written) \ BUN_ADDITIONAL_BUILTIN_NAMES(macro) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index 035f717ae9df..d8e31031f5e9 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -119,7 +119,13 @@ interface ConsoleWriter extends Bun.FileSink { flush(wait?: boolean): number | Promise; } -export function write(this: Console & { $writer: ConsoleWriter | undefined }, input) { +// The signature of a builtin stays on one line: bundle-functions.ts does not parse a parameter list with braces in it. +interface ConsoleWithWriter { + $writer: ConsoleWriter | undefined; + $writeAll: (this: ConsoleWriter, ...chunks: unknown[]) => number | Promise; +} + +export function write(this: ConsoleWithWriter, input) { if (!$isObject(this)) throw $ERR_INVALID_THIS("Console"); var writer = $getByIdDirectPrivate(this, "writer"); @@ -127,39 +133,12 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined }, in var length = $toLength(input?.length ?? 0); writer = Bun.stdout.writer({ highWaterMark: length > 65536 ? length : 65536 }); $putByIdDirectPrivate(this, "writer", writer); + // Writes every argument and flushes once, natively: there a failed write is an error value and a backed-up one + // is the sink's one pending operation, where here they would be a Promise each to tell apart and to handle. + $putByIdDirectPrivate(this, "writeAll", $newRustFunction("FileSink.rs", "consoleWrite", 1)); } - // The sink's write() returns a count, or a Promise of one in two cases. A backed-up sink returns its one - // outstanding Promise, the same for every write made while it is backed up, of the bytes those writes added. A - // write that fails on the spot (the pipe is already broken) returns a rejected Promise of its own. The caller - // gets a Promise over every distinct one: awaiting it waits for the drain and is where a write error (EPIPE) - // arrives, with none left over for an unhandled rejection. (A finished sink returns `true`: nothing written.) - var wrote = 0; - var pending: Promise[] | undefined; - const count = $argumentCount(); - var i = 0; - do { - const result = writer.write(arguments[i]); - if (typeof result === "number") wrote += result; - else if ($isPromise(result)) { - if (pending === undefined) pending = [result]; - // The shared Promise comes back from consecutive writes, so the last one is the only possible repeat. - else if (pending[pending.length - 1] !== result) $arrayPush(pending, result); - } - } while (++i < count); - - writer.flush(true); - if (pending === undefined) return wrote; - // Summed by hand: `Promise.all` is a property a script can replace. Every Promise after the first is marked - // handled, because its reaction is only attached once the ones before it have fulfilled; if one of those - // rejects instead, that is the error the caller gets, and this one's is not reported a second time. - var total = pending[0]; - for (var j = 1; j < pending.length; j++) { - const next = pending[j]; - $pokePromiseAsHandled(next); - total = total.$then(sum => next.$then(n => sum + n)); - } - return wrote === 0 ? total : total.$then(n => wrote + n); + return $getByIdDirectPrivate(this, "writeAll").$apply(writer, arguments); } // This is the `console.Console` constructor. It is mostly copied from Node. diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 004ec81d8baa..d50a77945093 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -1412,6 +1412,12 @@ impl FileSink { // `Sink.JSSink(@This(), "FileSink")` — generic-fn-returning-type → monomorphized type alias. pub(crate) type JSSink = crate::webcore::sink::JSSink; +/// `console.write()` (src/js/builtins/ConsoleObject.ts): `this` is the console's writer, a `Bun.stdout.writer()`. +#[bun_jsc::host_fn] +pub(crate) fn console_write(global: &JSGlobalObject, frame: &CallFrame) -> JsResult { + JSSink::js_write_all(global, frame) +} + crate::impl_js_sink_abi!(FileSink, "FileSink"); // `JsSinkType` impl: routes the codegen `FileSink__*` thunks (via diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index db938743d86d..87a85e520247 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -520,7 +520,16 @@ impl JSSink { ))); } - let arg = frame.argument(0); + Ok(Self::write_value(this, global, frame.argument(0))?.to_js(&cx)) + } + + /// One chunk from JS into the sink: bytes as they are, a string by its encoding. + fn write_value( + this: &mut JSSink, + global: &crate::webcore::jsc::JSGlobalObject, + arg: crate::webcore::jsc::JSValue, + ) -> crate::webcore::jsc::JsResult { + use streams::result::Writable; arg.ensure_still_alive(); let _keep = bun_jsc::EnsureStillAlive(arg); @@ -534,14 +543,11 @@ impl JSSink { if let Some(buffer) = arg.as_array_buffer(global) { let slice = buffer.slice(); if slice.is_empty() { - return Ok(JSValue::js_number(0.0)); + return Ok(Writable::Owned(0)); } // Borrowed view over GC-kept buffer for the duration of the call. let data = bun_ptr::RawSlice::new(slice); - return Ok(this - .sink - .write_bytes(&streams::Result::Temporary(data)) - .to_js(&cx)); + return Ok(this.sink.write_bytes(&streams::Result::Temporary(data))); } if !arg.is_string() { @@ -553,24 +559,75 @@ impl JSSink { let view = arg.to_js_string_view(global)?; if view.is_empty() { - return Ok(JSValue::js_number(0.0)); + return Ok(Writable::Owned(0)); } if view.is_utf16() { let utf16 = view.utf16(); let bytes: &[u8] = bytemuck::cast_slice(utf16); let data = bun_ptr::RawSlice::new(bytes); - return Ok(this - .sink - .write_utf16(&streams::Result::Temporary(data)) - .to_js(&cx)); + return Ok(this.sink.write_utf16(&streams::Result::Temporary(data))); } let data = bun_ptr::RawSlice::new(view.latin1()); - Ok(this - .sink - .write_latin1(&streams::Result::Temporary(data)) - .to_js(&cx)) + Ok(this.sink.write_latin1(&streams::Result::Temporary(data))) + } + + /// `console.write(...chunks)`: every argument written in order, one flush, one result. + /// + /// The sink's results are combined here, where a failed write is still an error value and a backed-up write + /// is still the sink's one pending operation. Turned into JS values one write at a time they are a rejected + /// Promise each, and a Promise the caller has to recognise as one it already holds. + pub(crate) fn js_write_all( + global: &crate::webcore::jsc::JSGlobalObject, + frame: &crate::webcore::jsc::CallFrame, + ) -> crate::webcore::jsc::JsResult { + use crate::webcore::jsc::JSValue; + use streams::result::Writable; + let cx = global.js_thread_of_caller(frame); + bun_core::mark_binding!(); + let Some(this) = Self::get_this(global, frame)? else { + return Ok(JSValue::js_number(0.0)); + }; + + if let Some(err) = this.sink.get_pending_error() { + return Err(global.throw_value(err)); + } + + let mut wrote: u64 = 0; + let mut pending: Option<(*mut streams::WritablePending, JSValue)> = None; + // No argument is one `undefined` chunk, which write_value() rejects as write() does. + for i in 0..frame.arguments_count().max(1) as usize { + match Self::write_value(this, global, frame.argument(i))? { + // Nothing after it can succeed either. The pending operation, if there is one, is given the error + // by the sink itself: this is only reached with nothing pending. + Writable::Err(err) => return Ok(Writable::Err(err).to_js(&cx)), + Writable::Pending(operation) => { + pending = Some((operation, Writable::Pending(operation).to_js(&cx))); + } + Writable::Owned(n) + | Writable::OwnedAndDone(n) + | Writable::Temporary(n) + | Writable::Backpressure(n) => wrote += n, + // A finished sink: nothing written. + Writable::Done => {} + } + } + + let Some((operation, promise)) = pending else { + Self::flush_value(this, global, &cx, true)?; + return Ok(JSValue::from(wrote)); + }; + // The Promise resolves to what the pending operation consumed. The counts are part of this call's total + // too: a chunk written before the sink backed up, or a small one buffered beside the operation. + // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive; nothing + // has run since the write that returned it, so it is still the pending one. + unsafe { + (*operation).consumed += wrote; + (*operation).result = Writable::Owned((*operation).consumed); + } + Self::flush_value(this, global, &cx, true)?; + Ok(promise) } /// `${abi_name}__flush` host-fn body. @@ -579,8 +636,6 @@ impl JSSink { frame: &crate::webcore::jsc::CallFrame, ) -> crate::webcore::jsc::JsResult { let cx = global.js_thread_of_caller(frame); - use crate::webcore::jsc::JSValue; - use bun_sys_jsc::ErrorJsc; bun_core::mark_binding!(); let Some(this) = Self::get_this(global, frame)? else { @@ -591,11 +646,22 @@ impl JSSink { return Err(global.throw_value(err)); } + let wait = frame.arguments_count() > 0 + && frame.argument(0).is_boolean() + && frame.argument(0).as_boolean(); + Self::flush_value(this, global, &cx, wait) + } + + fn flush_value( + this: &mut JSSink, + global: &crate::webcore::jsc::JSGlobalObject, + cx: &bun_jsc::JsThread<'_>, + wait: bool, + ) -> crate::webcore::jsc::JsResult { + use crate::webcore::jsc::JSValue; + use bun_sys_jsc::ErrorJsc; if T::HAS_FLUSH_FROM_JS { - let wait = frame.arguments_count() > 0 - && frame.argument(0).is_boolean() - && frame.argument(0).as_boolean(); - return match this.sink.flush_from_js(&cx, wait) { + return match this.sink.flush_from_js(cx, wait) { sys::Result::Ok(value) => Ok(value), sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), }; From 7288f3a735344ee1dd6a95342f9d87ce085ef1dc Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:08:33 +0000 Subject: [PATCH 05/27] console-write test: check the pipe is broken, and cover mixed argument sizes The child checks with a writer of its own that the pipe really is broken before it calls console.write, so the test cannot pass by the writes simply being queued. A short argument beside a large one is a second shape of the same failure: the short one is buffered, and the flush then threw past the Promise of the large one. --- test/js/bun/console/console-write.test.ts | 25 +++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index f8e2845a50ce..4d793b1e0cbd 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -138,13 +138,19 @@ try { expect(exitCode).toBe(0); }); -// Every write to a pipe that is already broken fails on the spot, with a rejected Promise of its own. -// console.write() kept only the last one, so a script that awaited it and caught the error still died of the -// unhandled rejection of an earlier argument's write. +// Every write to a pipe that is already broken fails on the spot. console.write() called the sink's write() once +// per argument and kept the last Promise it got back, so a script that awaited it and caught the error still died +// of the unhandled rejection of another argument's write (or, with a short last argument, of the flush throwing +// past the Promise it had). // -// The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and the writes -// are large enough to go to the pipe at once instead of being buffered. -test("an awaited console.write with several arguments handles every failed write", async () => { +// The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and checks with +// a writer of its own that the pipe really is broken, so the test cannot pass by the writes simply being queued. +const big = `Buffer.alloc(1024 * 1024, "a").toString()`; +test.each([ + ["two large arguments", `${big}, ${big}`], + ["a large argument, then a short one", `${big}, "\\n"`], + ["a short argument, then a large one", `"\\n", ${big}`], +])("an awaited console.write to a broken pipe fails once: %s", async (_label, args) => { await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -154,8 +160,11 @@ process.on("unhandledRejection", e => { console.error("unhandledRejection " + e?.code); }); require("node:fs").readSync(0, Buffer.alloc(1)); +const probe = Bun.stdout.writer().write(${big}); +console.error("pipe already broken: " + (probe instanceof Promise && Bun.peek.status(probe) === "rejected")); +if (probe instanceof Promise) probe.catch(() => {}); try { - await console.write(Buffer.alloc(1024 * 1024, "a").toString(), Buffer.alloc(1024 * 1024, "b").toString()); + await console.write(${args}); console.error("resolved"); } catch (e) { console.error("caught " + e.code); @@ -173,6 +182,6 @@ try { await proc.stdin.end(); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect(stderr).toBe("caught EPIPE\n"); + expect(stderr).toBe("pipe already broken: true\ncaught EPIPE\n"); expect(exitCode).toBe(0); }); From 10773d4924dace45b8686480edf122c2bfc20998 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:18:54 +0000 Subject: [PATCH 06/27] console.write: crediting the counts never replaces a stored error A write that fails beside a pending operation puts its error in the operation's result and returns the operation. Crediting this call's counts then overwrote that result with the byte total. The Promise still rejected, but only because the flush that follows hits the same broken pipe and stores the error again. The result is now refreshed only when it is the running total. --- src/runtime/webcore/Sink.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index 87a85e520247..d6da5a899b2d 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -622,9 +622,11 @@ impl JSSink { // too: a chunk written before the sink backed up, or a small one buffered beside the operation. // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive; nothing // has run since the write that returned it, so it is still the pending one. - unsafe { - (*operation).consumed += wrote; - (*operation).result = Writable::Owned((*operation).consumed); + let operation = unsafe { &mut *operation }; + operation.consumed += wrote; + // `result` is the running total, unless a write failed beside the operation: then it is that error. + if let Writable::Owned(total) = &mut operation.result { + *total = operation.consumed; } Self::flush_value(this, global, &cx, true)?; Ok(promise) From 33f6390a2cecd4b6c125c9e3681362867adb6312 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:26:16 +0000 Subject: [PATCH 07/27] console.write: a pending operation a later argument settles; flush after a failed write A later argument's write can settle the sink's pending operation on the spot, when the reader has made room and the chunk goes out with what was buffered before it. The counts were then credited to a slot that was no longer pending: this call's total lost them, and the next operation started from them. The loop now checks after every write, and carries a settled operation's value on as a count. A failed write returned without flushing, so a short chunk buffered before it was left for the auto flusher, which marked the sink finished: every later console.write reported 0 bytes and no error. The failed path flushes too; later writes fail with EPIPE as they do after any other failed call. --- src/runtime/webcore/Sink.rs | 57 ++++++++++++++++------- test/js/bun/console/console-write.test.ts | 11 ++++- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index d6da5a899b2d..690987e3aef3 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -595,15 +595,35 @@ impl JSSink { } let mut wrote: u64 = 0; - let mut pending: Option<(*mut streams::WritablePending, JSValue)> = None; + // The sink's pending operation, its Promise, and what that Promise resolves to as of the last write. + let mut pending: Option<(*mut streams::WritablePending, JSValue, u64)> = None; // No argument is one `undefined` chunk, which write_value() rejects as write() does. for i in 0..frame.arguments_count().max(1) as usize { - match Self::write_value(this, global, frame.argument(i))? { + let result = Self::write_value(this, global, frame.argument(i))?; + // A write can settle the pending operation on the spot: the reader made room, and this chunk went out + // with everything buffered before it. Its Promise has resolved to `consumed`, which this call's total + // carries on from as a count. Checked before `to_js` below, which arms the slot for the next operation. + if let Some((operation, _, consumed)) = pending { + // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. + if unsafe { (*operation).state } != streams::PendingState::Pending { + wrote += consumed; + pending = None; + } + } + match result { // Nothing after it can succeed either. The pending operation, if there is one, is given the error // by the sink itself: this is only reached with nothing pending. - Writable::Err(err) => return Ok(Writable::Err(err).to_js(&cx)), + Writable::Err(err) => { + // A short chunk before this one is still buffered. Left there, the auto flusher finds the + // broken pipe later and marks the sink finished, and every later write reports nothing at + // all. Flushed here it fails as this write did, which is the error the caller already has. + let _ = Self::flush_sink(this, &cx, true); + return Ok(Writable::Err(err).to_js(&cx)); + } Writable::Pending(operation) => { - pending = Some((operation, Writable::Pending(operation).to_js(&cx))); + let promise = Writable::Pending(operation).to_js(&cx); + // SAFETY: as above. + pending = Some((operation, promise, unsafe { (*operation).consumed })); } Writable::Owned(n) | Writable::OwnedAndDone(n) @@ -614,14 +634,14 @@ impl JSSink { } } - let Some((operation, promise)) = pending else { + let Some((operation, promise, _)) = pending else { Self::flush_value(this, global, &cx, true)?; return Ok(JSValue::from(wrote)); }; // The Promise resolves to what the pending operation consumed. The counts are part of this call's total // too: a chunk written before the sink backed up, or a small one buffered beside the operation. - // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive; nothing - // has run since the write that returned it, so it is still the pending one. + // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. It was + // still pending after the last write, and nothing has run since. let operation = unsafe { &mut *operation }; operation.consumed += wrote; // `result` is the running total, unless a write failed beside the operation: then it is that error. @@ -660,19 +680,24 @@ impl JSSink { cx: &bun_jsc::JsThread<'_>, wait: bool, ) -> crate::webcore::jsc::JsResult { - use crate::webcore::jsc::JSValue; use bun_sys_jsc::ErrorJsc; - if T::HAS_FLUSH_FROM_JS { - return match this.sink.flush_from_js(cx, wait) { - sys::Result::Ok(value) => Ok(value), - sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), - }; + match Self::flush_sink(this, cx, wait) { + sys::Result::Ok(value) => Ok(value), + sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), } + } - match this.sink.flush() { - sys::Result::Ok(()) => Ok(JSValue::UNDEFINED), - sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), + fn flush_sink( + this: &mut JSSink, + cx: &bun_jsc::JsThread<'_>, + wait: bool, + ) -> sys::Result { + if T::HAS_FLUSH_FROM_JS { + return this.sink.flush_from_js(cx, wait); } + this.sink + .flush() + .map(|()| crate::webcore::jsc::JSValue::UNDEFINED) } /// `${abi_name}__start` host-fn body. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 4d793b1e0cbd..ed22db6a0d8c 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -145,6 +145,9 @@ try { // // The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and checks with // a writer of its own that the pipe really is broken, so the test cannot pass by the writes simply being queued. +// +// A later console.write() still fails. With a short argument left in the buffer by the failed call, the sink was +// marked finished when that was flushed, and later writes reported 0 bytes and no error. const big = `Buffer.alloc(1024 * 1024, "a").toString()`; test.each([ ["two large arguments", `${big}, ${big}`], @@ -169,6 +172,12 @@ try { } catch (e) { console.error("caught " + e.code); } +await new Promise(resolve => setImmediate(resolve)); +try { + console.error("later: " + (await console.write("later"))); +} catch (e) { + console.error("later: caught " + e.code); +} `, ], env: bunEnv, @@ -182,6 +191,6 @@ try { await proc.stdin.end(); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect(stderr).toBe("pipe already broken: true\ncaught EPIPE\n"); + expect(stderr).toBe("pipe already broken: true\ncaught EPIPE\nlater: caught EPIPE\n"); expect(exitCode).toBe(0); }); From 28530c9c585a2665a48879fcb66ab0d395fa7e61 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:32:43 +0000 Subject: [PATCH 08/27] Sink: the flush result's conversion to JS is not generic JSSink is compiled once per sink type, and turning a flush's result into a JS value or a thrown error does not depend on the type. --- src/runtime/webcore/Sink.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index 690987e3aef3..b7610052a691 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -449,6 +449,19 @@ pub trait JsSinkType: Sized + JsSinkAbi { // no lut entry and no C++ caller. // ────────────────────────────────────────────────────────────────────────── +/// What a flush gave back, for JS: its value, or its error thrown. Outside `JSSink`, which is compiled once +/// per sink type, because none of this depends on the type. +fn flushed_to_js( + global: &crate::webcore::jsc::JSGlobalObject, + flushed: sys::Result, +) -> crate::webcore::jsc::JsResult { + use bun_sys_jsc::ErrorJsc; + match flushed { + sys::Result::Ok(value) => Ok(value), + sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), + } +} + impl JSSink { /// `JSSink.getThis` — recover `&mut JSSink` from `callframe.this()` or /// throw the appropriate detached/cast-failed error. @@ -635,7 +648,7 @@ impl JSSink { } let Some((operation, promise, _)) = pending else { - Self::flush_value(this, global, &cx, true)?; + flushed_to_js(global, Self::flush_sink(this, &cx, true))?; return Ok(JSValue::from(wrote)); }; // The Promise resolves to what the pending operation consumed. The counts are part of this call's total @@ -648,7 +661,7 @@ impl JSSink { if let Writable::Owned(total) = &mut operation.result { *total = operation.consumed; } - Self::flush_value(this, global, &cx, true)?; + flushed_to_js(global, Self::flush_sink(this, &cx, true))?; Ok(promise) } @@ -671,20 +684,7 @@ impl JSSink { let wait = frame.arguments_count() > 0 && frame.argument(0).is_boolean() && frame.argument(0).as_boolean(); - Self::flush_value(this, global, &cx, wait) - } - - fn flush_value( - this: &mut JSSink, - global: &crate::webcore::jsc::JSGlobalObject, - cx: &bun_jsc::JsThread<'_>, - wait: bool, - ) -> crate::webcore::jsc::JsResult { - use bun_sys_jsc::ErrorJsc; - match Self::flush_sink(this, cx, wait) { - sys::Result::Ok(value) => Ok(value), - sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), - } + flushed_to_js(global, Self::flush_sink(this, &cx, wait)) } fn flush_sink( From 6afac974df1c045a75e4be259acc275ba28cb16f Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 11:38:31 +0000 Subject: [PATCH 09/27] FileSink: a write that flushes earlier chunks reports its own bytes The POSIX streaming writer buffers a chunk below its chunk size and reports it as written. When a later write found the buffer not empty, it appended its chunk, wrote the whole buffer to the fd and returned that count, so `sink.write("a")` returned 1 and the 40000 byte write after it returned 40001. The same happened to bytes a pending write had already been credited with. `try_write_newly_buffered_data` now takes the length of the chunk being written. The buffer goes out in order, so the bytes ahead of that chunk are the earlier ones, and they are left out of the count. --- src/io/PipeWriter.rs | 20 ++++++++++++------- test/js/bun/util/filesink.test.ts | 33 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/io/PipeWriter.rs b/src/io/PipeWriter.rs index 859c676aa2f8..8ecc325a538e 100644 --- a/src/io/PipeWriter.rs +++ b/src/io/PipeWriter.rs @@ -854,11 +854,15 @@ impl PosixStreamingWriter { return WriteResult::Wrote(buf_len); } - self.try_write_newly_buffered_data() + self.try_write_newly_buffered_data(buf_len) } - fn try_write_newly_buffered_data(&mut self) -> WriteResult { + /// Writes `outgoing`, whose last `buf_len` bytes are the chunk being written now. The count in the result + /// is that chunk's: the bytes before it are earlier chunks, and each was reported when it was buffered. + fn try_write_newly_buffered_data(&mut self, buf_len: usize) -> WriteResult { debug_assert!(!self.is_done); + // The buffer goes out in order, so the first `earlier` bytes to reach the fd are not this chunk's. + let earlier = self.outgoing.size() - buf_len; // Borrow `self.outgoing` only for the syscall. `try_write` takes `&self` // so the shared borrow of `outgoing.slice()` is sound and ends before @@ -873,27 +877,29 @@ impl PosixStreamingWriter { if amt == self.outgoing.size() { self.outgoing.reset(); self.parent_on_write(amt, WriteStatus::Drained); + WriteResult::Wrote(amt.saturating_sub(earlier)) } else { self.outgoing.wrote(amt); self.parent_on_write(amt, WriteStatus::Pending); Self::register_poll(self); - return WriteResult::Pending(amt); + WriteResult::Pending(amt) } } WriteResult::Done(amt) => { self.outgoing.reset(); self.parent_on_write(amt, WriteStatus::EndOfFile); + WriteResult::Done(amt.saturating_sub(earlier)) } + // Not this chunk's count: the parent works out what was accepted from the buffer's size. WriteResult::Pending(amt) => { self.outgoing.wrote(amt); self.parent_on_write(amt, WriteStatus::Pending); Self::register_poll(self); + WriteResult::Pending(amt) } - WriteResult::Err(e) => return WriteResult::Err(e), + WriteResult::Err(e) => WriteResult::Err(e), } - - rc } pub fn write(&mut self, buf: &[u8]) -> WriteResult { @@ -924,7 +930,7 @@ impl PosixStreamingWriter { return WriteResult::Err(sys::Error::oom()); } - return self.try_write_newly_buffered_data(); + return self.try_write_newly_buffered_data(buf.len()); } let rc = self.try_write(self.force_sync, buf); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index a72ac5422b17..3f354f891eef 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1196,3 +1196,36 @@ try { expect(stderr).toBe("caught EPIPE, same promise: true\n"); expect(exitCode).toBe(0); }); + +// A chunk below the writer's chunk size is buffered and reported as written. The next, larger write sends the +// buffer and itself to the fd together, and used to report all of it: the buffered bytes were counted twice. +// The fourth write is not ASCII, which takes the writer's Latin-1 path. +it("a write() that flushes earlier buffered chunks reports its own bytes", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const sink = Bun.stdout.writer(); +const counts = [ + sink.write("a"), + sink.write(Buffer.alloc(40000, "b").toString()), + sink.write(new Uint8Array(7)), + sink.write(Buffer.alloc(40000, "é").toString()), +]; +await sink.flush(); +console.error(JSON.stringify(counts)); +`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.bytes(), proc.stderr.text(), proc.exited]); + expect({ stderr, stdoutLength: stdout.length }).toEqual({ + stderr: JSON.stringify([1, 40000, 7, 40000]) + "\n", + stdoutLength: 1 + 40000 + 7 + 40000, + }); + expect(exitCode).toBe(0); +}); From 63035f178b0128f4cfd116684ee08fb92f26e5a2 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 12:04:07 +0000 Subject: [PATCH 10/27] console.write: a flush that leaves the sink backed up gives the caller its Promise With the pipe full and its reader stalled, a short console.write() is buffered and the flush cannot push the buffer out. The flush left the sink with a pending Promise and console.write() returned the byte count, so when the reader then hung up the Promise was rejected with nobody holding it: a script that awaited every console.write() still died of an unhandled EPIPE rejection. When the final flush leaves an operation pending that was not there before it, the caller gets that operation's Promise, resolving to the call's total. --- src/runtime/webcore/FileSink.rs | 4 ++ src/runtime/webcore/Sink.rs | 22 +++++++- test/js/bun/console/console-write.test.ts | 69 ++++++++++++++++++++++- 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index d50a77945093..03351d99ca45 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -1470,6 +1470,10 @@ impl crate::webcore::sink::JsSinkType for FileSink { fn pending_state_is_pending(&self) -> bool { self.pending.get().state == streams::PendingState::Pending } + fn pending_operation(&mut self) -> Option<*mut streams::WritablePending> { + self.pending_state_is_pending() + .then(|| self.pending.as_ptr()) + } fn protect_js_wrapper(&mut self, global: &JSGlobalObject, this_value: JSValue) { Self::protect_js_wrapper(self, global, this_value) } diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index b7610052a691..1a74f0f1b2fd 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -396,6 +396,10 @@ pub trait JsSinkType: Sized + JsSinkAbi { fn get_pending_error(&mut self) -> Option { None } + /// The operation whose Promise is outstanding, for a sink that settles writes through one. + fn pending_operation(&mut self) -> Option<*mut streams::WritablePending> { + None + } fn source(&mut self) -> Option<&mut SourceHandle> { None } @@ -648,8 +652,22 @@ impl JSSink { } let Some((operation, promise, _)) = pending else { - flushed_to_js(global, Self::flush_sink(this, &cx, true))?; - return Ok(JSValue::from(wrote)); + let was_pending = this.sink.pending_operation().is_some(); + let flushed = flushed_to_js(global, Self::flush_sink(this, &cx, true))?; + let Some(operation) = this.sink.pending_operation().filter(|_| !was_pending) else { + return Ok(JSValue::from(wrote)); + }; + // Every chunk was buffered, and the flush could not push the buffer out: the sink is backed up after + // all, and `flushed` is the Promise of the operation the flush left pending. The caller gets it. With + // a count instead, nobody holds that Promise when the write fails, and the failure is an unhandled + // rejection. It resolves to this call's total, which what the flush did push out is already part of. + // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. + let operation = unsafe { &mut *operation }; + operation.consumed = wrote; + if let Writable::Owned(total) = &mut operation.result { + *total = wrote; + } + return Ok(flushed); }; // The Promise resolves to what the pending operation consumed. The counts are part of this call's total // too: a chunk written before the sink backed up, or a small one buffered beside the operation. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index ed22db6a0d8c..a6c4c209f386 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -1,5 +1,8 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { mkfifo } from "mkfifo"; +import { closeSync, constants, openSync } from "node:fs"; +import { join } from "node:path"; test("console.write rejects a non-object this", async () => { await using proc = Bun.spawn({ @@ -194,3 +197,67 @@ try { expect(stderr).toBe("pipe already broken: true\ncaught EPIPE\nlater: caught EPIPE\n"); expect(exitCode).toBe(0); }); + +// The reader has stalled and the pipe is full, so a short console.write() is buffered and its flush cannot push +// the buffer out. That left the sink with a pending Promise while console.write() returned the byte count: when +// the reader then hung up, the Promise was rejected with nobody holding it, and a script that awaited every +// console.write() still died of an unhandled rejection. +// +// stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills +// it and stays backed up, so console's own writer has nothing pending when the short write happens. +test.skipIf(isWindows)("an awaited console.write to a full pipe fails when the stalled reader hangs up", async () => { + using dir = tempDir("console-write-stalled", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +process.on("unhandledRejection", e => { + console.error("unhandledRejection " + e?.code); +}); +const filler = Bun.stdout.writer().write(Buffer.alloc(4 * 1024 * 1024, "f").toString()); +filler.catch(() => {}); +const result = console.write("line\\n"); +console.error("READY " + (result instanceof Promise ? "Promise" : result)); +try { + await result; + console.error("resolved"); +} catch (e) { + console.error("caught " + e.code); +} +`, + ], + env: bunEnv, + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + + expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } +}); From e096f7ce1b91cf532a2e74e05eb83b8cb2c8b550 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 12:13:22 +0000 Subject: [PATCH 11/27] FileSink: a write to a sink that failed reports the error it failed with A sink whose write fails from the event loop (the reader hung up, the disk filled) finishes, and a write() after that returned `true`: "done, nothing wrong". The bytes were dropped and nothing said so, and console.write() reported 0 bytes written. The sink already records the error it fails with. Settling a piped stream now reads it instead of taking it, and a finished sink's write() returns it, from the three write entry points and from the writer's own "done" result. flush() had an `unreachable!()` for an error from that result; it returns the error. start() clears the recorded error: a sink started again does not answer for its last run. Two tests pinned `true` for a write after a failure and now expect the error. --- src/runtime/webcore/FileSink.rs | 24 +++++-- test/js/bun/console/console-write.test.ts | 10 ++- test/js/bun/util/filesink.test.ts | 83 +++++++++++++++++++++-- 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 03351d99ca45..f16711d65a66 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -558,7 +558,7 @@ impl FileSink { let Some(global) = self.js_global() else { return; }; - let error = match self.stream_error.replace(None) { + let error = match self.stream_error.get() { Some(err) => Some(err.to_js(global)), None => self.pipe.get().error(), }; @@ -871,6 +871,8 @@ impl FileSink { } self.done.set(false); + // What the sink failed with before belongs to that run: `finished()` must not report it for this one. + self.stream_error.set(None); self.started.set(true); self.source.with_mut(|s| s.start()); sys::Result::Ok(()) @@ -1021,7 +1023,8 @@ impl FileSink { // A flush takes no new chunk from the caller; a pending one reports the // bytes it pushed out. It only reaches here when no write is pending. match self.to_result(rc, flushed) { - streams::Writable::Err(_) => unreachable!(), + // The writer had already finished, with an error: the flush fails with it. + streams::Writable::Err(err) => sys::Result::Err(err), result => sys::Result::Ok(result.to_js(cx)), } } @@ -1109,7 +1112,7 @@ impl FileSink { pub fn write(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return streams::Writable::Done; + return self.finished(); } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write` buffers/writes to fd; does not call JS. @@ -1153,7 +1156,7 @@ impl FileSink { pub(crate) fn write_latin1(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return streams::Writable::Done; + return self.finished(); } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write_latin1` buffers/writes; no JS. @@ -1170,7 +1173,7 @@ impl FileSink { pub(crate) fn write_utf16(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return streams::Writable::Done; + return self.finished(); } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write_utf16` buffers/writes; no JS. @@ -1512,6 +1515,15 @@ impl FileSink { (buffered_after + written).saturating_sub(buffered_before) as u64 // @truncate } + /// What a write to a finished sink reports. One that failed reports the error it failed with: `Done` reads + /// as "nothing written, nothing wrong", and a write after a reader hung up would be lost without a word. + fn finished(&self) -> streams::Writable { + match self.stream_error.get() { + Some(streams::StreamError::Error(err)) => streams::Writable::Err(err.clone()), + _ => streams::Writable::Done, + } + } + /// `accepted` is what the pending slot is credited with when `write_result` /// is `Pending`: a write's full chunk, or the bytes a flush pushed out. It /// is ignored for every other result. @@ -1521,7 +1533,7 @@ impl FileSink { if amt > 0 { return streams::Writable::OwnedAndDone(amt as u64); } - streams::Writable::Done + self.finished() } WriteResult::Wrote(amt) => { if amt > 0 { diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index a6c4c209f386..6cfe5615dfc0 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -205,6 +205,9 @@ try { // // stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills // it and stays backed up, so console's own writer has nothing pending when the short write happens. +// +// A later console.write() fails too: the sink reported "done" after a failure like this one, which read as +// 0 bytes written and nothing wrong. test.skipIf(isWindows)("an awaited console.write to a full pipe fails when the stalled reader hangs up", async () => { using dir = tempDir("console-write-stalled", {}); const fifo = join(String(dir), "stdout.fifo"); @@ -231,6 +234,11 @@ try { } catch (e) { console.error("caught " + e.code); } +try { + console.error("later: " + (await console.write("later"))); +} catch (e) { + console.error("later: caught " + e.code); +} `, ], env: bunEnv, @@ -255,7 +263,7 @@ try { stderr += decoder.decode(value, { stream: true }); } - expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); + expect(stderr).toBe("READY Promise\ncaught EPIPE\nlater: caught EPIPE\n"); expect(await proc.exited).toBe(0); } finally { if (readEndOpen) closeSync(readEnd); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 3f354f891eef..f7f7ab95037f 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1,7 +1,8 @@ import { createSocketPair, fileSinkInternals } from "bun:internal-for-testing"; import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, fileDescriptorLeakChecker, isLinux, isPosix, isWindows, tmpdirSync } from "harness"; +import { bunEnv, bunExe, fileDescriptorLeakChecker, isLinux, isPosix, isWindows, tempDir, tmpdirSync } from "harness"; import { mkfifo } from "mkfifo"; +import { closeSync, constants, openSync } from "node:fs"; import { join } from "node:path"; describe("FileSink", () => { @@ -430,9 +431,14 @@ it.skipIf(!isPosix)( } expect(caught?.code).toBe("EPIPE"); - // The Err arm also moves the sink to its terminal state: further writes - // short-circuit to Writable::Done (=> true). - expect(sink.write("x")).toBe(true); + // The Err arm also moves the sink to its terminal state: a further write + // reports the error the sink failed with, and nothing reaches the fd. + expect( + await Promise.resolve(sink.write("x")).then( + () => "resolved", + e => e.code, + ), + ).toBe("EPIPE"); // end() after the error reports the bytes that actually reached the fd; // the point is it doesn't claim the full chunk was delivered. @@ -619,8 +625,15 @@ it.skipIf(!isPosix)("writing after end() fails during flush does not crash", asy } expect(endErr).toBeDefined(); // Previously this would attempt to write to an invalid fd and crash with a - // debug assertion; now it should behave as if the sink is closed. - expect(() => writer.write("y")).not.toThrow(); + // debug assertion. The sink has failed: a write reports the error it failed + // with, and nothing reaches the fd. + const outcome = (result: unknown) => + Promise.resolve(result).then( + () => "resolved", + e => e.code, + ); + expect(await outcome(writer.write("y"))).toBe("EBADF"); + // Started again, with nothing to write to: that failure was the last run's. expect(() => writer.start({})).not.toThrow(); expect(() => writer.write("z")).not.toThrow(); expect(() => writer.flush()).not.toThrow(); @@ -1229,3 +1242,61 @@ console.error(JSON.stringify(counts)); }); expect(exitCode).toBe(0); }); + +// A sink whose write fails later, from the event loop, finishes. A write() after that returned `true`, which +// is "done, nothing wrong": the bytes were dropped and nothing said so. It reports the error the sink failed with. +// +// stdout is a FIFO whose read end this test holds open and never reads. Another writer fills it, so this sink's +// short write is buffered, its flush stays pending, and the failure arrives when the read end is closed. +it.skipIf(isWindows)("a write() after the sink failed rejects with that error", async () => { + using dir = tempDir("filesink-failed", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const filler = Bun.stdout.writer().write(Buffer.alloc(4 * 1024 * 1024, "f").toString()); +filler.catch(() => {}); +const sink = Bun.stdout.writer(); +sink.write("line\\n"); +const flushed = sink.flush(); +console.error("READY"); +const settle = p => p.then(v => "resolved " + v, e => "rejected " + e.code); +console.error("flush: " + (await settle(Promise.resolve(flushed)))); +console.error("later: " + (await settle(Promise.resolve(sink.write("later"))))); +`, + ], + env: bunEnv, + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + + expect(stderr).toBe("READY\nflush: rejected EPIPE\nlater: rejected EPIPE\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } +}); From 9716f7d6a548d7ed09d27510136ab1bb21a5f26f Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 12:31:17 +0000 Subject: [PATCH 12/27] FileSink: a sink that was closed stays "done" Reporting the error a sink failed with is for a sink that failed while it was open. After end() or close() the sink is finished because it was closed, and a write to it is `Done` as it always was, including when that end()'s flush failed: end() has reported that error. The test for it is unchanged again. --- src/runtime/webcore/FileSink.rs | 16 +++++++++++++--- test/js/bun/util/filesink.test.ts | 11 ++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index f16711d65a66..0f8cb742b94d 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -38,6 +38,8 @@ pub struct FileSink { pub(crate) source: JsCell, pub(crate) done: Cell, pub(crate) started: Cell, + /// `end()` or `close()` was called: the sink is finished because it was closed, whatever that flush did. + ended: Cell, pub(crate) must_be_kept_alive_until_eof: Cell, /// `to_result` returned `Backpressure` to a ByteStream; drain callbacks resume it. pub(crate) source_pending_pull: Cell, @@ -871,7 +873,8 @@ impl FileSink { } self.done.set(false); - // What the sink failed with before belongs to that run: `finished()` must not report it for this one. + // How the last run finished belongs to that run: `finished()` must not report it for this one. + self.ended.set(false); self.stream_error.set(None); self.started.set(true); self.source.with_mut(|s| s.start()); @@ -1226,6 +1229,7 @@ impl FileSink { } pub(crate) fn end(&self, _err: Option) -> sys::Result<()> { + self.ended.set(true); if self.done.get() { return sys::Result::Ok(()); } @@ -1294,6 +1298,7 @@ impl FileSink { } pub(crate) fn end_from_js(&self, cx: &bun_jsc::JsThread<'_>) -> sys::Result { + self.ended.set(true); if self.done.get() { if self.pending.get().state == streams::PendingState::Pending { if let streams::WritableFuture::Promise { strong, .. } = &self.pending.get().future @@ -1515,9 +1520,13 @@ impl FileSink { (buffered_after + written).saturating_sub(buffered_before) as u64 // @truncate } - /// What a write to a finished sink reports. One that failed reports the error it failed with: `Done` reads - /// as "nothing written, nothing wrong", and a write after a reader hung up would be lost without a word. + /// What a write to a finished sink reports. A sink that was closed is `Done`, as it always was. One that + /// failed while it was open reports the error it failed with: `Done` reads as "nothing written, nothing + /// wrong", and a write after a reader hung up would be lost without a word. fn finished(&self) -> streams::Writable { + if self.ended.get() { + return streams::Writable::Done; + } match self.stream_error.get() { Some(streams::StreamError::Error(err)) => streams::Writable::Err(err.clone()), _ => streams::Writable::Done, @@ -1599,6 +1608,7 @@ impl FileSink { source: JsCell::new(streams::SourceHandle::default()), done: Cell::new(false), started: Cell::new(false), + ended: Cell::new(false), must_be_kept_alive_until_eof: Cell::new(false), source_pending_pull: Cell::new(false), pollable: Cell::new(false), diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index f7f7ab95037f..3c05d511a890 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -625,15 +625,8 @@ it.skipIf(!isPosix)("writing after end() fails during flush does not crash", asy } expect(endErr).toBeDefined(); // Previously this would attempt to write to an invalid fd and crash with a - // debug assertion. The sink has failed: a write reports the error it failed - // with, and nothing reaches the fd. - const outcome = (result: unknown) => - Promise.resolve(result).then( - () => "resolved", - e => e.code, - ); - expect(await outcome(writer.write("y"))).toBe("EBADF"); - // Started again, with nothing to write to: that failure was the last run's. + // debug assertion; now it should behave as if the sink is closed. + expect(() => writer.write("y")).not.toThrow(); expect(() => writer.start({})).not.toThrow(); expect(() => writer.write("z")).not.toThrow(); expect(() => writer.flush()).not.toThrow(); From 958f02e6a908c2ee52d5ecbfdddaead7726ea729 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 12:52:36 +0000 Subject: [PATCH 13/27] FileSink: a write to a finished sink is "done" again Reverts the two commits that made a write() to a sink that had failed return the error as a rejected Promise. A script that writes lines without awaiting them and is piped into `head -1` runs to its end and exits 0: its short writes are buffered, the flush fails from the event loop, the sink finishes, and later writes return `true`. With a rejected Promise per later write, held by nobody, that script died of an unhandled rejection. A test now holds that behaviour in place. --- src/runtime/webcore/FileSink.rs | 34 ++------ test/js/bun/console/console-write.test.ts | 10 +-- test/js/bun/util/filesink.test.ts | 98 +++++++++-------------- 3 files changed, 43 insertions(+), 99 deletions(-) diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 0f8cb742b94d..03351d99ca45 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -38,8 +38,6 @@ pub struct FileSink { pub(crate) source: JsCell, pub(crate) done: Cell, pub(crate) started: Cell, - /// `end()` or `close()` was called: the sink is finished because it was closed, whatever that flush did. - ended: Cell, pub(crate) must_be_kept_alive_until_eof: Cell, /// `to_result` returned `Backpressure` to a ByteStream; drain callbacks resume it. pub(crate) source_pending_pull: Cell, @@ -560,7 +558,7 @@ impl FileSink { let Some(global) = self.js_global() else { return; }; - let error = match self.stream_error.get() { + let error = match self.stream_error.replace(None) { Some(err) => Some(err.to_js(global)), None => self.pipe.get().error(), }; @@ -873,9 +871,6 @@ impl FileSink { } self.done.set(false); - // How the last run finished belongs to that run: `finished()` must not report it for this one. - self.ended.set(false); - self.stream_error.set(None); self.started.set(true); self.source.with_mut(|s| s.start()); sys::Result::Ok(()) @@ -1026,8 +1021,7 @@ impl FileSink { // A flush takes no new chunk from the caller; a pending one reports the // bytes it pushed out. It only reaches here when no write is pending. match self.to_result(rc, flushed) { - // The writer had already finished, with an error: the flush fails with it. - streams::Writable::Err(err) => sys::Result::Err(err), + streams::Writable::Err(_) => unreachable!(), result => sys::Result::Ok(result.to_js(cx)), } } @@ -1115,7 +1109,7 @@ impl FileSink { pub fn write(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return self.finished(); + return streams::Writable::Done; } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write` buffers/writes to fd; does not call JS. @@ -1159,7 +1153,7 @@ impl FileSink { pub(crate) fn write_latin1(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return self.finished(); + return streams::Writable::Done; } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write_latin1` buffers/writes; no JS. @@ -1176,7 +1170,7 @@ impl FileSink { pub(crate) fn write_utf16(&self, data: &streams::Result) -> streams::Writable { if self.done.get() { - return self.finished(); + return streams::Writable::Done; } let buffered_before = self.writer.get().buffered_len(); // SAFETY(JsCell): `IOWriter::write_utf16` buffers/writes; no JS. @@ -1229,7 +1223,6 @@ impl FileSink { } pub(crate) fn end(&self, _err: Option) -> sys::Result<()> { - self.ended.set(true); if self.done.get() { return sys::Result::Ok(()); } @@ -1298,7 +1291,6 @@ impl FileSink { } pub(crate) fn end_from_js(&self, cx: &bun_jsc::JsThread<'_>) -> sys::Result { - self.ended.set(true); if self.done.get() { if self.pending.get().state == streams::PendingState::Pending { if let streams::WritableFuture::Promise { strong, .. } = &self.pending.get().future @@ -1520,19 +1512,6 @@ impl FileSink { (buffered_after + written).saturating_sub(buffered_before) as u64 // @truncate } - /// What a write to a finished sink reports. A sink that was closed is `Done`, as it always was. One that - /// failed while it was open reports the error it failed with: `Done` reads as "nothing written, nothing - /// wrong", and a write after a reader hung up would be lost without a word. - fn finished(&self) -> streams::Writable { - if self.ended.get() { - return streams::Writable::Done; - } - match self.stream_error.get() { - Some(streams::StreamError::Error(err)) => streams::Writable::Err(err.clone()), - _ => streams::Writable::Done, - } - } - /// `accepted` is what the pending slot is credited with when `write_result` /// is `Pending`: a write's full chunk, or the bytes a flush pushed out. It /// is ignored for every other result. @@ -1542,7 +1521,7 @@ impl FileSink { if amt > 0 { return streams::Writable::OwnedAndDone(amt as u64); } - self.finished() + streams::Writable::Done } WriteResult::Wrote(amt) => { if amt > 0 { @@ -1608,7 +1587,6 @@ impl FileSink { source: JsCell::new(streams::SourceHandle::default()), done: Cell::new(false), started: Cell::new(false), - ended: Cell::new(false), must_be_kept_alive_until_eof: Cell::new(false), source_pending_pull: Cell::new(false), pollable: Cell::new(false), diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 6cfe5615dfc0..a6c4c209f386 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -205,9 +205,6 @@ try { // // stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills // it and stays backed up, so console's own writer has nothing pending when the short write happens. -// -// A later console.write() fails too: the sink reported "done" after a failure like this one, which read as -// 0 bytes written and nothing wrong. test.skipIf(isWindows)("an awaited console.write to a full pipe fails when the stalled reader hangs up", async () => { using dir = tempDir("console-write-stalled", {}); const fifo = join(String(dir), "stdout.fifo"); @@ -234,11 +231,6 @@ try { } catch (e) { console.error("caught " + e.code); } -try { - console.error("later: " + (await console.write("later"))); -} catch (e) { - console.error("later: caught " + e.code); -} `, ], env: bunEnv, @@ -263,7 +255,7 @@ try { stderr += decoder.decode(value, { stream: true }); } - expect(stderr).toBe("READY Promise\ncaught EPIPE\nlater: caught EPIPE\n"); + expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); expect(await proc.exited).toBe(0); } finally { if (readEndOpen) closeSync(readEnd); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 3c05d511a890..13befe2929d7 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1,8 +1,7 @@ import { createSocketPair, fileSinkInternals } from "bun:internal-for-testing"; import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, fileDescriptorLeakChecker, isLinux, isPosix, isWindows, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, fileDescriptorLeakChecker, isLinux, isPosix, isWindows, tmpdirSync } from "harness"; import { mkfifo } from "mkfifo"; -import { closeSync, constants, openSync } from "node:fs"; import { join } from "node:path"; describe("FileSink", () => { @@ -431,14 +430,9 @@ it.skipIf(!isPosix)( } expect(caught?.code).toBe("EPIPE"); - // The Err arm also moves the sink to its terminal state: a further write - // reports the error the sink failed with, and nothing reaches the fd. - expect( - await Promise.resolve(sink.write("x")).then( - () => "resolved", - e => e.code, - ), - ).toBe("EPIPE"); + // The Err arm also moves the sink to its terminal state: further writes + // short-circuit to Writable::Done (=> true). + expect(sink.write("x")).toBe(true); // end() after the error reports the bytes that actually reached the fd; // the point is it doesn't claim the full chunk was delivered. @@ -1236,60 +1230,40 @@ console.error(JSON.stringify(counts)); expect(exitCode).toBe(0); }); -// A sink whose write fails later, from the event loop, finishes. A write() after that returned `true`, which -// is "done, nothing wrong": the bytes were dropped and nothing said so. It reports the error the sink failed with. -// -// stdout is a FIFO whose read end this test holds open and never reads. Another writer fills it, so this sink's -// short write is buffered, its flush stays pending, and the failure arrives when the read end is closed. -it.skipIf(isWindows)("a write() after the sink failed rejects with that error", async () => { - using dir = tempDir("filesink-failed", {}); - const fifo = join(String(dir), "stdout.fifo"); - mkfifo(fifo, 0o600); - const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - const writeEnd = openSync(fifo, constants.O_WRONLY); - let readEndOpen = true; - try { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` -const filler = Bun.stdout.writer().write(Buffer.alloc(4 * 1024 * 1024, "f").toString()); -filler.catch(() => {}); +// A CLI that writes lines without awaiting them and is piped into `head -1`: after the reader is gone its short +// writes are buffered, the flush fails from the event loop, and the sink finishes. Later writes return `true` +// and the script runs to its end. A rejected Promise per later write would be held by nobody, and the script +// would die of an unhandled rejection instead. +it("writes that are not awaited keep the script running after the reader has gone", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` const sink = Bun.stdout.writer(); -sink.write("line\\n"); -const flushed = sink.flush(); -console.error("READY"); -const settle = p => p.then(v => "resolved " + v, e => "rejected " + e.code); -console.error("flush: " + (await settle(Promise.resolve(flushed)))); -console.error("later: " + (await settle(Promise.resolve(sink.write("later"))))); +sink.write("first\\n"); +sink.flush(); +require("node:fs").readSync(0, Buffer.alloc(1)); +for (let batch = 0; batch < 20; batch++) { + for (let i = 0; i < 50; i++) sink.write("line " + i + "\\n"); + await new Promise(resolve => setImmediate(resolve)); +} +console.error("finished"); `, - ], - env: bunEnv, - stdout: writeEnd, - stderr: "pipe", - }); - closeSync(writeEnd); + ], + env: bunEnv, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); - const reader = proc.stderr.getReader(); - const decoder = new TextDecoder(); - let stderr = ""; - while (!stderr.includes("READY")) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - closeSync(readEnd); - readEndOpen = false; - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } + const reader = proc.stdout.getReader(); + await reader.read(); + await reader.cancel(); + proc.stdin.write("x"); + await proc.stdin.end(); - expect(stderr).toBe("READY\nflush: rejected EPIPE\nlater: rejected EPIPE\n"); - expect(await proc.exited).toBe(0); - } finally { - if (readEndOpen) closeSync(readEnd); - } + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toBe("finished\n"); + expect(exitCode).toBe(0); }); From a2aaf97b56c5ab02fa11c481fa880e5a4871151e Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 13:04:09 +0000 Subject: [PATCH 14/27] filesink test: the un-awaited writes case is POSIX behaviour On Windows a write goes to the pipe at once, so after the reader has gone each one fails on the spot with a rejected Promise of its own, and the script has always died of it there (Bun 1.4.0 does). --- test/js/bun/util/filesink.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 13befe2929d7..53ae1d9e9aa6 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1234,7 +1234,10 @@ console.error(JSON.stringify(counts)); // writes are buffered, the flush fails from the event loop, and the sink finishes. Later writes return `true` // and the script runs to its end. A rejected Promise per later write would be held by nobody, and the script // would die of an unhandled rejection instead. -it("writes that are not awaited keep the script running after the reader has gone", async () => { +// +// Not on Windows, where this script has always died that way: a write goes to the pipe at once there, so each +// one fails on the spot with a rejected Promise of its own. +it.skipIf(isWindows)("writes that are not awaited keep the script running after the reader has gone", async () => { await using proc = Bun.spawn({ cmd: [ bunExe(), From 9b14b3d0f543c19dfabd497aff2c3bcd12f361c7 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 13:26:48 +0000 Subject: [PATCH 15/27] Terminal: the writer no longer reports more than the chunk The comment in Terminal::write said a count could exceed the input when earlier data drained with it. That was the streaming writer counting the earlier bytes again, which it no longer does. Terminal returns the input length either way. --- src/runtime/api/bun/Terminal.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index 0a02a3ab95a3..2ec81ed827f0 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -1496,9 +1496,9 @@ impl Terminal { // StreamingWriter::write() buffers any bytes it couldn't flush // synchronously, so the full input has been accepted on every non-error - // return. The per-arm counts are sync-flushed bytes (and on a buffered - // writer can even exceed `input_len` when prior data drains), so - // returning them would make callers re-send an already-queued tail. + // return. A `Pending` or partial count is the bytes that reached the fd, + // not the bytes accepted, so returning it would make callers re-send an + // already-queued tail. match write_result { bun_io::WriteResult::Err(err) => { Err(global_object.throw_value(err.to_js(global_object))) From 5c98ceda0078781ad0a56dd94e1e9f70c92f497a Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 13:52:28 +0000 Subject: [PATCH 16/27] console.write: a Promise that never reaches the caller is not an unhandled rejection console.write(big, 123) throws for the argument that is not something to write, after the first one has become the sink's pending write. That write's Promise was made inside the call and the throw means the caller never gets it, so when the write later failed it was reported as an unhandled rejection that nobody could have handled. It is marked handled when the call throws. What gets written is unchanged, and an operation left pending by an earlier call keeps its Promise as it was: that one is the earlier caller's. --- src/runtime/webcore/Sink.rs | 18 ++++++- test/js/bun/console/console-write.test.ts | 64 +++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index e314b8c56be6..4e1dd04140e2 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -612,11 +612,27 @@ impl JSSink { } let mut wrote: u64 = 0; + // An operation left pending by an earlier call: its Promise is that caller's. + let pending_before = this.sink.pending_operation().is_some(); // The sink's pending operation, its Promise, and what that Promise resolves to as of the last write. let mut pending: Option<(*mut streams::WritablePending, JSValue, u64)> = None; // No argument is one `undefined` chunk, which write_value() rejects as write() does. for i in 0..frame.arguments_count().max(1) as usize { - let result = Self::write_value(this, global, frame.argument(i))?; + let result = match Self::write_value(this, global, frame.argument(i)) { + Ok(result) => result, + Err(thrown) => { + // This argument is not something to write, and the call throws. An earlier one is already + // the sink's pending operation, with a Promise made in this call that now never reaches + // the caller. Nobody can handle its rejection, so it must not be reported as unhandled. + if let (false, Some((_, promise, _))) = (pending_before, pending) { + if let Some(promise) = promise.as_promise() { + // SAFETY: `as_promise` returned this live JSPromise cell, which `promise` keeps alive. + unsafe { (*promise).set_handled() }; + } + } + return Err(thrown); + } + }; // A write can settle the pending operation on the spot: the reader made room, and this chunk went out // with everything buffered before it. Its Promise has resolved to `consumed`, which this call's total // carries on from as a count. Checked before `to_js` below, which arms the slot for the next operation. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index a6c4c209f386..4b2256964e58 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -261,3 +261,67 @@ try { if (readEndOpen) closeSync(readEnd); } }); + +// console.write(big, 123) throws for the argument that is not something to write, after the first one has become +// the sink's pending write. That write's Promise was made inside the call and never reaches the caller, so when +// the write later fails nobody can have handled it: it must not be reported as an unhandled rejection. +// +// stdout is a FIFO whose read end this test holds open and never reads, then closes. +test.skipIf(isWindows)("console.write that throws for a later argument leaves no unhandled rejection", async () => { + using dir = tempDir("console-write-bad-argument", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +process.on("unhandledRejection", e => { + console.error("unhandledRejection " + e?.code); +}); +try { + console.write(Buffer.alloc(4 * 1024 * 1024, "x").toString(), 123); + console.error("no throw"); +} catch (e) { + console.error("caught " + e.code); +} +console.error("READY"); +process.stdin.once("data", () => console.error("end")); +`, + ], + env: bunEnv, + stdin: "pipe", + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + // The child's event loop sees the hang-up before it sees this byte on stdin. + proc.stdin.write("x"); + await proc.stdin.end(); + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + + expect(stderr).toBe("caught ERR_INVALID_ARG_TYPE\nREADY\nend\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } +}); From 54188c46525d5b23d7b341ffee711399126d0b42 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 14:13:26 +0000 Subject: [PATCH 17/27] console.write: tell the sink's operations apart by their Promise js_write_all asked whether the sink had a pending operation, at entry and after each write. The question is whether the outstanding Promise is the one this call holds. An operation an earlier call left pending can settle during this call and this call can start one of its own, whose Promise was then left unhandled when a later argument threw. Settling can also run the script's microtasks, where a console.write() can leave the slot pending again for an operation of its own. The three checks compare the Promise: on a throw, in the loop, and after the flush. --- src/runtime/webcore/Sink.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index 4e1dd04140e2..00a8ab456e51 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -540,6 +540,16 @@ impl JSSink { Ok(Self::write_value(this, global, frame.argument(0))?.to_js(&cx)) } + /// The Promise of the sink's pending operation, when it has one that JS holds. + fn pending_promise(this: &mut JSSink) -> Option { + let operation = this.sink.pending_operation()?; + // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. + match unsafe { &(*operation).future } { + streams::WritableFuture::Promise { strong, .. } => Some(strong.value()), + streams::WritableFuture::None => None, + } + } + /// One chunk from JS into the sink: bytes as they are, a string by its encoding. fn write_value( this: &mut JSSink, @@ -612,8 +622,8 @@ impl JSSink { } let mut wrote: u64 = 0; - // An operation left pending by an earlier call: its Promise is that caller's. - let pending_before = this.sink.pending_operation().is_some(); + // The Promise of an operation an earlier call left pending: that caller's, not this call's. + let promise_before = Self::pending_promise(this); // The sink's pending operation, its Promise, and what that Promise resolves to as of the last write. let mut pending: Option<(*mut streams::WritablePending, JSValue, u64)> = None; // No argument is one `undefined` chunk, which write_value() rejects as write() does. @@ -624,7 +634,9 @@ impl JSSink { // This argument is not something to write, and the call throws. An earlier one is already // the sink's pending operation, with a Promise made in this call that now never reaches // the caller. Nobody can handle its rejection, so it must not be reported as unhandled. - if let (false, Some((_, promise, _))) = (pending_before, pending) { + if let Some((_, promise, _)) = + pending.filter(|held| Some(held.1) != promise_before) + { if let Some(promise) = promise.as_promise() { // SAFETY: `as_promise` returned this live JSPromise cell, which `promise` keeps alive. unsafe { (*promise).set_handled() }; @@ -635,10 +647,11 @@ impl JSSink { }; // A write can settle the pending operation on the spot: the reader made room, and this chunk went out // with everything buffered before it. Its Promise has resolved to `consumed`, which this call's total - // carries on from as a count. Checked before `to_js` below, which arms the slot for the next operation. - if let Some((operation, _, consumed)) = pending { - // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. - if unsafe { (*operation).state } != streams::PendingState::Pending { + // carries on from as a count. Settling can run the script's microtasks, and a console.write() in one + // of them can leave the slot pending again for an operation of its own, so the question is whether the + // outstanding Promise is still the one held here. Checked before `to_js` below, which arms the slot. + if let Some((_, promise, consumed)) = pending { + if Self::pending_promise(this) != Some(promise) { wrote += consumed; pending = None; } @@ -668,9 +681,11 @@ impl JSSink { } let Some((operation, promise, _)) = pending else { - let was_pending = this.sink.pending_operation().is_some(); + let promise_before_flush = Self::pending_promise(this); let flushed = flushed_to_js(global, Self::flush_sink(this, &cx, true))?; - let Some(operation) = this.sink.pending_operation().filter(|_| !was_pending) else { + let armed_by_flush = Self::pending_promise(this) + .is_some_and(|promise| promise == flushed && Some(promise) != promise_before_flush); + let Some(operation) = this.sink.pending_operation().filter(|_| armed_by_flush) else { return Ok(JSValue::from(wrote)); }; // Every chunk was buffered, and the flush could not push the buffer out: the sink is backed up after @@ -688,7 +703,7 @@ impl JSSink { // The Promise resolves to what the pending operation consumed. The counts are part of this call's total // too: a chunk written before the sink backed up, or a small one buffered beside the operation. // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. It was - // still pending after the last write, and nothing has run since. + // still this call's operation after the last write, and nothing runs between that check and here. let operation = unsafe { &mut *operation }; operation.consumed += wrote; // `result` is the running total, unless a write failed beside the operation: then it is that error. From 80f235d0de9e7d76d4bc4905b8855e6f5770a73d Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 14:30:20 +0000 Subject: [PATCH 18/27] console.write: carry only this call's bytes; shorter comments When this call's writes joined an operation an earlier call had left pending, and a later argument settled it on the spot, the count carried on was everything that operation had consumed, the earlier call's bytes included. It is what this call gave it. The comments in js_write_all say the one thing the code cannot. --- src/runtime/webcore/Sink.rs | 56 ++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index 00a8ab456e51..dfe886990495 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -622,53 +622,56 @@ impl JSSink { } let mut wrote: u64 = 0; - // The Promise of an operation an earlier call left pending: that caller's, not this call's. + // An operation an earlier call left pending: its Promise is that caller's, and so is what it has consumed. let promise_before = Self::pending_promise(this); - // The sink's pending operation, its Promise, and what that Promise resolves to as of the last write. + // SAFETY: the sink's pending slot, which the sink owns and `this` keeps alive. + let consumed_before = this + .sink + .pending_operation() + .map_or(0, |operation| unsafe { (*operation).consumed }); + // This call's pending operation: the slot, its Promise, and what it has consumed as of the last write. let mut pending: Option<(*mut streams::WritablePending, JSValue, u64)> = None; // No argument is one `undefined` chunk, which write_value() rejects as write() does. for i in 0..frame.arguments_count().max(1) as usize { let result = match Self::write_value(this, global, frame.argument(i)) { Ok(result) => result, Err(thrown) => { - // This argument is not something to write, and the call throws. An earlier one is already - // the sink's pending operation, with a Promise made in this call that now never reaches - // the caller. Nobody can handle its rejection, so it must not be reported as unhandled. + // A Promise made in this call never reaches the caller now: nobody could handle its rejection. if let Some((_, promise, _)) = pending.filter(|held| Some(held.1) != promise_before) { if let Some(promise) = promise.as_promise() { - // SAFETY: `as_promise` returned this live JSPromise cell, which `promise` keeps alive. + // SAFETY: the live JSPromise cell `as_promise` returned, which `promise` keeps alive. unsafe { (*promise).set_handled() }; } } return Err(thrown); } }; - // A write can settle the pending operation on the spot: the reader made room, and this chunk went out - // with everything buffered before it. Its Promise has resolved to `consumed`, which this call's total - // carries on from as a count. Settling can run the script's microtasks, and a console.write() in one - // of them can leave the slot pending again for an operation of its own, so the question is whether the - // outstanding Promise is still the one held here. Checked before `to_js` below, which arms the slot. + // That write can have settled the operation on the spot, and settling runs microtasks, where a nested + // console.write() can leave the slot pending again: compare Promises, before `to_js` below arms the + // slot. What this call gave the operation carries on as a count. if let Some((_, promise, consumed)) = pending { if Self::pending_promise(this) != Some(promise) { - wrote += consumed; + wrote += consumed + - if Some(promise) == promise_before { + consumed_before + } else { + 0 + }; pending = None; } } match result { - // Nothing after it can succeed either. The pending operation, if there is one, is given the error - // by the sink itself: this is only reached with nothing pending. + // With an operation pending, the sink gives it the error and returns it instead of this. Writable::Err(err) => { - // A short chunk before this one is still buffered. Left there, the auto flusher finds the - // broken pipe later and marks the sink finished, and every later write reports nothing at - // all. Flushed here it fails as this write did, which is the error the caller already has. + // Or the auto flusher fails on a short chunk still buffered, and finishes the sink. let _ = Self::flush_sink(this, &cx, true); return Ok(Writable::Err(err).to_js(&cx)); } Writable::Pending(operation) => { let promise = Writable::Pending(operation).to_js(&cx); - // SAFETY: as above. + // SAFETY: the sink's pending slot, as above. pending = Some((operation, promise, unsafe { (*operation).consumed })); } Writable::Owned(n) @@ -688,11 +691,9 @@ impl JSSink { let Some(operation) = this.sink.pending_operation().filter(|_| armed_by_flush) else { return Ok(JSValue::from(wrote)); }; - // Every chunk was buffered, and the flush could not push the buffer out: the sink is backed up after - // all, and `flushed` is the Promise of the operation the flush left pending. The caller gets it. With - // a count instead, nobody holds that Promise when the write fails, and the failure is an unhandled - // rejection. It resolves to this call's total, which what the flush did push out is already part of. - // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. + // The flush left an operation pending. The caller gets its Promise, which would have no holder when + // the write fails, resolving to this call's total: what the flush did push out is part of that. + // SAFETY: the sink's pending slot, as above. let operation = unsafe { &mut *operation }; operation.consumed = wrote; if let Writable::Owned(total) = &mut operation.result { @@ -700,13 +701,12 @@ impl JSSink { } return Ok(flushed); }; - // The Promise resolves to what the pending operation consumed. The counts are part of this call's total - // too: a chunk written before the sink backed up, or a small one buffered beside the operation. - // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. It was - // still this call's operation after the last write, and nothing runs between that check and here. + // The counts join the total: a chunk written before the sink backed up, or a small one buffered beside + // the operation. + // SAFETY: the sink's pending slot, as above; nothing has run since the check after the last write. let operation = unsafe { &mut *operation }; operation.consumed += wrote; - // `result` is the running total, unless a write failed beside the operation: then it is that error. + // Not when a write failed beside the operation: `result` is that error then. if let Writable::Owned(total) = &mut operation.result { *total = operation.consumed; } From ec2c02755835167d26a362c1344eef91bf6e7c3b Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 14:53:35 +0000 Subject: [PATCH 19/27] console-write and filesink tests: the new spawning tests run concurrently Each has its own child, pipe or FIFO and temp directory, and shares nothing with the others. --- test/js/bun/console/console-write.test.ts | 176 +++++++++++----------- test/js/bun/util/filesink.test.ts | 47 +++--- 2 files changed, 116 insertions(+), 107 deletions(-) diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 4b2256964e58..3ad7340e6507 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -152,7 +152,7 @@ try { // A later console.write() still fails. With a short argument left in the buffer by the failed call, the sink was // marked finished when that was flushed, and later writes reported 0 bytes and no error. const big = `Buffer.alloc(1024 * 1024, "a").toString()`; -test.each([ +test.concurrent.each([ ["two large arguments", `${big}, ${big}`], ["a large argument, then a short one", `${big}, "\\n"`], ["a short argument, then a large one", `"\\n", ${big}`], @@ -205,19 +205,21 @@ try { // // stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills // it and stays backed up, so console's own writer has nothing pending when the short write happens. -test.skipIf(isWindows)("an awaited console.write to a full pipe fails when the stalled reader hangs up", async () => { - using dir = tempDir("console-write-stalled", {}); - const fifo = join(String(dir), "stdout.fifo"); - mkfifo(fifo, 0o600); - const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - const writeEnd = openSync(fifo, constants.O_WRONLY); - let readEndOpen = true; - try { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` +test.concurrent.skipIf(isWindows)( + "an awaited console.write to a full pipe fails when the stalled reader hangs up", + async () => { + using dir = tempDir("console-write-stalled", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` process.on("unhandledRejection", e => { console.error("unhandledRejection " + e?.code); }); @@ -232,54 +234,57 @@ try { console.error("caught " + e.code); } `, - ], - env: bunEnv, - stdout: writeEnd, - stderr: "pipe", - }); - closeSync(writeEnd); + ], + env: bunEnv, + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); - const reader = proc.stderr.getReader(); - const decoder = new TextDecoder(); - let stderr = ""; - while (!stderr.includes("READY")) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - closeSync(readEnd); - readEndOpen = false; - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } - expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); - expect(await proc.exited).toBe(0); - } finally { - if (readEndOpen) closeSync(readEnd); - } -}); + expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } + }, +); // console.write(big, 123) throws for the argument that is not something to write, after the first one has become // the sink's pending write. That write's Promise was made inside the call and never reaches the caller, so when // the write later fails nobody can have handled it: it must not be reported as an unhandled rejection. // // stdout is a FIFO whose read end this test holds open and never reads, then closes. -test.skipIf(isWindows)("console.write that throws for a later argument leaves no unhandled rejection", async () => { - using dir = tempDir("console-write-bad-argument", {}); - const fifo = join(String(dir), "stdout.fifo"); - mkfifo(fifo, 0o600); - const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - const writeEnd = openSync(fifo, constants.O_WRONLY); - let readEndOpen = true; - try { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` +test.concurrent.skipIf(isWindows)( + "console.write that throws for a later argument leaves no unhandled rejection", + async () => { + using dir = tempDir("console-write-bad-argument", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` process.on("unhandledRejection", e => { console.error("unhandledRejection " + e?.code); }); @@ -292,36 +297,37 @@ try { console.error("READY"); process.stdin.once("data", () => console.error("end")); `, - ], - env: bunEnv, - stdin: "pipe", - stdout: writeEnd, - stderr: "pipe", - }); - closeSync(writeEnd); + ], + env: bunEnv, + stdin: "pipe", + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); - const reader = proc.stderr.getReader(); - const decoder = new TextDecoder(); - let stderr = ""; - while (!stderr.includes("READY")) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - closeSync(readEnd); - readEndOpen = false; - // The child's event loop sees the hang-up before it sees this byte on stdin. - proc.stdin.write("x"); - await proc.stdin.end(); - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + // The child's event loop sees the hang-up before it sees this byte on stdin. + proc.stdin.write("x"); + await proc.stdin.end(); + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } - expect(stderr).toBe("caught ERR_INVALID_ARG_TYPE\nREADY\nend\n"); - expect(await proc.exited).toBe(0); - } finally { - if (readEndOpen) closeSync(readEnd); - } -}); + expect(stderr).toBe("caught ERR_INVALID_ARG_TYPE\nREADY\nend\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } + }, +); diff --git a/test/js/bun/util/filesink.test.ts b/test/js/bun/util/filesink.test.ts index 53ae1d9e9aa6..d6e7f6ca0783 100644 --- a/test/js/bun/util/filesink.test.ts +++ b/test/js/bun/util/filesink.test.ts @@ -1200,7 +1200,7 @@ try { // A chunk below the writer's chunk size is buffered and reported as written. The next, larger write sends the // buffer and itself to the fd together, and used to report all of it: the buffered bytes were counted twice. // The fourth write is not ASCII, which takes the writer's Latin-1 path. -it("a write() that flushes earlier buffered chunks reports its own bytes", async () => { +it.concurrent("a write() that flushes earlier buffered chunks reports its own bytes", async () => { await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -1237,12 +1237,14 @@ console.error(JSON.stringify(counts)); // // Not on Windows, where this script has always died that way: a write goes to the pipe at once there, so each // one fails on the spot with a rejected Promise of its own. -it.skipIf(isWindows)("writes that are not awaited keep the script running after the reader has gone", async () => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` +it.concurrent.skipIf(isWindows)( + "writes that are not awaited keep the script running after the reader has gone", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` const sink = Bun.stdout.writer(); sink.write("first\\n"); sink.flush(); @@ -1253,20 +1255,21 @@ for (let batch = 0; batch < 20; batch++) { } console.error("finished"); `, - ], - env: bunEnv, - stdin: "pipe", - stdout: "pipe", - stderr: "pipe", - }); + ], + env: bunEnv, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); - const reader = proc.stdout.getReader(); - await reader.read(); - await reader.cancel(); - proc.stdin.write("x"); - await proc.stdin.end(); + const reader = proc.stdout.getReader(); + await reader.read(); + await reader.cancel(); + proc.stdin.write("x"); + await proc.stdin.end(); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect(stderr).toBe("finished\n"); - expect(exitCode).toBe(0); -}); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toBe("finished\n"); + expect(exitCode).toBe(0); + }, +); From c45bae875bd05ac19e6125553399d50a531ca203 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 23:51:07 +0000 Subject: [PATCH 20/27] console.write: back to the loop in JS; mark the Promises it drops as handled The native version had to reproduce what the loop of write() and then flush(true) does, case by case, and kept differing from it: which error a call with a failed write and an invalid argument throws, a thrown EPIPE turned into a rejection, a Promise returned where a count was, 0 where a finished sink gave `true`. The loop is the behaviour, so it stays, exactly as it was. What was wrong is that the sink hands console.write Promises the caller never gets, and each is an unhandled rejection when the pipe breaks: a write that fails on the spot returns a rejected one of its own, a flush that cannot finish returns one, and a throw loses the one in hand. Those are marked handled. The one an earlier call returned is left alone: a later write or flush can hand it back, and it is its caller's. Sink.rs, FileSink.rs and the js2native table are as on main again. --- src/codegen/generate-js2native.ts | 1 - src/js/builtins/BunBuiltinNames.h | 2 +- src/js/builtins/ConsoleObject.ts | 48 ++++-- src/runtime/webcore/FileSink.rs | 10 -- src/runtime/webcore/Sink.rs | 198 +++------------------- test/js/bun/console/console-write.test.ts | 10 +- 6 files changed, 71 insertions(+), 198 deletions(-) diff --git a/src/codegen/generate-js2native.ts b/src/codegen/generate-js2native.ts index 9a59572fa8ed..8e7168f1f745 100644 --- a/src/codegen/generate-js2native.ts +++ b/src/codegen/generate-js2native.ts @@ -46,7 +46,6 @@ const sourceFiles = readdirRecursiveWithExclusionsAndExtensionsSync( const rustIdentifierPaths: Record = { "bun.rs": "bun.rs", "ipc.rs": "runtime/ipc_host.rs", - "FileSink.rs": "runtime/webcore/FileSink.rs", "Counters.rs": "jsc/Counters.rs", "FrameworkRouter.rs": "runtime/bake/FrameworkRouter.rs", "JSBundler.rs": "runtime/api/JSBundler.rs", diff --git a/src/js/builtins/BunBuiltinNames.h b/src/js/builtins/BunBuiltinNames.h index bc6d5762b2fa..c4308b1a4752 100644 --- a/src/js/builtins/BunBuiltinNames.h +++ b/src/js/builtins/BunBuiltinNames.h @@ -165,6 +165,7 @@ using namespace JSC; macro(requireMap) \ macro(requireNativeModule) \ macro(resolveSync) \ + macro(returnedWrite) \ macro(sameSite) \ macro(secure) \ macro(self) \ @@ -197,7 +198,6 @@ using namespace JSC; macro(writable) \ macro(writableType) \ macro(write) \ - macro(writeAll) \ macro(writer) \ macro(written) \ BUN_ADDITIONAL_BUILTIN_NAMES(macro) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index d8e31031f5e9..e0c99e7c80be 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -119,13 +119,7 @@ interface ConsoleWriter extends Bun.FileSink { flush(wait?: boolean): number | Promise; } -// The signature of a builtin stays on one line: bundle-functions.ts does not parse a parameter list with braces in it. -interface ConsoleWithWriter { - $writer: ConsoleWriter | undefined; - $writeAll: (this: ConsoleWriter, ...chunks: unknown[]) => number | Promise; -} - -export function write(this: ConsoleWithWriter, input) { +export function write(this: Console & { $writer: ConsoleWriter | undefined; $returnedWrite: unknown }, input) { if (!$isObject(this)) throw $ERR_INVALID_THIS("Console"); var writer = $getByIdDirectPrivate(this, "writer"); @@ -133,12 +127,44 @@ export function write(this: ConsoleWithWriter, input) { var length = $toLength(input?.length ?? 0); writer = Bun.stdout.writer({ highWaterMark: length > 65536 ? length : 65536 }); $putByIdDirectPrivate(this, "writer", writer); - // Writes every argument and flushes once, natively: there a failed write is an error value and a backed-up one - // is the sink's one pending operation, where here they would be a Promise each to tell apart and to handle. - $putByIdDirectPrivate(this, "writeAll", $newRustFunction("FileSink.rs", "consoleWrite", 1)); } - return $getByIdDirectPrivate(this, "writeAll").$apply(writer, arguments); + // A backed-up writer (FileSink) returns a Promise instead of a count: one Promise, the same for every write made + // while it is backed up, of the bytes those writes added. The caller gets a Promise of the total then, because + // awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung up) arrives. + // + // The sink hands out other Promises that the caller never gets: a write that fails on the spot returns a rejected + // one of its own, a flush that cannot finish returns one, and a throw (an argument that is not something to write, + // a flush that fails) loses the one in hand. Nobody can handle those, so they are marked handled. Not the one an + // earlier call returned, which a later write or flush can hand back: that one is its caller's. + var wrote = 0; + var pending: Promise | undefined; + var finished: unknown; + const returned = $getByIdDirectPrivate(this, "returnedWrite"); + const count = $argumentCount(); + var i = 0; + try { + do { + const result = writer.write(arguments[i]); + if (typeof result === "number") wrote += result; + else if ($isPromise(result)) { + if (pending !== undefined && pending !== result && pending !== returned) $pokePromiseAsHandled(pending); + pending = result; + } else finished = result; + } while (++i < count); + + const flushed = writer.flush(true); + if ($isPromise(flushed) && flushed !== pending && flushed !== returned) $pokePromiseAsHandled(flushed); + } catch (e) { + if (pending !== undefined && pending !== returned) $pokePromiseAsHandled(pending); + throw e; + } + + // A finished sink's write() is `true`, and that is what a call that wrote nothing to one returns. + if (pending === undefined) return wrote === 0 && finished !== undefined ? finished : wrote; + $putByIdDirectPrivate(this, "returnedWrite", pending); + if (wrote === 0) return pending; + return pending.$then(n => wrote + n); } // This is the `console.Console` constructor. It is mostly copied from Node. diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index a005d85fdf55..a5ae5a945b37 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -1412,12 +1412,6 @@ impl FileSink { // `Sink.JSSink(@This(), "FileSink")` — generic-fn-returning-type → monomorphized type alias. pub(crate) type JSSink = crate::webcore::sink::JSSink; -/// `console.write()` (src/js/builtins/ConsoleObject.ts): `this` is the console's writer, a `Bun.stdout.writer()`. -#[bun_jsc::host_fn] -pub(crate) fn console_write(global: &JSGlobalObject, frame: &CallFrame) -> JsResult { - JSSink::js_write_all(global, frame) -} - crate::impl_js_sink_abi!(FileSink, "FileSink"); // `JsSinkType` impl: routes the codegen `FileSink__*` thunks (via @@ -1470,10 +1464,6 @@ impl crate::webcore::sink::JsSinkType for FileSink { fn pending_state_is_pending(&self) -> bool { self.pending.get().state == streams::PendingState::Pending } - fn pending_operation(&mut self) -> Option<*mut streams::WritablePending> { - self.pending_state_is_pending() - .then(|| self.pending.as_ptr()) - } fn protect_js_wrapper(&mut self, global: &JSGlobalObject, this_value: JSValue) { Self::protect_js_wrapper(self, global, this_value) } diff --git a/src/runtime/webcore/Sink.rs b/src/runtime/webcore/Sink.rs index dfe886990495..7821789b78fd 100644 --- a/src/runtime/webcore/Sink.rs +++ b/src/runtime/webcore/Sink.rs @@ -396,10 +396,6 @@ pub(crate) trait JsSinkType: Sized + JsSinkAbi { fn get_pending_error(&mut self) -> Option { None } - /// The operation whose Promise is outstanding, for a sink that settles writes through one. - fn pending_operation(&mut self) -> Option<*mut streams::WritablePending> { - None - } fn source(&mut self) -> Option<&mut SourceHandle> { None } @@ -453,19 +449,6 @@ pub(crate) trait JsSinkType: Sized + JsSinkAbi { // no lut entry and no C++ caller. // ────────────────────────────────────────────────────────────────────────── -/// What a flush gave back, for JS: its value, or its error thrown. Outside `JSSink`, which is compiled once -/// per sink type, because none of this depends on the type. -fn flushed_to_js( - global: &crate::webcore::jsc::JSGlobalObject, - flushed: sys::Result, -) -> crate::webcore::jsc::JsResult { - use bun_sys_jsc::ErrorJsc; - match flushed { - sys::Result::Ok(value) => Ok(value), - sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), - } -} - impl JSSink { /// `JSSink.getThis` — recover `&mut JSSink` from `callframe.this()` or /// throw the appropriate detached/cast-failed error. @@ -537,26 +520,7 @@ impl JSSink { ))); } - Ok(Self::write_value(this, global, frame.argument(0))?.to_js(&cx)) - } - - /// The Promise of the sink's pending operation, when it has one that JS holds. - fn pending_promise(this: &mut JSSink) -> Option { - let operation = this.sink.pending_operation()?; - // SAFETY: `operation` is the sink's pending slot, which the sink owns and `this` keeps alive. - match unsafe { &(*operation).future } { - streams::WritableFuture::Promise { strong, .. } => Some(strong.value()), - streams::WritableFuture::None => None, - } - } - - /// One chunk from JS into the sink: bytes as they are, a string by its encoding. - fn write_value( - this: &mut JSSink, - global: &crate::webcore::jsc::JSGlobalObject, - arg: crate::webcore::jsc::JSValue, - ) -> crate::webcore::jsc::JsResult { - use streams::result::Writable; + let arg = frame.argument(0); arg.ensure_still_alive(); let _keep = bun_jsc::EnsureStillAlive(arg); @@ -570,11 +534,14 @@ impl JSSink { if let Some(buffer) = arg.as_array_buffer(global) { let slice = buffer.slice(); if slice.is_empty() { - return Ok(Writable::Owned(0)); + return Ok(JSValue::js_number(0.0)); } // Borrowed view over GC-kept buffer for the duration of the call. let data = bun_ptr::RawSlice::new(slice); - return Ok(this.sink.write_bytes(&streams::Result::Temporary(data))); + return Ok(this + .sink + .write_bytes(&streams::Result::Temporary(data)) + .to_js(&cx)); } if !arg.is_string() { @@ -586,132 +553,24 @@ impl JSSink { let view = arg.to_js_string_view(global)?; if view.is_empty() { - return Ok(Writable::Owned(0)); + return Ok(JSValue::js_number(0.0)); } if view.is_utf16() { let utf16 = view.utf16(); let bytes: &[u8] = bytemuck::cast_slice(utf16); let data = bun_ptr::RawSlice::new(bytes); - return Ok(this.sink.write_utf16(&streams::Result::Temporary(data))); + return Ok(this + .sink + .write_utf16(&streams::Result::Temporary(data)) + .to_js(&cx)); } let data = bun_ptr::RawSlice::new(view.latin1()); - Ok(this.sink.write_latin1(&streams::Result::Temporary(data))) - } - - /// `console.write(...chunks)`: every argument written in order, one flush, one result. - /// - /// The sink's results are combined here, where a failed write is still an error value and a backed-up write - /// is still the sink's one pending operation. Turned into JS values one write at a time they are a rejected - /// Promise each, and a Promise the caller has to recognise as one it already holds. - pub(crate) fn js_write_all( - global: &crate::webcore::jsc::JSGlobalObject, - frame: &crate::webcore::jsc::CallFrame, - ) -> crate::webcore::jsc::JsResult { - use crate::webcore::jsc::JSValue; - use streams::result::Writable; - let cx = global.js_thread_of_caller(frame); - bun_core::mark_binding!(); - let Some(this) = Self::get_this(global, frame)? else { - return Ok(JSValue::js_number(0.0)); - }; - - if let Some(err) = this.sink.get_pending_error() { - return Err(global.throw_value(err)); - } - - let mut wrote: u64 = 0; - // An operation an earlier call left pending: its Promise is that caller's, and so is what it has consumed. - let promise_before = Self::pending_promise(this); - // SAFETY: the sink's pending slot, which the sink owns and `this` keeps alive. - let consumed_before = this + Ok(this .sink - .pending_operation() - .map_or(0, |operation| unsafe { (*operation).consumed }); - // This call's pending operation: the slot, its Promise, and what it has consumed as of the last write. - let mut pending: Option<(*mut streams::WritablePending, JSValue, u64)> = None; - // No argument is one `undefined` chunk, which write_value() rejects as write() does. - for i in 0..frame.arguments_count().max(1) as usize { - let result = match Self::write_value(this, global, frame.argument(i)) { - Ok(result) => result, - Err(thrown) => { - // A Promise made in this call never reaches the caller now: nobody could handle its rejection. - if let Some((_, promise, _)) = - pending.filter(|held| Some(held.1) != promise_before) - { - if let Some(promise) = promise.as_promise() { - // SAFETY: the live JSPromise cell `as_promise` returned, which `promise` keeps alive. - unsafe { (*promise).set_handled() }; - } - } - return Err(thrown); - } - }; - // That write can have settled the operation on the spot, and settling runs microtasks, where a nested - // console.write() can leave the slot pending again: compare Promises, before `to_js` below arms the - // slot. What this call gave the operation carries on as a count. - if let Some((_, promise, consumed)) = pending { - if Self::pending_promise(this) != Some(promise) { - wrote += consumed - - if Some(promise) == promise_before { - consumed_before - } else { - 0 - }; - pending = None; - } - } - match result { - // With an operation pending, the sink gives it the error and returns it instead of this. - Writable::Err(err) => { - // Or the auto flusher fails on a short chunk still buffered, and finishes the sink. - let _ = Self::flush_sink(this, &cx, true); - return Ok(Writable::Err(err).to_js(&cx)); - } - Writable::Pending(operation) => { - let promise = Writable::Pending(operation).to_js(&cx); - // SAFETY: the sink's pending slot, as above. - pending = Some((operation, promise, unsafe { (*operation).consumed })); - } - Writable::Owned(n) - | Writable::OwnedAndDone(n) - | Writable::Temporary(n) - | Writable::Backpressure(n) => wrote += n, - // A finished sink: nothing written. - Writable::Done => {} - } - } - - let Some((operation, promise, _)) = pending else { - let promise_before_flush = Self::pending_promise(this); - let flushed = flushed_to_js(global, Self::flush_sink(this, &cx, true))?; - let armed_by_flush = Self::pending_promise(this) - .is_some_and(|promise| promise == flushed && Some(promise) != promise_before_flush); - let Some(operation) = this.sink.pending_operation().filter(|_| armed_by_flush) else { - return Ok(JSValue::from(wrote)); - }; - // The flush left an operation pending. The caller gets its Promise, which would have no holder when - // the write fails, resolving to this call's total: what the flush did push out is part of that. - // SAFETY: the sink's pending slot, as above. - let operation = unsafe { &mut *operation }; - operation.consumed = wrote; - if let Writable::Owned(total) = &mut operation.result { - *total = wrote; - } - return Ok(flushed); - }; - // The counts join the total: a chunk written before the sink backed up, or a small one buffered beside - // the operation. - // SAFETY: the sink's pending slot, as above; nothing has run since the check after the last write. - let operation = unsafe { &mut *operation }; - operation.consumed += wrote; - // Not when a write failed beside the operation: `result` is that error then. - if let Writable::Owned(total) = &mut operation.result { - *total = operation.consumed; - } - flushed_to_js(global, Self::flush_sink(this, &cx, true))?; - Ok(promise) + .write_latin1(&streams::Result::Temporary(data)) + .to_js(&cx)) } /// `${abi_name}__flush` host-fn body. @@ -720,6 +579,8 @@ impl JSSink { frame: &crate::webcore::jsc::CallFrame, ) -> crate::webcore::jsc::JsResult { let cx = global.js_thread_of_caller(frame); + use crate::webcore::jsc::JSValue; + use bun_sys_jsc::ErrorJsc; bun_core::mark_binding!(); let Some(this) = Self::get_this(global, frame)? else { @@ -730,23 +591,20 @@ impl JSSink { return Err(global.throw_value(err)); } - let wait = frame.arguments_count() > 0 - && frame.argument(0).is_boolean() - && frame.argument(0).as_boolean(); - flushed_to_js(global, Self::flush_sink(this, &cx, wait)) - } - - fn flush_sink( - this: &mut JSSink, - cx: &bun_jsc::JsThread<'_>, - wait: bool, - ) -> sys::Result { if T::HAS_FLUSH_FROM_JS { - return this.sink.flush_from_js(cx, wait); + let wait = frame.arguments_count() > 0 + && frame.argument(0).is_boolean() + && frame.argument(0).as_boolean(); + return match this.sink.flush_from_js(&cx, wait) { + sys::Result::Ok(value) => Ok(value), + sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), + }; + } + + match this.sink.flush() { + sys::Result::Ok(()) => Ok(JSValue::UNDEFINED), + sys::Result::Err(err) => Err(global.throw_value(err.to_js(global)?)), } - this.sink - .flush() - .map(|()| crate::webcore::jsc::JSValue::UNDEFINED) } /// `${abi_name}__start` host-fn body. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 3ad7340e6507..9bbaea669774 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -199,14 +199,14 @@ try { }); // The reader has stalled and the pipe is full, so a short console.write() is buffered and its flush cannot push -// the buffer out. That left the sink with a pending Promise while console.write() returned the byte count: when -// the reader then hung up, the Promise was rejected with nobody holding it, and a script that awaited every -// console.write() still died of an unhandled rejection. +// the buffer out. The flush leaves the sink with a pending Promise, which console.write() drops as it always has: +// it returns the byte count. When the reader then hung up, that Promise was rejected with nobody holding it, and a +// script that awaited every console.write() still died of an unhandled rejection. // // stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills // it and stays backed up, so console's own writer has nothing pending when the short write happens. test.concurrent.skipIf(isWindows)( - "an awaited console.write to a full pipe fails when the stalled reader hangs up", + "a short console.write to a full pipe leaves no unhandled rejection when the stalled reader hangs up", async () => { using dir = tempDir("console-write-stalled", {}); const fifo = join(String(dir), "stdout.fifo"); @@ -257,7 +257,7 @@ try { stderr += decoder.decode(value, { stream: true }); } - expect(stderr).toBe("READY Promise\ncaught EPIPE\n"); + expect(stderr).toBe("READY 5\nresolved\n"); expect(await proc.exited).toBe(0); } finally { if (readEndOpen) closeSync(readEnd); From a64b952f81b7e50a64e87d7dcc4b03e86945fd04 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 23:54:27 +0000 Subject: [PATCH 21/27] console-write test: the later write failing is how it always was --- test/js/bun/console/console-write.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 9bbaea669774..f4bbea8a49e3 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -149,8 +149,7 @@ try { // The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and checks with // a writer of its own that the pipe really is broken, so the test cannot pass by the writes simply being queued. // -// A later console.write() still fails. With a short argument left in the buffer by the failed call, the sink was -// marked finished when that was flushed, and later writes reported 0 bytes and no error. +// A later console.write() to the broken pipe fails too. const big = `Buffer.alloc(1024 * 1024, "a").toString()`; test.concurrent.each([ ["two large arguments", `${big}, ${big}`], From 4e0d147fd4b255ba5966dc1a8cce9f76cf96f54d Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 23:55:23 +0000 Subject: [PATCH 22/27] fs.promises.writeFile test: short and long chunks from an async iterator writeFile adds up what each write reports and truncates the file to that total, so the streaming writer counting a buffered chunk twice left the file too long, ending in NUL bytes: 50004 bytes for 50002 on Bun 1.4.2. --- ...-promises-writeFile-async-iterator.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/js/node/fs/fs-promises-writeFile-async-iterator.test.ts b/test/js/node/fs/fs-promises-writeFile-async-iterator.test.ts index 6c497b97432d..8a12710768c2 100644 --- a/test/js/node/fs/fs-promises-writeFile-async-iterator.test.ts +++ b/test/js/node/fs/fs-promises-writeFile-async-iterator.test.ts @@ -51,3 +51,23 @@ test("fs.promises.writeFile async iterator throws on invalid input", async () => expect(() => writeFile(String(dir), fn)).toThrow(); expect(fn[Symbol.asyncIterator]).not.toBeCalled(); }); + +// writeFile adds up what each write reports and truncates the file to that total. A short chunk is buffered and +// reported as written, and the large chunk after it used to report the short one's bytes again, so the total was +// too big and the file ended in NUL bytes. +test("fs.promises.writeFile async iterator writes exactly the chunks, short and long", async () => { + await using dir = tempDir("fs-promises-writeFile-async-iterator-sizes", {}); + const path = dir + "/mixed.bin"; + const chunks = ["a", Buffer.alloc(40000, "b").toString(), "c", Buffer.alloc(10000, "é").toString()]; + + await writeFile( + path, + (async function* () { + yield* chunks; + })(), + ); + + const written = await Bun.file(path).bytes(); + expect(written.length).toBe(1 + 40000 + 1 + 10000); + expect(Buffer.from(written).equals(Buffer.from(chunks.join("")))).toBe(true); +}); From 92fb8621f162ea828942bf94f4a8672a4f60c392 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Mon, 21 Sep 2026 23:59:14 +0000 Subject: [PATCH 23/27] console.write: only a Promise that is already rejected is marked handled The sink hands its pending Promise out again, to a later write, to a flush, to the next call. Marked handled while nobody held it, it reached a caller who then let it go, and the rejection that caller was always told about was not reported. A write that fails on the spot returns a rejected Promise of its own, and the sink never hands out a settled one again, so that is the one that is safe to mark, and the one the broken pipe cases lose. A Promise still pending when a call throws, or that a flush leaves behind, is as it was: those two cases are not fixed here and not changed. --- src/js/builtins/ConsoleObject.ts | 17 +-- test/js/bun/console/console-write.test.ts | 158 ++-------------------- 2 files changed, 20 insertions(+), 155 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index e0c99e7c80be..0bc2b64d0371 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -133,10 +133,11 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret // while it is backed up, of the bytes those writes added. The caller gets a Promise of the total then, because // awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung up) arrives. // - // The sink hands out other Promises that the caller never gets: a write that fails on the spot returns a rejected - // one of its own, a flush that cannot finish returns one, and a throw (an argument that is not something to write, - // a flush that fails) loses the one in hand. Nobody can handle those, so they are marked handled. Not the one an - // earlier call returned, which a later write or flush can hand back: that one is its caller's. + // A write that fails on the spot (the pipe is already broken) returns a rejected Promise of its own instead. The + // caller gets the last one. Any other is lost: replaced by a later argument's, or dropped when a later argument + // or the flush throws. Nobody can handle its rejection, so it is marked handled. Only one that is already + // rejected: the sink hands its pending Promise out again, to callers whose rejection has to be reported. And not + // one an earlier call returned, which is that caller's. var wrote = 0; var pending: Promise | undefined; var finished: unknown; @@ -148,15 +149,15 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret const result = writer.write(arguments[i]); if (typeof result === "number") wrote += result; else if ($isPromise(result)) { - if (pending !== undefined && pending !== result && pending !== returned) $pokePromiseAsHandled(pending); + if (pending !== undefined && pending !== result && pending !== returned && $isPromiseRejected(pending)) + $pokePromiseAsHandled(pending); pending = result; } else finished = result; } while (++i < count); - const flushed = writer.flush(true); - if ($isPromise(flushed) && flushed !== pending && flushed !== returned) $pokePromiseAsHandled(flushed); + writer.flush(true); } catch (e) { - if (pending !== undefined && pending !== returned) $pokePromiseAsHandled(pending); + if (pending !== undefined && pending !== returned && $isPromiseRejected(pending)) $pokePromiseAsHandled(pending); throw e; } diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index f4bbea8a49e3..573bcb5a1aa3 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -1,8 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tempDir } from "harness"; -import { mkfifo } from "mkfifo"; -import { closeSync, constants, openSync } from "node:fs"; -import { join } from "node:path"; +import { bunEnv, bunExe, isWindows } from "harness"; test("console.write rejects a non-object this", async () => { await using proc = Bun.spawn({ @@ -141,10 +138,10 @@ try { expect(exitCode).toBe(0); }); -// Every write to a pipe that is already broken fails on the spot. console.write() called the sink's write() once -// per argument and kept the last Promise it got back, so a script that awaited it and caught the error still died -// of the unhandled rejection of another argument's write (or, with a short last argument, of the flush throwing -// past the Promise it had). +// Every write to a pipe that is already broken fails on the spot, with a rejected Promise of its own. +// console.write() kept the last one, and lost that too when a short argument's flush threw or a later argument +// was not something to write. A script that awaited it and caught the error still died of the unhandled rejection +// of a Promise it was never given. // // The child blocks in a synchronous read of stdin until the parent has closed its end of stdout, and checks with // a writer of its own that the pipe really is broken, so the test cannot pass by the writes simply being queued. @@ -152,10 +149,11 @@ try { // A later console.write() to the broken pipe fails too. const big = `Buffer.alloc(1024 * 1024, "a").toString()`; test.concurrent.each([ - ["two large arguments", `${big}, ${big}`], - ["a large argument, then a short one", `${big}, "\\n"`], - ["a short argument, then a large one", `"\\n", ${big}`], -])("an awaited console.write to a broken pipe fails once: %s", async (_label, args) => { + ["two large arguments", `${big}, ${big}`, "EPIPE"], + ["a large argument, then a short one", `${big}, "\\n"`, "EPIPE"], + ["a short argument, then a large one", `"\\n", ${big}`, "EPIPE"], + ["a large argument, then one that is not something to write", `${big}, 123`, "ERR_INVALID_ARG_TYPE"], +])("an awaited console.write to a broken pipe fails once: %s", async (_label, args, code) => { await using proc = Bun.spawn({ cmd: [ bunExe(), @@ -193,140 +191,6 @@ try { await proc.stdin.end(); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect(stderr).toBe("pipe already broken: true\ncaught EPIPE\nlater: caught EPIPE\n"); + expect(stderr).toBe(`pipe already broken: true\ncaught ${code}\nlater: caught EPIPE\n`); expect(exitCode).toBe(0); }); - -// The reader has stalled and the pipe is full, so a short console.write() is buffered and its flush cannot push -// the buffer out. The flush leaves the sink with a pending Promise, which console.write() drops as it always has: -// it returns the byte count. When the reader then hung up, that Promise was rejected with nobody holding it, and a -// script that awaited every console.write() still died of an unhandled rejection. -// -// stdout is a FIFO whose read end this test holds open and never reads. Another writer on the same pipe fills -// it and stays backed up, so console's own writer has nothing pending when the short write happens. -test.concurrent.skipIf(isWindows)( - "a short console.write to a full pipe leaves no unhandled rejection when the stalled reader hangs up", - async () => { - using dir = tempDir("console-write-stalled", {}); - const fifo = join(String(dir), "stdout.fifo"); - mkfifo(fifo, 0o600); - const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - const writeEnd = openSync(fifo, constants.O_WRONLY); - let readEndOpen = true; - try { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` -process.on("unhandledRejection", e => { - console.error("unhandledRejection " + e?.code); -}); -const filler = Bun.stdout.writer().write(Buffer.alloc(4 * 1024 * 1024, "f").toString()); -filler.catch(() => {}); -const result = console.write("line\\n"); -console.error("READY " + (result instanceof Promise ? "Promise" : result)); -try { - await result; - console.error("resolved"); -} catch (e) { - console.error("caught " + e.code); -} -`, - ], - env: bunEnv, - stdout: writeEnd, - stderr: "pipe", - }); - closeSync(writeEnd); - - const reader = proc.stderr.getReader(); - const decoder = new TextDecoder(); - let stderr = ""; - while (!stderr.includes("READY")) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - closeSync(readEnd); - readEndOpen = false; - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - - expect(stderr).toBe("READY 5\nresolved\n"); - expect(await proc.exited).toBe(0); - } finally { - if (readEndOpen) closeSync(readEnd); - } - }, -); - -// console.write(big, 123) throws for the argument that is not something to write, after the first one has become -// the sink's pending write. That write's Promise was made inside the call and never reaches the caller, so when -// the write later fails nobody can have handled it: it must not be reported as an unhandled rejection. -// -// stdout is a FIFO whose read end this test holds open and never reads, then closes. -test.concurrent.skipIf(isWindows)( - "console.write that throws for a later argument leaves no unhandled rejection", - async () => { - using dir = tempDir("console-write-bad-argument", {}); - const fifo = join(String(dir), "stdout.fifo"); - mkfifo(fifo, 0o600); - const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - const writeEnd = openSync(fifo, constants.O_WRONLY); - let readEndOpen = true; - try { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - ` -process.on("unhandledRejection", e => { - console.error("unhandledRejection " + e?.code); -}); -try { - console.write(Buffer.alloc(4 * 1024 * 1024, "x").toString(), 123); - console.error("no throw"); -} catch (e) { - console.error("caught " + e.code); -} -console.error("READY"); -process.stdin.once("data", () => console.error("end")); -`, - ], - env: bunEnv, - stdin: "pipe", - stdout: writeEnd, - stderr: "pipe", - }); - closeSync(writeEnd); - - const reader = proc.stderr.getReader(); - const decoder = new TextDecoder(); - let stderr = ""; - while (!stderr.includes("READY")) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - closeSync(readEnd); - readEndOpen = false; - // The child's event loop sees the hang-up before it sees this byte on stdin. - proc.stdin.write("x"); - await proc.stdin.end(); - for (;;) { - const { value, done } = await reader.read(); - if (done) break; - stderr += decoder.decode(value, { stream: true }); - } - - expect(stderr).toBe("caught ERR_INVALID_ARG_TYPE\nREADY\nend\n"); - expect(await proc.exited).toBe(0); - } finally { - if (readEndOpen) closeSync(readEnd); - } - }, -); From 44600fe87e324b2629582e5694d0643130673594 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 22 Sep 2026 00:04:02 +0000 Subject: [PATCH 24/27] console.write: a write that settled before the call was over counts console.write keeps the last Promise a write returned. A short write beside a pending one can settle it early, and a later large argument then starts a pending write of its own: the first one's bytes were left out of the total, which showed up as a total 70000 short on macOS with a slow reader. An earlier Promise has always settled by the time a write returns a different one, since the sink has one pending write at a time, so a fulfilled one's value is read and added. --- src/js/builtins/ConsoleObject.ts | 17 +++--- test/js/bun/console/console-write.test.ts | 74 ++++++++++++++++++++++- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index 0bc2b64d0371..a3277fe2275f 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -133,11 +133,12 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret // while it is backed up, of the bytes those writes added. The caller gets a Promise of the total then, because // awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung up) arrives. // - // A write that fails on the spot (the pipe is already broken) returns a rejected Promise of its own instead. The - // caller gets the last one. Any other is lost: replaced by a later argument's, or dropped when a later argument - // or the flush throws. Nobody can handle its rejection, so it is marked handled. Only one that is already - // rejected: the sink hands its pending Promise out again, to callers whose rejection has to be reported. And not - // one an earlier call returned, which is that caller's. + // The caller gets the last Promise. An earlier one has settled by the time a write returns a different one: the + // sink has one pending write at a time. Fulfilled (a short write beside it can settle it early), its bytes join + // the total. Rejected (the pipe is already broken, and every write fails on the spot), it is lost, as is the last + // one when a later argument or the flush throws: nobody can handle its rejection, so it is marked handled. Only + // one that is already rejected, since the sink hands its pending Promise out again, to callers whose rejection + // has to be reported. And not one an earlier call returned, which is that caller's. var wrote = 0; var pending: Promise | undefined; var finished: unknown; @@ -149,8 +150,10 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret const result = writer.write(arguments[i]); if (typeof result === "number") wrote += result; else if ($isPromise(result)) { - if (pending !== undefined && pending !== result && pending !== returned && $isPromiseRejected(pending)) - $pokePromiseAsHandled(pending); + if (pending !== undefined && pending !== result) { + if ($isPromiseFulfilled(pending)) wrote += $peekPromiseSettledValue(pending)!; + else if (pending !== returned && $isPromiseRejected(pending)) $pokePromiseAsHandled(pending); + } pending = result; } else finished = result; } while (++i < count); diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index 573bcb5a1aa3..cf2e3375d9d5 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -1,5 +1,8 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { mkfifo } from "mkfifo"; +import { closeSync, constants, openSync, readSync } from "node:fs"; +import { join } from "node:path"; test("console.write rejects a non-object this", async () => { await using proc = Bun.spawn({ @@ -194,3 +197,72 @@ try { expect(stderr).toBe(`pipe already broken: true\ncaught ${code}\nlater: caught EPIPE\n`); expect(exitCode).toBe(0); }); + +// console.write(a, "s", b): `a` overflows the pipe by a little and is pending; the short "s" beside it settles that +// write's Promise early, and `b` then starts a pending write of its own. Only the last Promise was kept, so the +// call resolved to a total without `a`. +// +// Linux, because the first argument has to be sized from the pipe's capacity (F_GETPIPE_SZ). stdout is a FIFO that +// this test does not read until the child has made the call. +test.concurrent.skipIf(!isLinux)("console.write counts a write that settled before the call was over", async () => { + using dir = tempDir("console-write-total", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const { dlopen } = require("bun:ffi"); +const F_GETPIPE_SZ = 1032; +const capacity = dlopen("libc.so.6", { fcntl: { args: ["int", "int"], returns: "int" } }).symbols.fcntl(1, F_GETPIPE_SZ); +const a = Buffer.alloc(capacity + 100, "a").toString(); +const b = Buffer.alloc(1024 * 1024, "b").toString(); +const total = console.write(a, "s", b); +console.error("expected " + (a.length + 1 + b.length)); +console.error("resolved " + (await total)); +`, + ], + env: bunEnv, + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("\n")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + const expected = Number(/expected (\d+)/.exec(stderr)?.[1]); + + const chunk = Buffer.alloc(1024 * 1024); + let arrived = 0; + while (arrived < expected) { + try { + const n = readSync(readEnd, chunk); + if (n === 0) break; + arrived += n; + } catch (e: any) { + if (e.code !== "EAGAIN") throw e; + await new Promise(resolve => setImmediate(resolve)); + } + } + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + + expect({ stderr, arrived }).toEqual({ stderr: `expected ${expected}\nresolved ${expected}\n`, arrived: expected }); + expect(await proc.exited).toBe(0); + } finally { + closeSync(readEnd); + } +}); From ca1353b655ecbcbdcdecb474f71cf3d99ff46498 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Tue, 22 Sep 2026 00:27:13 +0000 Subject: [PATCH 25/27] console-write test: find the C library the way the harness does The Alpine lanes have no libc.so.6, so the Linux-only test could not read the pipe's capacity there. It takes the path from libcPathForDlopen(), as test/mkfifo.ts does. --- test/js/bun/console/console-write.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index cf2e3375d9d5..c8e35343c11f 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, libcPathForDlopen, tempDir } from "harness"; import { mkfifo } from "mkfifo"; import { closeSync, constants, openSync, readSync } from "node:fs"; import { join } from "node:path"; @@ -218,7 +218,8 @@ test.concurrent.skipIf(!isLinux)("console.write counts a write that settled befo ` const { dlopen } = require("bun:ffi"); const F_GETPIPE_SZ = 1032; -const capacity = dlopen("libc.so.6", { fcntl: { args: ["int", "int"], returns: "int" } }).symbols.fcntl(1, F_GETPIPE_SZ); +const libc = ${JSON.stringify(libcPathForDlopen())}; +const capacity = dlopen(libc, { fcntl: { args: ["int", "int"], returns: "int" } }).symbols.fcntl(1, F_GETPIPE_SZ); const a = Buffer.alloc(capacity + 100, "a").toString(); const b = Buffer.alloc(1024 * 1024, "b").toString(); const total = console.write(a, "s", b); From 9975cb5dd940e20af370c6aee0a67fe8a6f72ee6 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 23 Sep 2026 01:53:11 +0000 Subject: [PATCH 26/27] console.write: what is not a Promise adds up as it did in 1.4.2 1.4.2 took the first write's result and `+=` each later one. The merged rewrite only added numbers, so a finished sink, whose write() returns `true`, gave `true` for several arguments where the release gives 2 or 3. The loop does the release's arithmetic again; only a Promise is treated differently, which is the bug. A test holds the finished sink's values in place: [true, 2, 3, 0, 1], the same on 1.4.2. --- src/js/builtins/ConsoleObject.ts | 39 ++++++++----- test/js/bun/console/console-write.test.ts | 69 +++++++++++++++++++++++ 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index a3277fe2275f..4d02354dda06 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -129,9 +129,13 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret $putByIdDirectPrivate(this, "writer", writer); } + // What is not a Promise is added up as it always was: the first result, then `+=` each later one. (A finished + // sink's write() is `true`, so two of them make 2.) + // // A backed-up writer (FileSink) returns a Promise instead of a count: one Promise, the same for every write made - // while it is backed up, of the bytes those writes added. The caller gets a Promise of the total then, because - // awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung up) arrives. + // while it is backed up, of the bytes those writes added. `+` made a string of it. The caller gets a Promise of + // the total, because awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung + // up) arrives. // // The caller gets the last Promise. An earlier one has settled by the time a write returns a different one: the // sink has one pending write at a time. Fulfilled (a short write beside it can settle it early), its bytes join @@ -139,23 +143,28 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret // one when a later argument or the flush throws: nobody can handle its rejection, so it is marked handled. Only // one that is already rejected, since the sink hands its pending Promise out again, to callers whose rejection // has to be reported. And not one an earlier call returned, which is that caller's. - var wrote = 0; + var wrote: number | undefined; var pending: Promise | undefined; - var finished: unknown; const returned = $getByIdDirectPrivate(this, "returnedWrite"); const count = $argumentCount(); var i = 0; try { do { - const result = writer.write(arguments[i]); - if (typeof result === "number") wrote += result; - else if ($isPromise(result)) { - if (pending !== undefined && pending !== result) { - if ($isPromiseFulfilled(pending)) wrote += $peekPromiseSettledValue(pending)!; - else if (pending !== returned && $isPromiseRejected(pending)) $pokePromiseAsHandled(pending); + var result: number | Promise = writer.write(arguments[i]); + if ($isPromise(result)) { + if (pending === undefined || pending === result) { + pending = result; + continue; } + const earlier = pending; pending = result; - } else finished = result; + if (!$isPromiseFulfilled(earlier)) { + if (earlier !== returned && $isPromiseRejected(earlier)) $pokePromiseAsHandled(earlier); + continue; + } + result = $peekPromiseSettledValue(earlier)!; + } + wrote = wrote === undefined ? result : wrote + result; } while (++i < count); writer.flush(true); @@ -164,11 +173,11 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret throw e; } - // A finished sink's write() is `true`, and that is what a call that wrote nothing to one returns. - if (pending === undefined) return wrote === 0 && finished !== undefined ? finished : wrote; + if (pending === undefined) return wrote; $putByIdDirectPrivate(this, "returnedWrite", pending); - if (wrote === 0) return pending; - return pending.$then(n => wrote + n); + if (wrote === undefined || wrote === 0) return pending; + const counted = wrote; + return pending.$then(n => counted + n); } // This is the `console.Console` constructor. It is mostly copied from Node. diff --git a/test/js/bun/console/console-write.test.ts b/test/js/bun/console/console-write.test.ts index c8e35343c11f..a01967f93381 100644 --- a/test/js/bun/console/console-write.test.ts +++ b/test/js/bun/console/console-write.test.ts @@ -267,3 +267,72 @@ console.error("resolved " + (await total)); closeSync(readEnd); } }); + +// What is not a Promise is added up as released: the first write's result, then `+=` each later one. A sink that +// has finished after a failure returns `true` from write(), so one argument gives `true` and two give 2. Not a +// count of anything, but it is what these calls return, and scripts that keep writing after their reader has gone +// see it. +// +// stdout is a FIFO whose read end this test holds open without reading, then closes. The child fills the pipe and +// leaves one short chunk buffered, whose flush cannot finish. When the read end goes, the sink finishes, and the +// one sign of it a script gets is the unhandled rejection of that flush's Promise, which console.write() dropped. +// (A script that writes before the event loop has seen the hang-up gets EPIPE thrown from every call instead.) +test.concurrent.skipIf(isWindows)("console.write to a finished sink adds up what write() returns", async () => { + using dir = tempDir("console-write-finished", {}); + const fifo = join(String(dir), "stdout.fifo"); + mkfifo(fifo, 0o600); + const readEnd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + const writeEnd = openSync(fifo, constants.O_WRONLY); + let readEndOpen = true; + try { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const fs = require("node:fs"); +const finished = new Promise(resolve => process.on("unhandledRejection", resolve)); +console.write("0"); +for (const size of [1024, 1]) { + try { + for (;;) fs.writeSync(1, Buffer.alloc(size, "f")); + } catch {} +} +console.write("short"); +console.error("READY"); +fs.readSync(0, Buffer.alloc(1)); +await finished; +console.error(JSON.stringify([console.write("x"), console.write("x", "y"), console.write("x", "y", "z"), console.write(""), console.write("", "x")])); +`, + ], + env: bunEnv, + stdin: "pipe", + stdout: writeEnd, + stderr: "pipe", + }); + closeSync(writeEnd); + + const reader = proc.stderr.getReader(); + const decoder = new TextDecoder(); + let stderr = ""; + while (!stderr.includes("READY")) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + closeSync(readEnd); + readEndOpen = false; + proc.stdin.write("x"); + await proc.stdin.end(); + for (;;) { + const { value, done } = await reader.read(); + if (done) break; + stderr += decoder.decode(value, { stream: true }); + } + + expect(stderr).toBe("READY\n" + JSON.stringify([true, 2, 3, 0, 1]) + "\n"); + expect(await proc.exited).toBe(0); + } finally { + if (readEndOpen) closeSync(readEnd); + } +}); From 8ab6db262a69d805dd4296824920d16cefc03a69 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 23 Sep 2026 02:00:54 +0000 Subject: [PATCH 27/27] console.write: no private slot; a failed write's Promise is told apart when it arrives A write that fails on the spot returns a Promise of its own, already rejected when write() returns it. The sink's Promise is never that: the slot gives it up before settling it, so it is still pending whenever the sink hands it out. That is decided as each result arrives and kept in a local, so nothing has to be remembered between calls, and the sink's Promise is never marked handled whatever happens to it later. BunBuiltinNames.h is as on main again. --- src/js/builtins/BunBuiltinNames.h | 1 - src/js/builtins/ConsoleObject.ts | 35 +++++++++++++++---------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/js/builtins/BunBuiltinNames.h b/src/js/builtins/BunBuiltinNames.h index c4308b1a4752..2974058b0b6e 100644 --- a/src/js/builtins/BunBuiltinNames.h +++ b/src/js/builtins/BunBuiltinNames.h @@ -165,7 +165,6 @@ using namespace JSC; macro(requireMap) \ macro(requireNativeModule) \ macro(resolveSync) \ - macro(returnedWrite) \ macro(sameSite) \ macro(secure) \ macro(self) \ diff --git a/src/js/builtins/ConsoleObject.ts b/src/js/builtins/ConsoleObject.ts index 4d02354dda06..d28118ecce7a 100644 --- a/src/js/builtins/ConsoleObject.ts +++ b/src/js/builtins/ConsoleObject.ts @@ -119,7 +119,7 @@ interface ConsoleWriter extends Bun.FileSink { flush(wait?: boolean): number | Promise; } -export function write(this: Console & { $writer: ConsoleWriter | undefined; $returnedWrite: unknown }, input) { +export function write(this: Console & { $writer: ConsoleWriter | undefined }, input) { if (!$isObject(this)) throw $ERR_INVALID_THIS("Console"); var writer = $getByIdDirectPrivate(this, "writer"); @@ -137,31 +137,31 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret // the total, because awaiting it waits for the drain and is where a write error (EPIPE from a reader that hung // up) arrives. // - // The caller gets the last Promise. An earlier one has settled by the time a write returns a different one: the - // sink has one pending write at a time. Fulfilled (a short write beside it can settle it early), its bytes join - // the total. Rejected (the pipe is already broken, and every write fails on the spot), it is lost, as is the last - // one when a later argument or the flush throws: nobody can handle its rejection, so it is marked handled. Only - // one that is already rejected, since the sink hands its pending Promise out again, to callers whose rejection - // has to be reported. And not one an earlier call returned, which is that caller's. + // A write that fails on the spot (the pipe is already broken) returns a Promise of its own instead, already + // rejected. The sink's Promise is never that: it is still pending whenever the sink hands it out. The caller gets + // the last Promise, so a failed write's is lost when a later write returns another, or when a later argument or + // the flush throws. Nobody can handle its rejection then, so it is marked handled. The sink's Promise never is: + // the sink hands it out again, to callers whose rejection has to be reported. + // + // An earlier Promise of the sink's has settled by the time a write returns a different one, since the sink has + // one pending write at a time. Fulfilled (a short write beside it can settle it early), its bytes join the total. var wrote: number | undefined; var pending: Promise | undefined; - const returned = $getByIdDirectPrivate(this, "returnedWrite"); + var failedOnTheSpot = false; const count = $argumentCount(); var i = 0; try { do { var result: number | Promise = writer.write(arguments[i]); if ($isPromise(result)) { - if (pending === undefined || pending === result) { - pending = result; - continue; - } + if (pending === result) continue; const earlier = pending; + const earlierFailedOnTheSpot = failedOnTheSpot; pending = result; - if (!$isPromiseFulfilled(earlier)) { - if (earlier !== returned && $isPromiseRejected(earlier)) $pokePromiseAsHandled(earlier); - continue; - } + failedOnTheSpot = $isPromiseRejected(result); + if (earlier === undefined) continue; + if (earlierFailedOnTheSpot) $pokePromiseAsHandled(earlier); + if (!$isPromiseFulfilled(earlier)) continue; result = $peekPromiseSettledValue(earlier)!; } wrote = wrote === undefined ? result : wrote + result; @@ -169,12 +169,11 @@ export function write(this: Console & { $writer: ConsoleWriter | undefined; $ret writer.flush(true); } catch (e) { - if (pending !== undefined && pending !== returned && $isPromiseRejected(pending)) $pokePromiseAsHandled(pending); + if (failedOnTheSpot) $pokePromiseAsHandled(pending!); throw e; } if (pending === undefined) return wrote; - $putByIdDirectPrivate(this, "returnedWrite", pending); if (wrote === undefined || wrote === 0) return pending; const counted = wrote; return pending.$then(n => counted + n);