Re: [whatwg/streams] Release reader immediately when shutting down a pipe (PR #1208)

@MattiasBuelens commented on this pull request.



> @@ -475,6 +483,20 @@ function WritableStreamDefaultWriterGetDesiredSize(writer) {
   return WritableStreamDefaultControllerGetDesiredSize(stream._controller);
 }
 
+function WritableStreamDefaultWriterIsOrBecomesErrored(writer, errorListener) {

Here's two possible tests for this:
```javascript
promise_test(async t => {
  const rs = recordingReadableStream();
  const ws = recordingWritableStream();

  const pipeToPromise = rs.pipeTo(ws);
  await flushAsyncEvents();

  rs.controller.error(error1);
  ws.controller.error(error2);

  await promise_rejects_exactly(t, error1, pipeToPromise, 'pipeTo must reject with readable\'s error');
  assert_array_equals(rs.eventsWithoutPulls, []);
  assert_array_equals(ws.events, []);

  await promise_rejects_exactly(t, error1, rs.getReader().closed);
  await promise_rejects_exactly(t, error2, ws.getWriter().closed);
}, 'Piping: error the readable stream right before erroring the writable stream');

promise_test(async t => {
  const rs = recordingReadableStream();
  const ws = recordingWritableStream();

  const pipeToPromise = rs.pipeTo(ws);
  await flushAsyncEvents();

  ws.controller.error(error1);
  rs.controller.error(error2);

  await promise_rejects_exactly(t, error1, pipeToPromise, 'pipeTo must reject with writable\'s error');
  assert_array_equals(rs.eventsWithoutPulls, ['cancel', error1]);
  assert_array_equals(ws.events, []);

  await promise_rejects_exactly(t, error1, ws.getWriter().closed);
  await rs.getReader().closed;
}, 'Piping: error the writable stream right before erroring the readable stream');
```
The behavior might be a bit surprising though. In the first test, *ws* is still writable when we call `rs.controller.error()`, so we end up in:
https://github.com/whatwg/streams/blob/5afb0e014c08be94e49db7c0305aa211545e3add/reference-implementation/lib/abstract-ops/readable-streams.js#L294


This adds at least one microtask of delay (even if there are no pending writes), so we will *not yet* call `ws.abort(error1)`. Instead, `ws.controller.error(error2)` goes through, and the abort gets ignored later on.

However, in the second test, because *ws* immediately becomes errored, we *don't* wait for pending writes to complete and instead we *synchronously* call `rs.cancel(error1)`. Therefore, `rs.controller.error(error2)` gets ignored, and the stream ends up *cancelled* instead of *errored*.

---

The specification is a bit vague about this. It says:
> Wait until every chunk that has been read has been written (i.e. the corresponding promises have settled).

It doesn't say *how long* this step can take. We may want to require that if there are *no* pending writes (i.e. we've never started any writes, or all writes have already settled), then this step must complete *synchronously*. Then, in the first test, we *would* call `ws.abort(error1)` synchronously and prevent `ws.controller.error(error2)`. However, that might be tricky to actually implement correctly... 🤔 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1208#discussion_r793139011

You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/streams/pull/1208/review/864228136@github.com>