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

Iterating cancelled ReadableStream #39715

Closed
ronag opened this issue Aug 9, 2021 · 6 comments
Closed

Iterating cancelled ReadableStream #39715

ronag opened this issue Aug 9, 2021 · 6 comments

Comments

@ronag
Copy link
Member

ronag commented Aug 9, 2021

This is kind of weird behavior I'm not sure what to think about. Iterating a cancelled readable stream will not throw...

const { ReadableStream } = require('stream/web')

const stream = new ReadableStream({
  async start (controller) {
  },
  async pull () {
  },
  async cancel (reason) {
  }
})

async function main () {
  for await (const chunk of stream) {
  }
  assert(false) // ouch
}

stream.cancel(new Error())

main().catch(err => {
  console.log(err)
})

This means that e.g. helpers from #39594 can succeeded without any data.

@ronag
Copy link
Member Author

ronag commented Aug 9, 2021

@jasnell

@ronag
Copy link
Member Author

ronag commented Aug 9, 2021

Also as far as I know there is no introspection here where one could determine if the stream is cancelled before iterating it.

@jasnell
Copy link
Member

jasnell commented Aug 9, 2021

Yeah, this is expected behavior. The semantics of cancel() are essentially "I'm not interested in the data any more", so iterating over a canceled stream will just result in no data. Changing that behavior would require a change to the spec. I've been thinking about opening a proposal for the spec to add an api that allows introspection of the state.

@ronag
Copy link
Member Author

ronag commented Aug 9, 2021

Hm, this makes it difficult to implement fetch. Right now if the stream is cancelled we get a false success :/

@jasnell
Copy link
Member

jasnell commented Aug 9, 2021

The way fetch is currently defined, it's able to access the internal state of the ReadableStream. We'll likely have to do the same unless we're able to get a couple new public APIs added.

@ronag
Copy link
Member Author

ronag commented Aug 10, 2021

Actually fetch spec says to "error" the stream and not "cancel", so I assume it refers to controller.error which does error the iterator.

@ronag ronag closed this as completed Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants