From 534cc2a843337fbf05f59ef32ef97c6e19c9232b Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Tue, 23 Jun 2026 01:45:55 +0100 Subject: [PATCH 1/8] streams: use Web IDL "a promise resolved with" semantics for callback results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Web IDL "a promise resolved with x" is `new Promise(r => r(x))` — always a fresh promise, with the thenable-assimilation hop observable in the spec's microtask ordering. `Promise.resolve(x)` returns x unchanged when x is already a native Promise and skips that hop; the whatwg/streams ref-impl's `promiseResolvedWith` explicitly avoids it for this reason. `shieldingPromiseResolve` (the wrapper every `$promiseInvokeOrNoop*` call goes through for start/pull/cancel/write/close/abort/transform/flush across all three controller types) used `Promise.$resolve(result)`, and `writableStreamDefaultControllerStart` wrapped its `startAlgorithm()` result the same way. When the result is already a Promise — TransformStream's `startAlgorithm` returns the constructor's startPromise capability — the short-circuit shifts `[[started]]` one microtask early relative to the spec, which is observable in the WPT transform-streams tests that depend on a controller-abort/cancel reaction observing the writable mid-"erroring" rather than already "errored". Both sites now go through `$newPromise()` + `$resolvePromise()`, matching the ref-impl. No callers depend on the identity short-circuit (every consumer only `.$then()`s the result). Claude-Session: https://claude.ai/code/session_01Qn8dDrArUZxc4towq11qzu --- src/js/builtins/StreamInternals.ts | 9 +++++++-- src/js/builtins/WritableStreamInternals.ts | 7 ++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/js/builtins/StreamInternals.ts b/src/js/builtins/StreamInternals.ts index daf9b96569b9..c411a3048c39 100644 --- a/src/js/builtins/StreamInternals.ts +++ b/src/js/builtins/StreamInternals.ts @@ -31,9 +31,14 @@ export function markPromiseAsHandled(promise: Promise) { $pokePromiseAsHandled(promise); } +// Web IDL "a promise resolved with x" — always a fresh promise (the +// assimilation hop when x is a thenable is observable in the spec's microtask +// ordering). Promise.$resolve would return x unchanged when x is already a +// native Promise and skip that hop; the streams ref-impl explicitly avoids +// Promise.resolve here for the same reason. export function shieldingPromiseResolve(result) { - const promise = Promise.$resolve(result); - if (promise.$then === undefined) promise.$then = $Promise.prototype.$then; + const promise = $newPromise(); + $resolvePromise(promise, result); return promise; } diff --git a/src/js/builtins/WritableStreamInternals.ts b/src/js/builtins/WritableStreamInternals.ts index 9fe4583c6407..5f79c9ff5f0d 100644 --- a/src/js/builtins/WritableStreamInternals.ts +++ b/src/js/builtins/WritableStreamInternals.ts @@ -587,7 +587,12 @@ export function writableStreamDefaultControllerStart(controller) { const startAlgorithm = $getByIdDirectPrivate(controller, "startAlgorithm"); $putByIdDirectPrivate(controller, "startAlgorithm", undefined); const stream = $getByIdDirectPrivate(controller, "stream"); - return Promise.$resolve(startAlgorithm.$call()).$then( + // Web IDL "a promise resolved with x" is always a fresh promise — when + // startAlgorithm() returns one (as TransformStream's does), the assimilation + // hop is observable in the spec's microtask ordering. Promise.$resolve would + // return the input promise unchanged and skip that hop. + const startResult = startAlgorithm.$call(); + return new Promise(resolve => resolve(startResult)).$then( () => { const state = $getByIdDirectPrivate(stream, "state"); $assert(state === "writable" || state === "erroring"); From 72b45a26088c6e954762c1a4b9ad4d18f4581dd8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 23 Jun 2026 01:31:27 +0000 Subject: [PATCH 2/8] streams: make shieldingPromiseResolve tamper-proof for native promises 534cc2a843 switched shieldingPromiseResolve to $newPromise() + $resolvePromise(p, result) to get the Web IDL 'a promise resolved with' two-hop microtask ordering the WPT transform-streams/cancel tests depend on. But $resolvePromise does thenable assimilation via Get(result, 'then') and invokes it when it differs from the builtin, so a monkey-patched Promise.prototype.then is reached for every async underlying-source/sink/transformer callback result. readablestreamtoarraybuffer.test.ts pins exactly that. For native-promise results, replicate PromiseResolveThenableJob's timing manually: $enqueueJob a job that chains through the intrinsic .$then. Same two-hop ordering (WPT cancel.any.js stays 11/11), Promise.prototype.then is not touched. writableStreamDefaultControllerStart now routes through the same helper instead of new Promise(r => r(startResult)), which had the same Get(x, 'then') exposure. --- src/js/builtins/StreamInternals.ts | 21 ++++++++++++++++++++- src/js/builtins/WritableStreamInternals.ts | 5 +++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/js/builtins/StreamInternals.ts b/src/js/builtins/StreamInternals.ts index c411a3048c39..564c031b395e 100644 --- a/src/js/builtins/StreamInternals.ts +++ b/src/js/builtins/StreamInternals.ts @@ -36,9 +36,28 @@ export function markPromiseAsHandled(promise: Promise) { // ordering). Promise.$resolve would return x unchanged when x is already a // native Promise and skip that hop; the streams ref-impl explicitly avoids // Promise.resolve here for the same reason. +// +// $resolvePromise performs thenable assimilation via Get(x, "then") and +// invokes it when it differs from the builtin, so a monkey-patched +// Promise.prototype.then is reached. For native promises replicate the +// PromiseResolveThenableJob timing — queue a job that chains the reaction +// — through the intrinsic .$then: same two-hop microtask ordering, +// tamper-proof. export function shieldingPromiseResolve(result) { const promise = $newPromise(); - $resolvePromise(promise, result); + if ($isPromise(result)) { + $enqueueJob( + (p, r) => + r.$then( + v => $resolvePromise(p, v), + e => $rejectPromise(p, e), + ), + promise, + result, + ); + } else { + $resolvePromise(promise, result); + } return promise; } diff --git a/src/js/builtins/WritableStreamInternals.ts b/src/js/builtins/WritableStreamInternals.ts index 5f79c9ff5f0d..60de6f5f485b 100644 --- a/src/js/builtins/WritableStreamInternals.ts +++ b/src/js/builtins/WritableStreamInternals.ts @@ -591,8 +591,9 @@ export function writableStreamDefaultControllerStart(controller) { // startAlgorithm() returns one (as TransformStream's does), the assimilation // hop is observable in the spec's microtask ordering. Promise.$resolve would // return the input promise unchanged and skip that hop. - const startResult = startAlgorithm.$call(); - return new Promise(resolve => resolve(startResult)).$then( + // $shieldingPromiseResolve takes the hop via .$then so a monkey-patched + // Promise.prototype.then is not reached during assimilation. + return $shieldingPromiseResolve(startAlgorithm.$call()).$then( () => { const state = $getByIdDirectPrivate(stream, "state"); $assert(state === "writable" || state === "erroring"); From 6d74418507bd9b0bec1eb3e766f9cf1c67b74494 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 23 Jun 2026 11:18:33 +0000 Subject: [PATCH 3/8] streams: return raw start() result from underlying-sink startAlgorithm UnderlyingSinkStartCallback's IDL return type is `any`, so Web IDL "invoke" performs no promise conversion at that layer. The single "a promise resolved with startResult" wrap happens in SetUpWritableStreamDefaultController step 17 (via $shieldingPromiseResolve in writableStreamDefaultControllerStart). Going through $promiseInvokeOrNoopMethodNoCatch here applied an extra $shieldingPromiseResolve wrap, which with the new always-fresh-promise semantics delayed [[started]] by ~2 microtask ticks for new WritableStream({start}). Matches ReadableStreamDefaultControllerStart and the whatwg/streams reference implementation. Adds microtask-ordering tests that match Node exactly for both the promise-returning and sync-returning start() cases. --- src/js/builtins/WritableStreamInternals.ts | 8 ++- test/js/web/streams/streams.test.js | 58 ++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/js/builtins/WritableStreamInternals.ts b/src/js/builtins/WritableStreamInternals.ts index 60de6f5f485b..a94132ac449c 100644 --- a/src/js/builtins/WritableStreamInternals.ts +++ b/src/js/builtins/WritableStreamInternals.ts @@ -632,7 +632,13 @@ export function setUpWritableStreamDefaultControllerFromUnderlyingSink( if ("start" in underlyingSinkDict) { const startMethod = underlyingSinkDict["start"]; - startAlgorithm = () => $promiseInvokeOrNoopMethodNoCatch(underlyingSink, startMethod, [controller]); + // UnderlyingSinkStartCallback's IDL return type is `any`, so Web IDL + // "invoke" performs no promise conversion here. The single "a promise + // resolved with startResult" wrap happens in + // writableStreamDefaultControllerStart (SetUpWritableStreamDefaultController + // step 17). A synchronous throw propagates out of the WritableStream + // constructor, matching spec. + startAlgorithm = () => startMethod.$call(underlyingSink, controller); } if ("write" in underlyingSinkDict) { const writeMethod = underlyingSinkDict["write"]; diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index 537be8d970d5..cadca4fb48da 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -260,6 +260,64 @@ describe("WritableStream", () => { await rs.pipeTo(ws); expect(received).toBe("hello world"); }); + + // SetUpWritableStreamDefaultController step 17: "Let startPromise be a + // promise resolved with startResult." Web IDL "a promise resolved with x" is + // always a fresh promise; when x is a thenable the PromiseResolveThenableJob + // hop is observable in microtask ordering. Promise.resolve(x) would return x + // unchanged when x is already a native Promise and skip that hop. + describe('[[started]] timing (Web IDL "a promise resolved with")', () => { + async function observe(sink) { + const order = []; + const { promise: done, resolve } = Promise.withResolvers(); + const ws = new WritableStream({ + ...sink, + write() { + order.push("write"); + resolve(); + }, + }); + ws.getWriter().write("x"); + queueMicrotask(() => { + order.push("mt1"); + queueMicrotask(() => order.push("mt2")); + }); + await done; + return order; + } + + it("start() returns a fulfilled Promise: write after the assimilation hop", async () => { + expect(await observe({ start: () => Promise.resolve() })).toEqual(["mt1", "mt2", "write"]); + }); + + it("start() returns undefined: write before the first queued microtask (single wrap, no double-wrap regression)", async () => { + expect(await observe({ start() {} })).toEqual(["write", "mt1", "mt2"]); + }); + + it("no start(): write before the first queued microtask", async () => { + expect(await observe({})).toEqual(["write", "mt1", "mt2"]); + }); + + it("TransformStream writable startAlgorithm returns startPromise: transform after the assimilation hop", async () => { + const order = []; + const { promise: done, resolve } = Promise.withResolvers(); + const ts = new TransformStream({ + transform(chunk, controller) { + order.push("transform"); + controller.enqueue(chunk); + resolve(); + }, + }); + ts.readable.getReader().read(); + ts.writable.getWriter().write("x"); + queueMicrotask(() => { + order.push("mt1"); + queueMicrotask(() => order.push("mt2")); + }); + await done; + expect(order).toEqual(["mt1", "mt2", "transform"]); + }); + }); }); describe("ReadableStream.prototype.tee", () => { From 78d7e70dfbdc764a6819d8adacddfd6d4c41c17c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:07:52 +0000 Subject: [PATCH 4/8] streams: catch-and-reject in shieldingPromiseResolve's assimilation job r.$then is a prototype-chain lookup of @then, so a start()-returned native promise with a null prototype would throw TypeError inside the $enqueueJob callback and leave the wrapper pending forever. And even with @then found, intrinsic Promise.prototype.then runs SpeciesConstructor(this), so a throwing @@species on the result threw inside the job the same way. Call $Promise.prototype.$then.$call(r, ...) (independent of r's prototype) wrapped in try/catch that rejects the wrapper on abrupt completion, matching PromiseResolveThenableJob step 3. A throwing @@species now errors the stream instead of leaving [[started]] pending plus an uncaught exception; null-prototype results flip [[started]] and writes proceed. Tests cover both tamper vectors; the @@species case now matches Node. --- src/js/builtins/StreamInternals.ts | 29 +++++++++++++--------- test/js/web/streams/streams.test.js | 37 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/js/builtins/StreamInternals.ts b/src/js/builtins/StreamInternals.ts index 564c031b395e..bd27aee0d0b8 100644 --- a/src/js/builtins/StreamInternals.ts +++ b/src/js/builtins/StreamInternals.ts @@ -37,21 +37,28 @@ export function markPromiseAsHandled(promise: Promise) { // native Promise and skip that hop; the streams ref-impl explicitly avoids // Promise.resolve here for the same reason. // -// $resolvePromise performs thenable assimilation via Get(x, "then") and -// invokes it when it differs from the builtin, so a monkey-patched -// Promise.prototype.then is reached. For native promises replicate the -// PromiseResolveThenableJob timing — queue a job that chains the reaction -// — through the intrinsic .$then: same two-hop microtask ordering, -// tamper-proof. +// For native-promise results replicate PromiseResolveThenableJob's timing by +// queuing a job that chains through the intrinsic Promise.prototype.@then: +// same two-hop microtask ordering, and the job's try/catch + reject is the +// spec job's step 3 (abrupt completion of the then call rejects the wrapper, +// e.g. a throwing @@species on the result). $resolvePromise alone would do +// Get(result, "then") once Promise.prototype.then has been replaced, reaching +// the user override for every async callback result. export function shieldingPromiseResolve(result) { const promise = $newPromise(); if ($isPromise(result)) { $enqueueJob( - (p, r) => - r.$then( - v => $resolvePromise(p, v), - e => $rejectPromise(p, e), - ), + (p, r) => { + try { + $Promise.prototype.$then.$call( + r, + v => $resolvePromise(p, v), + e => $rejectPromise(p, e), + ); + } catch (e) { + $rejectPromise(p, e); + } + }, promise, result, ); diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index cadca4fb48da..4ade31a56c6f 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -317,6 +317,43 @@ describe("WritableStream", () => { await done; expect(order).toEqual(["mt1", "mt2", "transform"]); }); + + // PromiseResolveThenableJob step 3: an abrupt completion of the then call + // rejects the wrapper. Intrinsic Promise.prototype.then runs + // SpeciesConstructor(this), so a throwing @@species on the start()- + // returned promise must reject startPromise and error the stream, not + // leave startPromise pending forever. + it("start() returns a Promise whose @@species throws: the stream errors with the thrown value", async () => { + const speciesError = new Error("species-boom"); + const p = Promise.resolve(); + p.constructor = { get [Symbol.species]() { throw speciesError; } }; + + let closedResult = "pending"; + const writer = new WritableStream({ start: () => p }).getWriter(); + writer.closed.then( + () => { closedResult = "fulfilled"; }, + e => { closedResult = e; }, + ); + // [[started]] settles within a bounded number of microtask rounds; poll + // rather than awaiting writer.closed so a never-settling wrapper fails + // fast instead of hanging the test. + for (let i = 0; i < 20; i++) await Promise.resolve(); + expect(closedResult).toBe(speciesError); + }); + + // The assimilation job looks up the intrinsic @then on Promise.prototype + // directly, not via the result's prototype chain, so a native promise with + // a detached prototype still lets [[started]] flip and writes proceed. + it("start() returns a Promise with a null prototype: [[started]] flips and write proceeds", async () => { + const p = Promise.resolve(); + Object.setPrototypeOf(p, null); + + let written = false; + const writer = new WritableStream({ start: () => p, write() { written = true; } }).getWriter(); + writer.write("x").catch(() => {}); + for (let i = 0; i < 20; i++) await Promise.resolve(); + expect(written).toBe(true); + }); }); }); From 9467b395cad8e5bebb9b05f2067ae135cb0eaaa9 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:10:09 +0000 Subject: [PATCH 5/8] [autofix.ci] apply automated fixes --- test/js/web/streams/streams.test.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index 4ade31a56c6f..244d53c5454d 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -326,13 +326,21 @@ describe("WritableStream", () => { it("start() returns a Promise whose @@species throws: the stream errors with the thrown value", async () => { const speciesError = new Error("species-boom"); const p = Promise.resolve(); - p.constructor = { get [Symbol.species]() { throw speciesError; } }; + p.constructor = { + get [Symbol.species]() { + throw speciesError; + }, + }; let closedResult = "pending"; const writer = new WritableStream({ start: () => p }).getWriter(); writer.closed.then( - () => { closedResult = "fulfilled"; }, - e => { closedResult = e; }, + () => { + closedResult = "fulfilled"; + }, + e => { + closedResult = e; + }, ); // [[started]] settles within a bounded number of microtask rounds; poll // rather than awaiting writer.closed so a never-settling wrapper fails @@ -349,7 +357,12 @@ describe("WritableStream", () => { Object.setPrototypeOf(p, null); let written = false; - const writer = new WritableStream({ start: () => p, write() { written = true; } }).getWriter(); + const writer = new WritableStream({ + start: () => p, + write() { + written = true; + }, + }).getWriter(); writer.write("x").catch(() => {}); for (let i = 0; i < 20; i++) await Promise.resolve(); expect(written).toBe(true); From d5e1f713d0f84eedad058aeaf3672c16938aff4e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 23 Jun 2026 13:56:59 +0000 Subject: [PATCH 6/8] ci: retrigger From 8eb15672e467862b60b463ac4d4a5100fd3c50f4 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Wed, 24 Jun 2026 15:29:36 +0100 Subject: [PATCH 7/8] streams: spec-exact shieldingPromiseResolve; subprocess-isolate watchpoint tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shieldingPromiseResolve is now $newPromise() + $resolvePromise() — exactly Web IDL "a promise resolved with x" (NewPromiseCapability + Resolve), no $isPromise branch. The branch was a JS-level workaround for an upstream JSC bug (PromiseResolveThenableJobFastSlow bare-returns on a SpeciesConstructor throw, oven-sh/WebKit#256) plus a non-spec tamper-proof preservation; the JSC fix belongs in WebKit, and per spec the assimilation does observe a patched Promise.prototype.then (Node matches). readablestreamtoarraybuffer.test.ts: sync start(). The test pins that Bun.readableStreamToArray returns an InternalPromise; the async start() was incidental setup that only had counter==0 under the old Promise.$resolve short-circuit. streams.test.js: subprocess-isolate the .then-observability test (patching Promise.prototype.then permanently invalidates JSC's promiseThenWatchpointSet for the process) and the @@species test (so a pristine watchpoint routes to the FastSlow path under test). @@species test is .todo until oven-sh/WebKit#256 lands and WEBKIT_VERSION bumps. .then-observability now asserts exactly 2 (catches double-wrap regression). Null-proto comment corrected to spec semantics. WritableStreamInternals.ts: drop the stale ".$then so a monkey-patched .then is not reached" comment. Benched (release, macOS arm64, 100k no-op transform writes): sync 0.47→0.50 µs/write (+6%), async 0.46→0.50 (+9%). The previous $isPromise+$enqueueJob shape measured 0.56 async (+22%); the spec-exact intrinsic is both simpler and faster. Claude-Session: https://claude.ai/code/session_01Qn8dDrArUZxc4towq11qzu --- src/js/builtins/StreamInternals.ts | 37 +------- src/js/builtins/WritableStreamInternals.ts | 10 +- .../util/readablestreamtoarraybuffer.test.ts | 8 +- test/js/web/streams/streams.test.js | 94 ++++++++++++------- 4 files changed, 77 insertions(+), 72 deletions(-) diff --git a/src/js/builtins/StreamInternals.ts b/src/js/builtins/StreamInternals.ts index bd27aee0d0b8..2e2c102c26cd 100644 --- a/src/js/builtins/StreamInternals.ts +++ b/src/js/builtins/StreamInternals.ts @@ -31,40 +31,13 @@ export function markPromiseAsHandled(promise: Promise) { $pokePromiseAsHandled(promise); } -// Web IDL "a promise resolved with x" — always a fresh promise (the -// assimilation hop when x is a thenable is observable in the spec's microtask -// ordering). Promise.$resolve would return x unchanged when x is already a -// native Promise and skip that hop; the streams ref-impl explicitly avoids -// Promise.resolve here for the same reason. -// -// For native-promise results replicate PromiseResolveThenableJob's timing by -// queuing a job that chains through the intrinsic Promise.prototype.@then: -// same two-hop microtask ordering, and the job's try/catch + reject is the -// spec job's step 3 (abrupt completion of the then call rejects the wrapper, -// e.g. a throwing @@species on the result). $resolvePromise alone would do -// Get(result, "then") once Promise.prototype.then has been replaced, reaching -// the user override for every async callback result. +// Web IDL "a promise resolved with x": NewPromiseCapability + Resolve(x). +// Always a fresh promise — Promise.$resolve(x) would return x unchanged when +// x is already a native Promise and skip the assimilation hop, which is +// observable in WPT's microtask ordering. export function shieldingPromiseResolve(result) { const promise = $newPromise(); - if ($isPromise(result)) { - $enqueueJob( - (p, r) => { - try { - $Promise.prototype.$then.$call( - r, - v => $resolvePromise(p, v), - e => $rejectPromise(p, e), - ); - } catch (e) { - $rejectPromise(p, e); - } - }, - promise, - result, - ); - } else { - $resolvePromise(promise, result); - } + $resolvePromise(promise, result); return promise; } diff --git a/src/js/builtins/WritableStreamInternals.ts b/src/js/builtins/WritableStreamInternals.ts index a94132ac449c..8faaa527071c 100644 --- a/src/js/builtins/WritableStreamInternals.ts +++ b/src/js/builtins/WritableStreamInternals.ts @@ -587,12 +587,10 @@ export function writableStreamDefaultControllerStart(controller) { const startAlgorithm = $getByIdDirectPrivate(controller, "startAlgorithm"); $putByIdDirectPrivate(controller, "startAlgorithm", undefined); const stream = $getByIdDirectPrivate(controller, "stream"); - // Web IDL "a promise resolved with x" is always a fresh promise — when - // startAlgorithm() returns one (as TransformStream's does), the assimilation - // hop is observable in the spec's microtask ordering. Promise.$resolve would - // return the input promise unchanged and skip that hop. - // $shieldingPromiseResolve takes the hop via .$then so a monkey-patched - // Promise.prototype.then is not reached during assimilation. + // SetUpWritableStreamDefaultController step 17: "a promise resolved with + // startResult". When startAlgorithm() returns a promise (TransformStream's + // does), Promise.$resolve would return it unchanged and skip the spec's + // assimilation hop. return $shieldingPromiseResolve(startAlgorithm.$call()).$then( () => { const state = $getByIdDirectPrivate(stream, "state"); diff --git a/test/js/bun/util/readablestreamtoarraybuffer.test.ts b/test/js/bun/util/readablestreamtoarraybuffer.test.ts index 7b15222bc3c3..f0f8cb7f972f 100644 --- a/test/js/bun/util/readablestreamtoarraybuffer.test.ts +++ b/test/js/bun/util/readablestreamtoarraybuffer.test.ts @@ -1,7 +1,11 @@ import { expect, test } from "bun:test"; test("readableStreamToArrayBuffer works", async () => { - // the test calls InternalPromise.then. this test ensures that such function is not user-overridable. + // Bun.readableStreamToArray returns an InternalPromise, whose own .then is + // not Promise.prototype.then; this test pins that the helper's chaining is + // unaffected by a user-patched .then. Sync start() so the spec's start- + // result wrap (which per Web IDL does call public .then for thenables) is + // not in play. let _then = Promise.prototype.then; let counter = 0; // @ts-ignore @@ -12,7 +16,7 @@ test("readableStreamToArrayBuffer works", async () => { try { const result = await Bun.readableStreamToArrayBuffer( new ReadableStream({ - async start(controller) { + start(controller) { controller.enqueue(new TextEncoder().encode("bun is")); controller.enqueue(new TextEncoder().encode(" awesome!")); controller.close(); diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index 244d53c5454d..d56f710c1486 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -318,40 +318,39 @@ describe("WritableStream", () => { expect(order).toEqual(["mt1", "mt2", "transform"]); }); - // PromiseResolveThenableJob step 3: an abrupt completion of the then call - // rejects the wrapper. Intrinsic Promise.prototype.then runs - // SpeciesConstructor(this), so a throwing @@species on the start()- - // returned promise must reject startPromise and error the stream, not - // leave startPromise pending forever. - it("start() returns a Promise whose @@species throws: the stream errors with the thrown value", async () => { - const speciesError = new Error("species-boom"); - const p = Promise.resolve(); - p.constructor = { - get [Symbol.species]() { - throw speciesError; - }, - }; - - let closedResult = "pending"; - const writer = new WritableStream({ start: () => p }).getWriter(); - writer.closed.then( - () => { - closedResult = "fulfilled"; - }, - e => { - closedResult = e; - }, - ); - // [[started]] settles within a bounded number of microtask rounds; poll - // rather than awaiting writer.closed so a never-settling wrapper fails - // fast instead of hanging the test. - for (let i = 0; i < 20; i++) await Promise.resolve(); - expect(closedResult).toBe(speciesError); + // PromiseResolveThenableJob: an abrupt completion of the then call + // rejects the wrapper. Promise.prototype.then runs SpeciesConstructor, + // so a throwing @@species on the start()-returned promise must reject + // startPromise and error the stream. Subprocess-isolated so a pristine + // promiseThenWatchpointSet routes to promiseResolveThenableJobFastSlow + // (the JSC code under test). Upstream JSC bug: + // https://github.com/oven-sh/WebKit/pull/256 + it.todo("start() returns a Promise whose @@species throws: the stream errors with the thrown value", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const speciesError = new Error("species-boom"); + const p = Promise.resolve(); + p.constructor = { get [Symbol.species]() { throw speciesError; } }; + let result = "pending"; + new WritableStream({ start: () => p }).getWriter().closed.then( + () => result = "fulfilled", + e => result = e === speciesError ? "speciesError" : String(e), + ); + for (let i = 0; i < 20; i++) await Promise.resolve(); + console.log(result);`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "speciesError", exitCode: 0 }); }); - // The assimilation job looks up the intrinsic @then on Promise.prototype - // directly, not via the result's prototype chain, so a native promise with - // a detached prototype still lets [[started]] flip and writes proceed. + // Resolve(x) does Get(x, "then"); a null-proto Promise has no .then, so + // it is treated as a non-thenable and the wrap fulfills with the promise + // object itself — [[started]] flips immediately and writes proceed. it("start() returns a Promise with a null prototype: [[started]] flips and write proceeds", async () => { const p = Promise.resolve(); Object.setPrototypeOf(p, null); @@ -1425,3 +1424,34 @@ it("ReadableStream BYOB read pending at cancel() resolves with undefined", async expect(value).toBeUndefined(); await reader.closed; }); + +// Web IDL "a promise resolved with x" is NewPromiseCapability + Resolve(x); +// Resolve(x) on a thenable does Get(x, "then") and queues a job to call it. +// So when an underlying-source/sink callback returns a Promise, the wrap +// calls Promise.prototype.then once — observable, per spec, like Node. +// Subprocess-isolated: patching Promise.prototype.then permanently +// invalidates JSC's promiseThenWatchpointSet for the process, which would +// route every later test through the generic thenable path. +it("wrapping an async stream callback result observes Promise.prototype.then (Web IDL 'a promise resolved with')", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const _then = Promise.prototype.then; + let counter = 0; + Promise.prototype.then = function (...args) { counter++; return _then.apply(this, args); }; + new ReadableStream({ async start() {} }); + new WritableStream({ async start() {} }); + await Bun.sleep(0); + Promise.prototype.then = _then; + console.log(counter);`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Exactly one assimilation per async start() result. A larger count would + // indicate a double-wrap regression (the FromUnderlyingSink change exists + // to prevent that). + expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "2", exitCode: 0 }); +}); From 5c71bacf0473d66c62dc1f3e8239d87e6e2a383c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:44:12 +0000 Subject: [PATCH 8/8] test(streams): include stderr in subprocess assertions; drain microtasks instead of Bun.sleep(0) stderr in the asserted object surfaces the subprocess's diagnostic output in the failure diff without pinning it to empty (ASAN/debug builds emit benign warnings). The .then-observability fixture now drains microtasks via `for (let i = 0; i < 20; i++) await 1` (await on a non-thenable doesn't reach the patched .then) instead of Bun.sleep(0). The PromiseResolveThenableJob is a microtask, so no task-level yield is needed. Matches the flush pattern in the adjacent null-proto test. --- test/js/web/streams/streams.test.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/js/web/streams/streams.test.js b/test/js/web/streams/streams.test.js index d56f710c1486..9310cc44ee73 100644 --- a/test/js/web/streams/streams.test.js +++ b/test/js/web/streams/streams.test.js @@ -345,7 +345,11 @@ describe("WritableStream", () => { stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "speciesError", exitCode: 0 }); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: "speciesError", + stderr: expect.any(String), + exitCode: 0, + }); }); // Resolve(x) does Get(x, "then"); a null-proto Promise has no .then, so @@ -1442,7 +1446,10 @@ it("wrapping an async stream callback result observes Promise.prototype.then (We Promise.prototype.then = function (...args) { counter++; return _then.apply(this, args); }; new ReadableStream({ async start() {} }); new WritableStream({ async start() {} }); - await Bun.sleep(0); + // The PromiseResolveThenableJob runs as a microtask; drain enough + // rounds for both assimilations to complete. await on a non-thenable + // doesn't reach the patched .then. + for (let i = 0; i < 20; i++) await 1; Promise.prototype.then = _then; console.log(counter);`, ], @@ -1453,5 +1460,9 @@ it("wrapping an async stream callback result observes Promise.prototype.then (We // Exactly one assimilation per async start() result. A larger count would // indicate a double-wrap regression (the FromUnderlyingSink change exists // to prevent that). - expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "2", exitCode: 0 }); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: "2", + stderr: expect.any(String), + exitCode: 0, + }); });