Support async generator functions in `Response` and `Request` for bodies by Jarred-Sumner · Pull Request #8941 · oven-sh/bun

Jarred-Sumner · 2024-02-16T10:10:46Z

What does this PR do?

This makes the following code work:

await new Response(
  async function* stream() {
    yield "hey";
    await Bun.sleep(42);
    yield Buffer.alloc(100);
  }
).arrayBuffer();

await new Response({
  async *[Symbol.asyncIterator]() {
    yield "hey";
    await Bun.sleep(42);
    yield Buffer.alloc(100);
  },
}).arrayBuffer();

How did you verify your code works?

There are tests. The ones for .json() fail. Need to fix that before we can merge

paperclover

did not read the test. looks good. i'm gonna give this a go on astro before marking this as approved

github-actions · 2024-02-16T10:36:19Z

❌ @Jarred-Sumner 3 files with test failures on bun-darwin-aarch64:

test/js/bun/shell/bunshell.test.ts 1 failing
test/js/bun/sqlite/sqlite.test.js 1 failing
test/js/node/process/process.test.js 1 failing

View test output

^{#b98058cc300f3814053479be071ef92c92a90f70}

github-actions · 2024-02-16T10:42:59Z

❌ @Jarred-Sumner 3 files with test failures on linux-x64:

test/js/bun/shell/bunshell.test.ts 1 failing
test/js/bun/shell/leak.test.ts 2 failing
test/js/node/process/process.test.js 1 failing

View test output

^{#b98058cc300f3814053479be071ef92c92a90f70}

github-actions · 2024-02-16T10:42:59Z

❌ @Jarred-Sumner 4 files with test failures on linux-x64-baseline:

test/js/bun/shell/bunshell.test.ts 1 failing
test/js/bun/shell/leak.test.ts 2 failing
test/js/node/process/process.test.js 1 failing
test/js/web/fetch/fetch-gzip.test.ts 1 failing

View test output

^{#b98058cc300f3814053479be071ef92c92a90f70}

github-actions · 2024-02-16T11:38:34Z

❌ @Jarred-Sumner 4 files with test failures on bun-darwin-x64:

test/js/bun/shell/bunshell.test.ts 1 failing
test/js/bun/sqlite/sqlite.test.js 1 failing
test/js/node/process/process.test.js 1 failing
test/js/third_party/postgres/postgres.test.ts 1 failing

View test output

^{#b98058cc300f3814053479be071ef92c92a90f70}

github-actions · 2024-02-16T11:41:53Z

❌🪟 @Jarred-Sumner, there are 14 test regressions on Windows x86_64

test\bundler\bundler_edgecase.test.ts
test\cli\hot\hot.test.ts
test\cli\run\env.test.ts
test\cli\run\transpiler-cache.test.ts
test\cli\run\require-cache.test.ts
test\js\bun\shell\shelloutput.test.ts
test\js\bun\shell\throw.test.ts
test\js\deno\fetch\response.test.ts
test\js\bun\http\bun-server.test.ts
test\js\third_party\es-module-lexer\es-module-lexer.test.ts
test\js\node\watch\fs.watchFile.test.ts
test\js\web\fetch\body.test.ts
test\js\web\fetch\fetch.test.ts
test\js\web\workers\worker.test.ts

Full Test Output

Electroid · 2024-02-16T17:06:14Z

+      async fetch(req) {
+        return new Response(
+          async function* () {
+            throw new Error("Oops");


Can we either move this test to a subprocess or add a special property that hides the errors from being printed? This makes our GitHub CI spammy with errors

That should be fixed in the GitHub actions printer IMO

Related to #8949

paperclover

nice

…sult (#40216) ### Problem - `new Response(asyncIterable)` and `new Request(url, { body: asyncIterable })` append the value of the final `{ done: true, value }` result to the body. An async generator's return value becomes part of the HTTP response: `async function* g() { yield "chunk;"; return "RETURN-VALUE"; }` gives `"chunk;RETURN-VALUE"`. Node gives `"chunk;"`. So do `for await`, `ReadableStream.from`, `Readable.from` and `Array.fromAsync` in Bun. - The cause is `asyncIterHandleNextResult` in `src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp`. It wrote the value of every result and only then checked `done`. The JS version from #8941 did the same, and the C++ port (#33193, #40047) kept it. ### Fix - The source reads the result with `IteratorDoneValue::Skip` (the mode `ReadableStream.from` uses) and finishes the stream on a done result without a write. A done result's `value` getter is no longer read. - `m_iteratorDone` only carried the done state across a backpressure suspension on that final write. A done result no longer writes, so the flag and its two checks go away. - This is a behavior change. Four tests from #8941 asserted the old behavior and are updated. `docs/runtime/streams.mdx` now states that the return value is not part of the body. - Verified: `test/js/bun/http/async-iterator-stream.test.ts` (new block "the value of a done result is not part of the body", plus the four updated cases) and `test/js/web/streams/streams.test.js` ("iterator result consumers"). Both fail on 1.4.0 and pass with this change. Also ran `bun-server.test.ts`, `body.test.ts`, `body-stream.test.ts`, `body-async-iterator.test.ts`, `node-http-nested-cork.test.ts` and `spawn-stdin-readable-stream-integration.test.ts`. ### Background - The async-iterable body is a Bun extension to `BodyInit`. `readableStreamFromAsyncIterator` wraps the iterator in a `type: "direct"` `ReadableStream`. Its `pull` drives `iterator.next(controller)` in a loop and writes each value to the controller (the HTTP sink when the response is served). - IteratorStepValue is the ECMAScript operation behind `for await`. It reads `done` first and reads `value` only when `done` is false. A generator's `return v` produces `{ done: true, value: v }`, so `v` is the result of the iteration, not an element. - `Bun::getIteratorResult` (`src/jsc/bindings/ObjectBindings.h`) reads both slots directly when the result has the realm's iterator result structure. For any other object it calls the getters. `IteratorDoneValue::Skip` skips the `value` getter when `done` is true. The caller still has to ignore the value on a done result, which the source now does. <details><summary>Notes</summary> Reproduction on bun 1.4.0, canary and main (3e347b3): ```js async function* g() { yield "chunk;"; return "RETURN-VALUE"; } await new Response(g()).text(); // "chunk;RETURN-VALUE", node 26: "chunk;" (await Array.fromAsync(ReadableStream.from(g()))).join(""); // "chunk;" on bun too ``` The same happens for a hand-written iterator whose last result is `{ done: true, value: "X" }` (body `"aX"`), for a sync generator under `Symbol.asyncIterator` (body `"s1SYNC-RETURN"`), and over the wire from a `Bun.serve` handler that returns `new Response(g())`. The change is in a single function. The rest of the diff removes the state that only the old behavior needed: - `m_iteratorDone` in `JSAsyncIteratorSourceOperation.h`. - The `m_iteratorDone` exit in `driveAsyncIterator` and the finish-after-drain check in `onAsyncIterableSourceFlushFulfilled`. A write now happens only for a result that is not done, so the drain of a write can never be the end of the iterator. Test changes: - `async-iterator-stream.test.ts`: the four cases that returned a value (`return "!"`, `return Buffer.from(...)`, `{ value: Buffer.from("world!"), done: true }`) now assert that the value is not in the body, and still assert the yielded chunks. New cases cover the generator (object, function and `Request` forms), a hand-written iterator, a sync generator under `Symbol.asyncIterator`, and a `Bun.serve` response. - `streams.test.js`: the accessor-order test for `Response` now expects `["done0", "value0", "done1", "value1", "done2"]` (no `value2`), which matches the `ReadableStream.from` test next to it. The `{value, done}`-with-a-done-accessor test expects `"m0"`. Four tests in `bun-server.test.ts` fail in this container with and without the change (IPv6 and `localhost` resolution). They are not related. </details>  --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/streams/streams.test.js

…ble body (#43758) ### Problem - An async-iterable body (`new Response(asyncGen())`, a `fetch` upload, a `Bun.serve` response) swallows each iterator error with `code: "ERR_INVALID_THIS"`, for example from `ReadableStream.prototype.tee.call({})`. `text()` resolves truncated, a server accepts the upload as complete, and a `read()` loop never settles. - The cause is the by-code check in `asyncIterFinishWithError` (`src/jsc/bindings/webcore/streams/BunAsyncIterableSource.cpp:180`). #43714 removed the same check for `ERR_INVALID_STATE`. ### Fix - Remove the check and `errorCodeIs()`, which has no caller left. The error now rejects the pull like every other error. - Correct because no sink sends this code to the pump now. A detached sink threw it on `write()` when #8941 added the check. Since #15234 it throws an `Error` with no code, or returns 0. - Verified: `test/js/bun/http/async-iterator-stream.test.ts`. The `ERR_INVALID_STATE` test of #43714 is now a `test.each` over both codes, with an upload whose server must read an aborted body. Main fails the `ERR_INVALID_THIS` case. Also `body-async-iterator`, `serve-direct-readable-stream`, `streams`, `body-stream`. Self-reviewed: 3 concerns raised, 3 addressed. ### Background - `BunAsyncIterableSource.cpp` feeds a `ReadableStream` from the iterator. Its pump calls `iterator.next()` and writes each value to the consumer's controller. - The pull is the promise that the pump returns to the stream. A resolved pull ends the body, a rejected pull fails it. - A sink is a native consumer (an HTTP response, a file, an upload). A detached sink lost its destination. ### Downsides - A generator that ends its body early with an `ERR_INVALID_THIS` error now fails the body. The repo has no such use. - Such an error now resets a `Bun.serve` response. Before, the client got a complete 200. <details><summary>Notes</summary> Repro (runs under Node.js and Bun): ```js const ways = { "own error with the code": () => { throw Object.assign(new Error("mine"), { code: "ERR_INVALID_THIS" }); }, "ReadableStream.prototype.tee.call({})": () => { ReadableStream.prototype.tee.call({}); }, "control: plain TypeError": () => { throw new TypeError("plain"); }, }; const body = way => (async function* () { yield "first;"; ways[way](); yield "NEVER"; })(); const settle = p => Promise.race([p.then(v => "resolved " + JSON.stringify(v), e => "rejected " + (e.code ?? e.name)), new Promise(r => setTimeout(() => r("NEVER SETTLES"), 3000))]); for (const way of Object.keys(ways)) { console.log(way); console.log(" text() ", await settle(new Response(body(way)).text())); console.log(" reader loop ", await settle((async () => { const r = new Response(body(way)).body.getReader(); while (!(await r.read()).done); return "drained"; })())); } ``` | way | consumer | Bun 1.3.14 | Bun 1.4.3-canary (367d939) | this PR and Node.js v26.3.0 | | --- | --- | --- | --- | --- | | own error with the code, `tee.call({})` | `text()` | never settles | resolves `"first;"` | rejects | | own error with the code, `tee.call({})` | reader loop | never settles | never settles | rejects | | `locked` getter of `ReadableStream` or `WritableStream` on `{}` | both | rejects (the error had no code) | as the rows above | rejects | So the swallow is older than the C++ streams rewrite (#33193) for an error that already carried the code. It is new in 1.4.0 for the two `locked` getters, which got the code with the rewrite. Wider probes, all on an ASAN debug build with `Malloc=1`, no sanitizer report: - 8 things to throw (own error, frozen error, a plain object with the code, three wrong-receiver calls, and a control) x 7 places (after the first `yield`, before it, after the last one, in a `finally` at the normal end, a custom iterator whose `next()` rejects or throws, an async generator function as the body) x 8 consumers (`text()`, `arrayBuffer()`, reader loop, `for await`, `pipeTo()`, `Bun.write()`, `fetch` upload, `Bun.serve` response). The 1.4.3 canary swallows 336 of 336 cells with the code. This PR: each cell has the outcome of the plain `TypeError` control, none hangs, and there is no unhandled rejection. - 11 consumers (the above plus `request.text()`, `Readable.fromWeb()`, a node:http response fed through `pipeline`) after a 6 byte and an 8 MiB prefix: the 1.4.3 canary swallows 22 of 22 cells, this PR 0. - Cases where the consumer goes away: `reader.cancel()` with and without a reason, a `for await` that breaks, a client that aborts a `Bun.serve` stream, a raw socket destroyed in the middle of the response, an upload whose server closes early, an upload aborted by its signal. In each case the generator ends quietly, throws a plain `Error` from its `finally`, or throws an error with the code from its `finally`. The 21 outcomes are the same before and after this change, with no unhandled rejection. These cases do not reach the error tail: the `cancel()` or `close()` hook of the source sets `m_cancelled` first. Producers of the code. Under `src/jsc/bindings/webcore/streams` only the brand checks of the spec classes throw it (`ReadableStream`, `WritableStream`, `TransformStream`, their readers, writers and controllers, the queuing strategies, the compression and text streams). The pump calls none of them. `JSDirectStreamController.cpp`, `BunStreamSource.cpp`, `BunStreamConsumers.cpp` and the generated `JSSink.cpp` do not throw it. On the sink side only `JSSink::get_this` (`src/runtime/webcore/Sink.rs:473`) throws it, for `CAST_FAILED`. `${name}__fromJS` (`src/codegen/generate-jssink.ts`) returns that value only when `this` is neither the sink nor its controller, and the pump calls `write`, `flush` and `end` with the controller as `this`. The five methods of the JS-facing `JSDirectStreamController` are bound functions and are no-ops once the source ended. The test: the generator of the upload throws once its request is at the server, so the server side is deterministic. With the `src/` of main (4ada08b) the `ERR_INVALID_THIS` case fails: `text()` and the upload resolve, the server reads `complete, 6 bytes`, and `read()` never settles. The `ERR_INVALID_STATE` case passes there. With this change both pass, 20 of 20 runs on a Linux ASAN build and 20 of 20 on a Windows x64 debug build. An upload that never settles ends in the timeout of the test runner. The test has no timer of its own. Self-review, three concerns, all addressed. (1) Can a sink or controller still send the code to the pump when the consumer is gone? No, see the list of producers above. (2) The first version of the test repeated the #43714 test. The two are now one `test.each`. (3) The upload raced the throw against the connect. The generator now throws once its request is at the server, and the test asserts what the server read. The review bot raised two optional points on the test. The server-side assertion covers the first. For the second (an upload that never settles has no diagnostics) I changed the comment and did not add a timer, because the test rules of this repo do not allow one. Suites run with the debug build: `test/js/bun/http/async-iterator-stream.test.ts` (97 pass), `test/js/web/fetch/body-async-iterator.test.ts`, `test/js/bun/http/serve-direct-readable-stream.test.ts` (169 pass), `test/js/bun/http/serve-error-handler-stream.test.ts`, `test/js/web/fetch/body-clone.test.ts`, `test/js/web/streams/streams.test.js` (614 pass), `test/js/web/fetch/body.test.ts`, `test/js/web/fetch/body-stream.test.ts` (9086 pass), `test/js/bun/http/bun-server.test.ts`. The two coded-error tests also pass with `BUN_JSC_validateExceptionChecks=1`. </details>

Jarred-Sumner added 3 commits February 16, 2024 02:00

💅

fb4ae64

Implement new Response(async function*) (async generators as bodies)

34bd8d2

Update streams.md

4155acc

Jarred-Sumner requested a review from paperclover February 16, 2024 10:10

paperclover reviewed Feb 16, 2024

View reviewed changes

paperclover and others added 3 commits February 16, 2024 02:46

we're winning now

c472b54

[autofix.ci] apply automated fixes

b501cc1

bug

23b3a41

Electroid reviewed Feb 16, 2024

View reviewed changes

Comment thread test/js/bun/http/async-iterator-stream.test.ts Outdated

Jarred-Sumner added 2 commits February 16, 2024 19:24

wip

659ed20

Fix test failures

11453f6

paperclover approved these changes Feb 17, 2024

View reviewed changes

paperclover merged commit abf1239 into main Feb 17, 2024

paperclover deleted the jarred/async-iteratable branch February 17, 2024 04:02

paperclover mentioned this pull request Feb 17, 2024

new Response looks not fully compatible with node on Astro@4.4.0 #8923

Closed

Jarred-Sumner mentioned this pull request Aug 13, 2026

Failed to parse JSON when running Trigger.dev Express server with Bun (tsx works) #8702

Closed

robobun mentioned this pull request Aug 23, 2026

Response(asyncIterable): do not write the value of a done iterator result #40216

Merged

robobun mentioned this pull request Sep 22, 2026

streams: do not swallow an ERR_INVALID_THIS error from an async-iterable body #43758

Merged

Navigation Menu

Support async generator functions in `Response` and `Request` for bodies - #8941

Support async generator functions in `Response` and `Request` for bodies#8941
paperclover merged 8 commits into
mainfrom
jarred/async-iteratable

Jarred-Sumner commented Feb 16, 2024 •

edited

Loading

Uh oh!

paperclover left a comment

Uh oh!

github-actions Bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

github-actions Bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

Electroid Feb 16, 2024

Uh oh!

Jarred-Sumner Feb 16, 2024

Uh oh!

jdalton Feb 17, 2024

Uh oh!

Uh oh!

paperclover left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Jarred-Sumner commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

How did you verify your code works?

Uh oh!

paperclover left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌🪟 @Jarred-Sumner, there are 14 test regressions on Windows x86_64

Uh oh!

Electroid Feb 16, 2024

Choose a reason for hiding this comment

Uh oh!

Jarred-Sumner Feb 16, 2024

Choose a reason for hiding this comment

Uh oh!

jdalton Feb 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paperclover left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jarred-Sumner commented Feb 16, 2024 •

edited

Loading

github-actions Bot commented Feb 16, 2024 •

edited

Loading

github-actions Bot commented Feb 16, 2024 •

edited

Loading

github-actions Bot commented Feb 16, 2024 •

edited

Loading

github-actions Bot commented Feb 16, 2024 •

edited

Loading

github-actions Bot commented Feb 16, 2024 •

edited

Loading