- From: Takeshi Yoshino <notifications@github.com>
- Date: Mon, 27 Jun 2016 00:45:32 -0700
- To: whatwg/streams <streams@noreply.github.com>
- Cc:
- Message-ID: <whatwg/streams/issues/464/228675316@github.com>
> I think the difference is in consumer vs. producer. ... I agree with you on this point. Unfinished WS is likely to be broken while unfinished RS is not broken. > A consumer is no longer interested in data in two cases: one, ... They're no longer interested in data after seeing closure, I agree. But transition itself often has some meaning, i.e. processed data FULLY. That's the main point I used for the counter argument. > 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. (Maybe cancelation as success isn't super-great either, but I think for the particular case of streams it works pretty OK.) Hmm, so, there's philosophy behind design of closed that rejection of closed should not depend on what actions we made on the RS? I agree that less path-dependency is good. But for fulfillment, we now have two paths to reach it. In rs.closed.then(tellUserTheDownloadSucceeded), tellUserTheDownloadSucceeded should know whether cancel has happened or not to do the right thing (show popup or do nothing). So, I'm ... convinced with that rejecting closed for cancel() is bad as well as fulfilling closed. You're also saying "Maybe cancelation as success isn't super-great...". Yeah. > 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 totally understand your logic. But the only disagreement is that having two different paths reach to fulfillment of closed on RS is fine while having two different paths "we aborted" and "WS errored" reach to rejection of closed on WS is bad. > And (2) doesn't seem like a bad thing; I don't understand why it's a problem. To surface an error, we need to know both who's the culprit and the type of error. We can of course include (a hint of) who's the culprit in the exception by e.g. having a file reading RS error using a descriptive error type, say, FileReadableStreamFailedByXXXException while having a file writing WS error FileWritableStreamFailedByXXXException. With this, a rejection callback set to ws.closed can show the right message (right culprit info) to user (or some other code) just by investigating the exception. It's not needed to know which path it has taken (called abort() or not). But this is redundant. For example, XXX can be common error such as "OutOfMemory". We can just use a single OutOfMemoryException across streams if we don't rely on the requirement above. I'm leaning toward forcing users to have their code interact with a WS while understanding what it did (cancel()-ed or not, etc.) to know in which path it is on to correctly interpret signal from WS (closed, etc.), than relying on that streams include (hint for knowing) culprit information in the exceptions. > 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)". Sorry, I was unclear about what I really don't like. Let me clarify. I'm uncomfortable with that an error passed to .abort() is set to storedError as-is on closed. See the OutOfMemory example above. I'm fine with including it on closed. "as-is" is the problem. For example, I'm totally fine if storedError is set to WritableStreamAbortedError and the instance has a reason field which includes the error passed to .abort(). --- 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-228675316
Received on Monday, 27 June 2016 07:46:10 UTC