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

Should consumers of readers/streams be notified why a stream was canceled? #448

Closed
jplaisted opened this issue May 14, 2016 · 2 comments
Closed

Comments

@jplaisted
Copy link

With ReadableStreams it appears that if you cancel a stream (or its Reader) the reason for the cancel is only given to the underlying source and not to any consumers. For consumers any pending promises from getReader()/read()/close() will be resolved with undefined/undefined/undefined, true. There is no indication to them that the stream could have ended early or why it did so.

I think this would be convenient to have for consumers; that way they can handle ALL instances of a stream ending early, expected or not, in the same method. Meaning I feel that cancel should reject all pending promises with the cancel reason rather than resolve them with no information.

This would also be slightly confusing for handling getReader() as the reader could be undefined if the stream was canceled.

I also get that there are streams where there is no such thing as ending "early" (some kind of dynamic infinite stream that is being cleaned up); but, again, if its an expected reason then the consumer should be able to ignore it.

In the current system a consumer will cancel a stream, store a bit that they did cancel it, and then check said bit any time a promise resolves so they ignore the result. Instead what I'm proposing is they just cancel it with a reason, then when a promise rejects if the reason is expected then ignore it. No need to store a variable to remember that you canceled the stream to ignore "phony" results.

@domenic
Copy link
Member

domenic commented May 16, 2016

We're trying to make a distinction between "cancelation", which is just "the stream is ending early and you should not care about the results anymore", versus "erroring", which is "the stream has ended early in a bad way; be aware." It's the latter that causes promise rejections. Cancelation should generally be "expected", in that there's nothing wrong with a canceled stream.

In the current system a consumer will cancel a stream, store a bit that they did cancel it, and then check said bit any time a promise resolves so they ignore the result. Instead what I'm proposing is they just cancel it with a reason, then when a promise rejects if the reason is expected then ignore it. No need to store a variable to remember that you canceled the stream to ignore "phony" results.

This is not generally how cancelation is meant to be used. If you cancel the stream, you should not be reading from it any more. There are no "phony" results; there are in fact zero more results at all.

@jplaisted
Copy link
Author

Hmm I think I had confused myself. Few weeks later I'm a bit more familiar with the spec.

I was thinking ReadableStreams had a close promise, but its their readers that do. So what I was referring to was you could construct a ReadableStream, pass it off to something to consume it, and then wait for it to close before performing some other action. However it would not have been clear at that upper layer is the closure was abrupt or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants