streams: follow a promise returned by direct pull() or async-iterable next() through its own then() by robobun · Pull Request #46295 · oven-sh/bun

robobun · 2026-10-21T05:58:58Z

Problem

Since 1.4.0 (webstreams: rewrite ReadableStream, WritableStream, and TransformStream in C++ (zero JS builtins) #33193), a direct stream's pull() and an async-iterable body's next() never call then() on a returned promise. A returned Bun.$ or Bun.sql query never starts. A body that waits for it never ends.
A native reaction (JSDirectStreamController.cpp:731, BunAsyncIterableSource.cpp:346) reads the promise's state.
Found by a fuzzer. async plus await works.

Fix

Both now apply promiseResolvedWithFast, the rule of spec stream callbacks. A promise without the built-in then() (a subclass, an own or replaced then) gets a follower, a new promise resolved with it. A job calls then() once.
That helper now compares the then function when JSC's watchpoint on it has ended, as after Object.freeze(Promise.prototype). Such a realm costs no follower.
Only JSDirectStreamSource calls a direct source's functions. New, not restored: native-sink pull() (Bun.serve, Bun.write) and sink-side cancel().
Verified: direct-stream-returned-promise.test.ts (26 of 33 fail on main), body-async-iterator.test.ts (12 of 15). Self-reviewed: 32 concerns raised, 29 addressed.

Background

A direct stream (type: "direct") writes to its consumer. A whole-body consumer ends the body when pull()'s promise resolves.
Weighed: then() called in place (sql: consolidate the open Bun.sql, bun:sqlite and node:sqlite pull requests #45304). It fails when the state never carries the result (p-lazy).

Downsides

A then() that never calls back hangs a next() body. One that throws fails it. Node and 1.3.14 agree.
A pull() that writes from events and returns a lazy promise ends the body when it resolves. A reader calls it again.
Release: +19 to +32 instructions per returned promise (+190 to +210 if frozen), +10 per pull() without one, 0 cells, +1,664 B text.

Notes

Scope

The fuzz report names six places that wait on a returned promise by its state. The Bun.serve handlers are in #46075. This PR takes the two stream places. It is based on main and shares no code with the other PRs of the family (#45901, #45843, #41008). The two HTMLRewriter places are not here: see "Not changed".

Repro

// A pending Promise subclass that starts its work in then(): the shape of Bun.$ and of a Bun.sql query.
class Lazy extends Promise {
  #start;
  static get [Symbol.species]() { return Promise; }
  constructor(work) { let resolve; super(r => { resolve = r; }); this.#start = () => resolve(work()); }
  then(a, b) { const start = this.#start; this.#start = undefined; start?.(); return super.then(a, b); }
}
const stream = new ReadableStream({ type: "direct", pull: c => new Lazy(() => { c.write("ok"); c.close(); }) });
console.log(await new Response(stream).text());

Binaries: official 1.3.14, a release build of main, a release build of this branch. The count of then() calls is in brackets.

The promise comes from	1.3.14	1.4.3 and main	this PR
direct `pull()`, read by a reader, `Response.text()` or `Bun.readableStreamToBytes`	answers (1)	no answer (0)	answers (1)
direct `pull()`, read by a `Bun.serve` body, `Bun.spawn` stdin or `Bun.write`	no answer (0)	no answer (0)	answers (1)
`cancel()` after the client left a `Bun.serve` body	not started (0)	not started (0)	started (1)
`next()` of `new Response(asyncIterable)`	answers (1 for each result)	no answer (0)	answers (1 for each result)

Node 26.3.0 answers the last row with 1 call for each result. The first and the last row are the regression from #33193 (92459cd).

The rule, in `then()` calls

One process for each cell. The returned value is:

(a) a pending subclass with its own then()
(b) the same, fulfilled when it is made
(c) a plain promise with an own then property
(d) a plain promise after Promise.prototype.then is replaced
(e) a plain thenable

	a	b	c	d	e
`await`, Node and Bun	1	1	0	0	1
default stream `pull()`, Node and Bun main	1	1	1	1	1
`next()`, Node 26.3.0	1	1	1	1	1
`next()`, Bun 1.3.14	1	0	0	0	1
`next()`, main	0	0	0	0	1
`next()`, this PR	1	1	1	1	1
direct `pull()`, Bun 1.3.14	1	1	1	1	0
direct `pull()`, main	0	0	0	0	0
direct `pull()`, this PR	1	1	1	1	0

next() now counts as Node does. A direct pull() counts as 1.3.14 did.
This is not the rule of await. await does not call an own then of a plain promise (columns c and d).
Column e at a direct pull(): no release follows a plain thenable there. This PR does not change that.
An async-iterable body is a direct source of Bun's own. Its pull() returns a promise that Bun made, and reactionPromiseSlow exempts it. A replaced Promise.prototype.then sees only the promises that next() returned. A test covers it.

What changes for a value, from main to this PR

A promise without the built-in then(): 0 calls of then() become 1, from a job. A subclass that overrides nothing counts too.
then is not callable (promise.then = undefined): Bun waits on the state, as on main.
A then getter that throws, or a then() that throws: the body fails with that error. Main does not read then.
A then() that never calls its callbacks: next() gets no answer, as in Node and 1.3.14. A direct pull() whose work closes the controller still answers.
p-lazy (fulfilled when it is made, works in then()): main ends the body at once. The work never runs. This PR runs the work and waits for it.
When a follower takes the reactions, the returned promise is marked as handled in every state. It is never reported as an unhandled rejection, as on main. Node 26.3.0 and a default stream pull() on main report one when then() does not subscribe to the promise.
A rejection that arrives after the sink closed or detached is still dropped (Bun.serve direct streams: flush on controller.close(), end the response once, call cancel() only on a client abort #41894). A contract shape covers it with a promise that has its own then().
A sink-side cancel() (a client abort, a write error): then() is now called once, as reader.cancel() already does on main. Nothing waits for the result, as before.
A plain promise: the same cell as on main, no new cell, the same order of jobs. That holds in every realm where then is still the built-in function: see "A realm where JSC's watchpoint has ended".
then() runs in the async context of the pull() or next() call that returned the promise (checked with AsyncLocalStorage). streams: run every callback of a direct source in the stream's AsyncLocalStorage context #41561 wants to change that context for two of the callers. A test here records that this PR does not move it.

A realm where JSC's watchpoint has ended

JSC keeps a watchpoint on Promise.prototype.then. JSPromise::isThenFastAndNonObservable() reads it, and on main promiseResolvedWithFast asks only that function whether a promise is plain. These end the watchpoint for the life of the realm, although then is still the built-in function:

Object.freeze(Promise.prototype) or Object.seal(Promise.prototype)
a then that is made read-only
a then that is replaced and then put back

On main every promise then gets a follower at the spec callbacks, for example at a default stream's pull(). An earlier revision of this PR added the same cost at a direct pull() and at next(). The fuzz report measured that revision: 513 ms on main and 1,000 ms on the PR for 1,000,000 one-byte chunks in a frozen realm.

promiseResolvedWithFast now makes its own test when the watchpoint has ended (hasUnpatchedThen). The promise must have the plain promise structure of its realm, and the own then of that realm's Promise.prototype must be a data property that holds the built-in function. Such a promise takes internal reactions as it is. A replaced then, an accessor, an own then and a subclass still get a follower.

Cells for each read (Promise, Function, Object), from heapStats() slopes with the collector off. Release builds of main (449877f) and of this branch (0dfb3c8).

`Promise.prototype`	async generator body	direct stream, `async pull()`	default stream, `async pull()`
untouched, main and this PR	1, 0, 1	2, 1, 1	2, 1, 1
frozen or sealed, main	1, 0, 1	2, 1, 1	4, 5, 2
`then` read-only, or replaced and put back, main	1, 0, 1	2, 1, 1	4, 3, 1
each of those states, this PR	1, 0, 1	2, 1, 1	2, 1, 1

User-space instructions for each iteration in a frozen realm. The method is under "Measurements".

Fixture	main	main again	this PR	delta
async generator body, `arrayBuffer()`	3890.9	3890.1	4083.6	+193
async generator body, reader	3389.5	3389.1	3582.5	+193
iterator whose `next()` is already settled	2152.8	2154.9	2345.1	+191
direct, reader, `pull()` returns a pending promise	3393.2	3394.8	3605.0	+211
direct, reader, `async pull()` already settled	2653.7	2653.7	2858.5	+205
direct, reader, sync `pull()`	2064.1	2064.8	2078.3	+14
default stream, reader, `async pull()`	6353.9	6352.8	3425.8	-2,927

The +190 to +210 are one call out of line and the test of the then property, once for each returned promise. No cell is allocated, and read() is answered in the same microtask turn as on main.
Wall clock, 1,000,000 chunks of 16 bytes, 7 interleaved runs, median (worst). Frozen realm, async generator into arrayBuffer(): 317 (355) ms on main, 321 (337) ms on this PR. Frozen realm, default stream with an async pull(): 712 (863) ms on main, 387 (506) ms on this PR.
A default stream in such a realm changes too, because it uses the same helper. It now takes a plain promise as it does in an untouched realm, with no follower and fewer microtask turns.
The same test could live in JSPromise::isThenFastAndNonObservable(). That is a change in WebKit and is not part of this PR.

A `pull()` that writes from events

pull(c) {
  c.write("hello ");
  setTimeout(() => { c.write("world"); c.close(); }, 20);
  return sql`insert into t values ('ran')`; // not awaited
}

	Read by `Response.text()`, `readableStreamToBytes`, `Bun.serve` or `Bun.write`	Read by a reader	The query
main	`hello world`	`hello world`, 1 `pull()` call	never runs
this PR	`hello`	`pull()` runs again for each later read	runs
`async pull()` with `await sql`, main and this PR	`hello`	`pull()` runs again for each later read	runs

The documented rule decides both columns (packages/bun-types/bun.d.ts:581). The stream "ends when it resolves". A reader "calls pull() again for a later read, once the previous call has settled". On main the promise of the query never settles, because nothing starts it.
For Bun.serve and Bun.write the late c.write() throws This HTTPResponseSink has already been closed (or FileSink). The async spelling does the same on main.
A pull() that writes on every call and never closes by itself is an endless stream for a reader. With a SQLite Bun.sql query that loop did not return to the event loop in my probe: the 20 ms timer never fired. The async spelling does the same on main.

Real values

p-lazy 5.0.0, postgres 3.4.9, Postgres on 127.0.0.1. Each query row is a pull() that writes done, returns the value and does not close, read by Response.text().

Value	1.3.14	main	this PR
`p-lazy` from a `pull()` that writes `a`, waits, writes `b`, closes	`ab`	empty body, the work never runs	`ab`
`p-lazy` from `next()`	rejects	rejects (`Async iterator result is not an object`)	`ok`, as in Node
a `postgres` query from `pull()`	runs, the body never ends	never runs, the body never ends	runs, body `done`
a `Bun.sql` query (Postgres) from `pull()`	the same	the same	the same
a `Bun.$` command from `pull()`	the same	the same	the same

In TypeScript only the Bun.sql query compiles as the result of pull() (void | PromiseLike<void>). A Bun.$ command needs a cast.

Not changed

HTMLRewriter handlers (element(), comments(), text(), document end()). No release follows a promise there, and workerd 1.20261019.1 reads the state too (0 then() calls, no answer). To follow there is a new behaviour and a difference from workerd. A maintainer has to decide that first.
A plain thenable at a direct pull().
The result of a source's close(reason) hook. The hook is not in the docs or in DirectUnderlyingSource, and main discards its result at each of its three call sites. It is a few lines in JSDirectStreamSource::close() if it must start a returned promise too.
Default and byte streams, WritableStream and TransformStream. They already follow.
Other callbacks of Bun that read a returned promise by its state. The open PRs for them are Bun.serve: call then() on a returned promise that has its own then() #46075, Bun.build: await a pending Promise subclass from plugin setup() through its own then() #45901, Bun.cron: wait on a returned thenable or Promise subclass through its own then() #45843, bun:test: wait on a returned thenable or Promise subclass through its own then() #41008, bun:test: settle thenables through their own then() in .resolves/.rejects #40996 and bun:test: wait for a thenable that a withImplementation() callback returns #46110.

Precedents

sql: consolidate the open Bun.sql, bun:sqlite and node:sqlite pull requests #45304 (merged): expect(p).resolves calls then() once while p is pending (call_then_if_pending, src/runtime/test_runner/expect.rs:475). It then reads the state of p.
bun test: Vitest compatibility #44833 (open): promise_to_await follows a pending promise through promise resolution and reads a settled promise by its state.

Questions for a maintainer

The rule. This PR follows columns b, c and d of the table and leaves column e at a direct pull(). The precedents above read a settled promise (column b) by its state. With that rule here, p-lazy from pull() stays broken and next() differs from Node. One status test in promiseMayNeedFollower switches the rule.
The owner. JSDirectStreamSource becomes the only caller of a source's functions. The alternative is the same helper at the four call sites, with public accessors. That diff is smaller, and a fifth call site can skip the helper.
HTMLRewriter handlers. A patch exists (one helper in html_rewriter.rs, tests, docs). It needs a rebase on Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021. I can open it if the difference from workerd is wanted.
The test of the then function. It is in promiseResolvedWithFast, so default streams change in a realm with a frozen Promise.prototype too. There the spec's "a promise resolved with p" calls the built-in then() from a job, and Bun now attaches its reaction to p at once. The difference is one microtask turn and the read of constructor. The test can move to promiseForReactionsSlow if default streams must keep main's timing in such a realm.

Measurements

Release builds of main (449877f) and of this branch (0dfb3c8), linux-x64, one build directory.

size: text 107,557,538 to 107,559,202 B (+1,664). data and bss do not change. The stripped file is 107,730,656 B on both.
Symbols: new cancelFromSink 664 B, hasUnpatchedThen 337 B, reactionPromiseSlow 247 B, promiseForReactionsSlow 211 B. Grown: callDirectPull +334, consumeDirectStreamToArrayBuffer +110, driveAsyncIterator +94.
Host functions, exported symbols, startup work: +0.

User-space instructions for each iteration in an untouched realm (perf_event_open, slope from N to 2N with N = 100,000, median of 5 runs, concurrent JIT and GC off). Main measured twice gives the noise floor. Every fixture returns plain promises only.

Fixture	main	main again	this PR	delta
async generator body, `arrayBuffer()`	3422.0	3422.0	3443.3	+21
async generator body, reader	2922.5	2921.1	2941.3	+19
iterator whose `next()` is already settled	2154.1	2152.5	2174.5	+21
direct, reader, `pull()` returns a pending promise	3215.9	3219.3	3246.8	+28 to +31
direct, reader, `async pull()` already settled	2473.9	2472.5	2504.1	+30 to +32
direct, reader, sync `pull()`	1888.1	1890.3	1899.5	+9 to +11
default stream, reader, `async pull()`	3008.5	3013.8	3014.9	+1 to +6

Calls, from gdb breakpoint counts at N = 2,000 and N = 6,000 in an untouched realm. At the async generator body and at the direct stream: promiseForReactionsSlow 0, promiseResolvedWith 0 and hasUnpatchedThen at most 1 for the whole run. At the default stream: one call of the plain-promise test for each pull(), on main and on this branch.
The same counts in a frozen realm: hasUnpatchedThen once for each returned promise and promiseResolvedWith 0 on this branch. On main the default stream calls promiseResolvedWith once for each pull().
Cells, from heapStats() slopes with the collector off: plain promises give the same counts on both binaries. Each followed value costs +2 Promise, +4 Function, +1 Object, +1 FullPromiseReaction.
With Promise.prototype.then replaced by another function, each pending result costs 1 call of the replacement, +2 Promise and +2 Function. A default stream pull() on main costs the same.

Tests

test/js/web/streams/direct-stream-returned-promise.test.ts (new, 33 tests). 26 fail on main. The other 7 guard code that the diff adds or moves:
- a then that is not callable (3 tests)
- the errors for a missing pull()
- the async context of pull()
- what a sink-side cancel() reports
- no report of an unhandled rejection for a promise that its then() does not subscribe to
test/js/web/fetch/body-async-iterator.test.ts (+15 tests). 12 fail on main.
test/js/web/streams/direct-stream-contract.ts (+6 shapes, run by streams.test.js and serve-direct-readable-stream.test.ts): 92 new cells, 59 fail on main. The 441 old cells give the same result on both builds.
Each failure on main is an assertion or the fixture's own rejection (then() was not called ...). None is a timeout.
Also run on the debug build of this branch: streams.test.js (801 pass), serve-direct-readable-stream.test.ts (193 pass), async-iterator-stream.test.ts (139 pass), wpt-streams.test.ts (1175 pass), readablestreamtoarraybuffer.test.ts (2 pass), and the other files of test/js/web/streams/. One test of streams-leak.test.ts times out on the debug build, on main too. The two test files above also pass with BUN_JSC_validateExceptionChecks=1.
Three tests count Promise cells in a child with BUN_JSC_useGC=0, one for each realm: untouched, frozen, and then replaced and put back. A count is then a number of allocations. On main they fail: the lazy promise in each child is not followed, and the default stream takes 4.0 promises for each read in the last two realms. Against a build that makes a follower for every promise, the untouched realm gives 3.00 and 2.00 promises for each read. This branch gives 2.00 and 1.00.
The new test file is separate on purpose. It holds only the rows about the returned value, and alone it runs in about 8 seconds on a debug build. The lifecycle rows are in the shared contract module. The spawned tests run with test.concurrent.
The counts in this section are from debug builds of 449877f and of the branch at 0dfb3c8.

Self-review: the 3 concerns not taken

One fixture file shared with the other PRs of the family. They are separate PRs on main, and a shared file would stack them.
A typed bun_jsc face for the helper. No caller outside the streams module remains, because the HTMLRewriter change is not in this PR.
A committed test against Postgres. The test file has a Bun.sql row on SQLite, which needs no service. I ran the Postgres rows by hand (table above).

[human-review] gate passed · iteration 0 · 11 files touched

fails on main (without fix)

ASAN without fix: 38 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/web/fetch/body-async-iterator.test.ts test/js/web/streams/direct-stream-returned-promise.test.ts
bun test v1.4.4 (367d939d9)

test/js/web/streams/direct-stream-returned-promise.test.ts:
11 | function rejectUnlessAwaited(promise: Promise<unknown> & { thenCalls: number }, reject: (reason: unknown) => void) {
12 |   setImmediate(() => {
13 |     if (promise.thenCalls) return;
14 |     // Only a wait on the state sees this rejection, so it is not an unhandled one.
15 |     Promise.prototype.then.call(promise, undefined, () => {});
16 |     reject(new Error(NOT_AWAITED));
                    ^
error: then() was not called on a promise that starts its work in then()
      at <anonymous> (/workspace/bun/test/js/web/streams/lazy-promise.ts:16:16)
(fail) what pull() returns > a pending promise that starts in then(), like Bun.$ or a Bun.sql query > a reader calls then() once and waits for the work [61.90ms]
11 | function rejectUnlessAwaited(promise: Promise<unknown> & { thenCalls: number }, reject: (reason: unknown) => void) {
12 |   setI
... (truncated)

release without fix: 78 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/js/web/streams/direct-stream-returned-promise.test.ts:
11 | function rejectUnlessAwaited(promise: Promise<unknown> & { thenCalls: number }, reject: (reason: unknown) => void) {
12 |   setImmediate(() => {
13 |     if (promise.thenCalls) return;
14 |     // Only a wait on the state sees this rejection, so it is not an unhandled one.
15 |     Promise.prototype.then.call(promise, undefined, () => {});
16 |     reject(new Error(NOT_AWAITED));
                    ^
error: then() was not called on a promise that starts its work in then()
      at <anonymous> (/workspace/bun/test/js/web/streams/lazy-promise.ts:16:16)
(fail) what pull() returns > a pending promise that starts in then(), like Bun.$ or a Bun.sql query > a reader calls then() once and waits for the work [3.20ms]
11 | function rejectUnlessAwaited(promise: Promise<unknown> & { thenCalls: number }, reject: (reason: unknown) => void) {
12 |   setImmediate(() => {
13 |     if (promise.thenCalls) return;
14 |     // Only a wait on the state sees this rejection, so it is not an unhandled one.
15 |     Promise.prototype.then.call(promise, undefined, () => {});
16 |     reject
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/web/fetch/body-async-iterator.test.ts test/js/web/streams/direct-stream-returned-promise.test.ts
bun test v1.4.4 (367d939d9)

test/js/web/streams/direct-stream-returned-promise.test.ts:
(pass) what pull() returns > a pending promise that starts in then(), like Bun.$ or a Bun.sql query > a reader calls then() once and waits for the work [90.79ms]
(pass) what pull() returns > a pending promise that starts in then(), like Bun.$ or a Bun.sql query > a native sink (Bun.write) calls then() once and waits for the work [40.74ms]
(pass) what pull() returns > a pending promise that starts in then(), like Bun.$ or a Bun.sql query > a buffer consumer (Bun.readableStreamToBytes) calls then() once and waits for the work [6.99ms]
(pass) what pull() returns > a pending promise whose own state never carries the result > a reader calls then() once and waits for the work [14.48ms]
(pass) what pull() returns > a pending promise whose own state never carries the result > a native sink (Bun.write) calls then() once and waits for the work [7.94ms]
(pa
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     0dfb3c8a83
  features     lto, baseline

23 deps, 394 codegen, 1302 objects in 1162ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
282 units: 194 lib, 16 proc-macro (host), 27 custom-build (host), 23 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[3/4] reconfigure
[1/1671] mkdir stamps
[2/1671] mkdir codegen
[3/1671] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/1671] install /workspace/bun
bun install v1.4.3-canary.1 (367d939d9)

Checked 22 installs across 44 packages (no changes) [521.00ms]
[5/1671] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (367d939d9)

Checked 1 install across 2 packages (no changes) [17.00ms]
[6/1671] install /workspace/bun/src/node-fallbacks
bun install v1.4.3-canary.1 (367d939d9)

Checked 111 installs acro
... (truncated)

diff hotspot

.../webcore/streams/BunAsyncIterableSource.cpp     |  12 +-
 .../webcore/streams/BunStreamConsumers.cpp         |  15 +-
 .../bindings/webcore/streams/BunStreamSource.cpp   |  20 +-
 .../webcore/streams/JSDirectStreamController.cpp   |  42 +-
 .../webcore/streams/JSDirectStreamSource.h         |  40 +-
 .../bindings/webcore/streams/WebStreamsInternals.h |  15 +
 .../bindings/webcore/streams/WebStreamsMisc.cpp    |  33 +-
 test/js/web/fetch/body-async-iterator.test.ts      | 180 +++++++
 test/js/web/streams/direct-stream-contract.ts      |  68 +++
 .../streams/direct-stream-returned-promise.test.ts | 551 +++++++++++++++++++++
 test/js/web/streams/lazy-promise.ts                |  94 ++++
 11 files changed, 1026 insertions(+), 44 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                                      reads  edits  tests
…jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp      4      1     63
src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp       2      1     62
src/jsc/bindings/webcore/streams/BunStreamSource.cpp          2      1     62
…c/bindings/webcore/streams/JSDirectStreamController.cpp      3      4     63
src/jsc/bindings/webcore/streams/JSDirectStreamSource.h       2      3     63
src/jsc/bindings/webcore/streams/WebStreamsInternals.h        3      2     64
src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp           3      3     62
test/js/web/fetch/body-async-iterator.test.ts                 2      0     44
test/js/web/streams/direct-stream-contract.ts                 1      2     67
…t/js/web/streams/direct-stream-returned-promise.test.ts      4      3     41
test/js/web/streams/lazy-promise.ts                           1      3     62

robobun · 2026-10-21T05:59:11Z

Status: ready for review. The diff is green.

The two changed test files fail on main and pass on this branch, on a debug build and on a release build (see the evidence block in the description).
CI on 0dfb3c8 (build 133093): 179 of 181 jobs passed. The other 2 are the darwin x64 - test-bun jobs. They wait for an agent, and the same 2 jobs expired on the first build of this PR.
Each test that failed in that build passed on its retry. None is in a file that this PR touches.
All review threads are answered and resolved. The description has four questions for a maintainer.

How it was reproduced

// A pending Promise subclass that starts its work in then(): the shape of Bun.$ and of a Bun.sql query.
class Lazy extends Promise {
  #start;
  static get [Symbol.species]() { return Promise; }
  constructor(work) { let resolve; super(r => { resolve = r; }); this.#start = () => resolve(work()); }
  then(a, b) { const start = this.#start; this.#start = undefined; start?.(); return super.then(a, b); }
}
const stream = new ReadableStream({ type: "direct", pull: c => new Lazy(() => { c.write("ok"); c.close(); }) });
console.log(await new Response(stream).text());

Official 1.3.14 prints ok and calls then() once.
1.4.3 and main never call then(). The body never ends.
The same value from next() of new Response(asyncIterable): 1.3.14 and Node 26.3.0 answer, main does not.
This branch answers at both places with 1 then() call for each returned promise.
After the rebase, with debug builds: on main (449877f) a Bun.$ command that pull() or next() returns never ends the body. On this branch (709232e) the body ends.

PR: #46295

robobun · 2026-10-21T05:59:14Z

^{Updated 7:59 AM PT - Oct 26th, 2026}

✅ @robobun, your commit 0dfb3c8a834996d1a75a8577de222efc130b1cff passed in Build #133093! 🎉

🧪 To try this PR locally:

bunx bun-pr 46295

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

bun-46295 --bun

coderabbitai · 2026-10-21T06:06:40Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: oven-sh/bun
Branch: robobun/fa54ff54/pending-subclass-callback-results
File: .coderabbit.yaml
Commit: b4f9da1
Layers: Repository YAML
Review profile: ASSERTIVE
Plan: Advanced
Run ID: d5a5310a-4bab-4264-be60-6a8208bc8b2a

Commits

Reviewing files that changed from the base of the PR and between 709232e and b4f9da1.

Files selected for processing: 2

test/js/web/fetch/body-async-iterator.test.ts
test/js/web/streams/direct-stream-returned-promise.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Async iterable and direct stream paths now normalize promise results for reaction handling. Direct stream pull and cancellation use JSDirectStreamSource methods. Tests cover custom then() behavior, promise settlement, consumers, and cancellation.

Changes

Stream promise handling

Layer / File(s)	Summary
Async iterable promise normalization `src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp`, `src/jsc/bindings/webcore/streams/WebStreamsInternals.h`, `src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp`, `test/js/web/fetch/body-async-iterator.test.ts`	Async iterable results use promise reaction helpers. Tests cover custom `then()` behavior, non-callable `then` properties, and cancellation during `then` lookup.
Direct stream source callbacks `src/jsc/bindings/webcore/streams/JSDirectStreamSource.h`, `src/jsc/bindings/webcore/streams/JSDirectStreamController.cpp`, `src/jsc/bindings/webcore/streams/BunStreamSource.cpp`	`JSDirectStreamSource` now provides pull and cancellation methods. Direct stream callers use these methods and retain exception and promise reaction handling.
Direct stream consumers and tests `src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`, `test/js/web/streams/direct-stream-contract.ts`, `test/js/web/streams/direct-stream-returned-promise.test.ts`, `test/js/web/streams/lazy-promise.ts`	Consumers treat only promise pull results as asynchronous. Tests cover pull results, completion and rejection behavior, consumers, and cancellation cleanup.

Suggested reviewers: dylan-conway

Priority: Normal

Merge Risk: Minimal · up to b4f9d

No actionable merge-blocking risk was identified in the reviewed changes.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: direct-stream pull() and async-iterable next() results are now followed through their own then() method. It matches the changeset, which applies promiseResolvedWithFas…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description clearly explains the problem, implementation, scope, tradeoffs, and verification results. It does not use the template headings exactly, but it provides the required information in equ…

Full details: Title check

Explanation

The title names the main change: direct-stream pull() and async-iterable next() results are now followed through their own then() method. It matches the changeset, which applies promiseResolvedWithFast to both callbacks and adds the follower helper. It is specific and reads clearly in history. The title is long (99 characters), which is the only weakness.

Full details: Description check

Explanation

The description clearly explains the problem, implementation, scope, tradeoffs, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections.

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked that promiseForReactionsSlow in WebStreamsMisc.cpp cannot dereference a null follower (promiseResolvedWithFast returns nullptr only for non-objects, and the input is always a JSPromise cell), and that the three refactored pull() call sites keep the same this-value and the same TypeError text for a missing pull as before.

Extended reasoning...

The change touches the C++ web-streams layer (direct source pull/cancel, async-iterable next) to route a returned promise with a non-built-in then() through a follower promise, plus new tests; it touches no auth, crypto or injection surface. Inline findings were reported, so the note above only records what else was examined and ruled out.

…ull(), sink-side cancel() and async-iterable next() A direct stream's pull(), the cancel() that runs when a sink closes under the source, and the next() of an async-iterable body hung an internal reaction on the promise cell that the callback returned. A promise whose then() is not the built-in one (a subclass, an own then, any promise once Promise.prototype.then is replaced) never got a then() call. Bun.$ and a Bun.sql query start their work in then(), so the work never started, and a body that waited for it never ended. The reader and buffer-consumer paths of pull() and next() called then() in 1.3.14. These places now apply the rule that the streams module has for its spec callbacks (promiseResolvedWithFast): such a promise is waited for through a follower that calls then() once from a job. A plain promise of the realm is used as it is. A promise whose then is not callable is read by its state. JSDirectStreamSource is now the only caller of a source's functions. pull() and cancelFromSink() hand back a promise that an internal reaction can hang on, and the raw accessors are private.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (4):

Unresolved: 4 minor or pre-existing.

The two tests that spawn a child read only its stdout, so an error that the child printed did not reach the failure message. They now pipe stderr and assert that it is empty. The raw-socket fixture also rejects when the response ends before its first chunk, as the in-process helper does.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (4):

Unresolved: 4 minor or pre-existing.

- promiseForReactionsSlow declares a throw scope and returns before it reads the follower when an exception is pending. - Each added comment is one line. - The spawned tests pipe stderr and assert it with stdout and the exit code. The subprocess fixture rejects when the socket closes before the first chunk. - The Promise-count test runs its child with the collector off, so a count is a number of allocations. It fails when a collection still ran.

… reactions

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

…t-in function promiseResolvedWithFast asked JSC's isThenFastAndNonObservable() whether a promise is plain. That function reads a watchpoint on Promise.prototype.then, and the watchpoint also ends when the prototype is frozen or sealed, when `then` is made read-only, and when `then` is replaced and put back. In those realms every promise got a follower, although then() was still the built-in function. The helper now compares the function itself when the watchpoint has ended, so a direct pull(), an async-iterable next() and the spec stream callbacks take a plain promise as it is there too. The spawned tests run with test.concurrent.

claude

I re-reviewed the latest push (0dfb3c8) and found no bugs; because this changes when native stream reactions call a user-visible then() and adds a hand-rolled fallback to JSC's watchpoint, a maintainer's look at the semantics is still worthwhile.

What was reviewed on this push:

hasUnpatchedThen in src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp: the fallback is non-throwing (getDirectOffset/getDirect only), checks the promise structure before the prototype slot, rejects accessor/custom attributes, and compares against promiseProtoThenFunction(); cross-realm promises take the slow path and come back as themselves. One design point for a human: when the watchpoint is gone but then is intact, Bun now attaches reactions directly where JSC's own await would still call then() — unobservable in practice, but it is a deliberate divergence from the engine's rule.
promiseForReactionsSlow now runs under a throw scope and marks the original cell handled unconditionally when a follower takes the reactions; the three pull()/next() callers and cancelFromSink all check for an exception before using the result.
pullFunction()/cancelFunction()/thisValue() going private: no remaining external callers in src/jsc/bindings/webcore/streams; the callDirectPull null-source path keeps the same "underlyingSource.pull is not a function" message.
driveAsyncIterator: a then getter that throws or cancels mid-next() is handled by the exception check after promiseForReactions and the existing m_cancelled checks in asyncIterHandleNextResult and the fulfilled handler.

Extended reasoning...

The PR touches seven files in the WebStreams C++ bindings (direct stream pull/cancel, the async-iterable body source, and the shared promise helpers) plus about 900 lines of new tests, changing how a promise returned by user callbacks is followed through its own then(). It touches no auth, crypto, or injection surface, but it does decide when native code invokes a user-overridable then() and adds a fallback that compares the Promise.prototype.then slot directly when JSC's watchpoint has fired. The bug hunt ran dry with no findings, the prior inline threads on throw scope and markAsHandled are addressed in code, and the latest commit's fallback and test.concurrent changes read correctly. The change is not approvable automatically because the microtask-ordering and then()-call semantics are a design decision across several consumers (reader, Response.text, Bun.serve, Bun.write) that a maintainer should weigh, and the Promise-count heapStats tests depend on allocation thresholds.

robobun assigned Jarred-Sumner Oct 21, 2026

github-actions Bot added the claude label Oct 21, 2026

claude Bot reviewed Oct 21, 2026

View reviewed changes

robobun mentioned this pull request Oct 24, 2026

Response(asyncIterable): do not wait for what return() gives back #46762

Open

robobun force-pushed the robobun/fa54ff54/pending-subclass-callback-results branch from af14fef to 709232e Compare October 26, 2026 11:30