Re: [whatwg/streams] [WritableStream] Shouldn't the state after writer.abort() be closed? (#464)

> But we can have similar argument against readableStream.cancel(). Readable stream's "closed" state means end-of-data for normal cases. A cancelled stream is not telling us that it reached the end-of-data.

I think the difference is in consumer vs. producer. If a consumer is no longer interested in the stream, then it is not an error: nothing is broken. Everything is going as expected. A consumer is no longer interested in data in two cases: one, when there is no more data. And two, when they explicitly signal disinterest. Thus, cancel() sends you to the closed state.

Whereas if a producer can no longer give any data (and wants to throw away any queued data), then something is usually broken.

> If we have any logic that tries to do something further after .abort(), .cancel(), that's just wrong. If there's a fulfillment callback set to readableStream.closed, it should be neutered when we call readableStream.cancel().

I tend to agree with this. This is what motivates my ["third state" thinking for cancelable promises](https://github.com/domenic/cancelable-promise/blob/master/Third%20State.md), which would cause both fulfillment and rejection callbacks to be skipped. However, the timing is pretty poor here. We could potentially consider changing the semantics of `.cancel()` to cancel the `.closed` promise (instead of fulfilling it), but it would be backward-incompatible. I didn't want to introduce a path dependency on cancelable promises. Especially since they are controversial and the third state idea might not be able to make it through committee :(.

> Similarly, any code that is waiting for writableStream should be neutered when we're calling writableStream.abort().

I think this is a bit different. To me it makes sense that any code that is waiting for the writable stream that gets aborted should be told about abort as an error. You abort() because something broke. It is often good to learn about what broke and surface that error.

I agree it's a subtle distinction.

> The current setup has 2 points look strange to me:

To me these are not very big worries. (1) inconsistency doesn't worry me because we specifically named them differently. And (2) doesn't seem like a bad thing; I don't understand why it's a problem.

> (i) Ideally I'd like to change cancel() to enter "cancelled" state and make methods throw/reject with TypeError("Cancelled") and make abort() to enter "aborted" state and make methods throw/reject with TypeError("Aborted").

I think this is a bad user experience for people using readable streams. Imagine writing code like `rs.closed.catch(tellUserTheDownloadFailed)`. Then the user navigates to another part of your webapp, which means the download doesn't matter any more, so you do `rs.cancel()`. Then the user gets a popup saying the download failed, which is a bad user experience. It's similar to my arguments against cancelation as an error [in the cancelable promises repo](https://github.com/domenic/cancelable-promise/blob/master/Third%20State.md). (Maybe cancelation as success isn't super-great either, but I think for the particular case of streams it works pretty OK.)

> (ii) Changing abort() to transition to closed solves (1) and (2) but doesn't satisfy (a).

I don't object _too_ strongly to this. In this case we would rename `abort()` to `cancel()` and treat it as (C) from my earlier post. ("immediately close. Throw away any queued writes, and then move the stream to the closed state").

It comes down to the subtle issue of whether you believe that when  producer can no longer give any data (and wants to throw away any queued data), then something is usually broken. If you think that producers actually want to do that for other reasons than things being broken, then maybe (ii) is OK. But I think usually in this case something will be broken.

> (iii) Changing abort(reason) not to set reason to storedError but set it to TypeError("Aborted") solves (2) and satisfies (a).

I would be OK with this, but I don't understand why (2) is a problem we want to solve. It seems like a reasonable thing to say "the reason the stream is broken is because `reason`" and that further interactions with the stream will tell you that. In other words, it sounds like you would prefer "the reason the stream is broken is because it was aborted" whereas I am thinking it's useful to say "the reason the stream is broken is because `reason` (which is why we aborted it)".

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/issues/464#issuecomment-228124026

Received on Thursday, 23 June 2016 17:35:33 UTC