Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch: using for await with .body does not throw error after being consumed #1655

Closed
KhafraDev opened this issue Sep 17, 2022 · 1 comment
Closed
Labels
bug Something isn't working fetch

Comments

@KhafraDev
Copy link
Member

KhafraDev commented Sep 17, 2022

Bug Description

Using for await (const whatever of response.body) {...} you can kinda-kinda not consume a body multiple times.

Reproducible By

import { fetch } from 'undici'

const response = await fetch('https://www.google.com')

for await (const chunk of response.body) {}
for await (const chunk of response.body) {}

Expected Behavior

I would consider any output other than throwing an error a bug. For all body mixin methods, an error is thrown when consuming the body more than once. Currently the second iterator won't return anything which is also very confusing behavior.

I guess something along the lines of:

const readable = new ReadableStream({
  start () {},
  pull () {}
})

readable[Symbol.asyncIterator] = undefined

This will match browser behavior by causing an error to be thrown when using it in a for await loop like browsers.

Or it could be reimplemented to include a body used check and throw an error if its been consumed?

Logs & Screenshots

Environment

Additional context

Node implements an async iterator onto the web ReadableStream, unlike browsers.

Run in repl/browser console:

ReadableStream.prototype[Symbol.asyncIterator]

This is where a ReadableStream is constructed:

function ReadableStreamFrom (iterable) {

I didn't really know what to do with this one. Browsers (Firefox & chrome) throw an error but deno has the same "buggy"/confusing behavior as node.

@KhafraDev KhafraDev added bug Something isn't working fetch labels Sep 17, 2022
@KhafraDev
Copy link
Member Author

So the stream spec was updated to include async iterators in whatwg/streams#980, which means that we can't override the async iterator implementation, or get rid of it completely.

I opened an issue on the spec repo for this at whatwg/fetch#1488 which seems like the more applicable place to post this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fetch
Projects
None yet
Development

No branches or pull requests

1 participant