Re: [whatwg/streams] Support teeing readable byte streams (#1114)

@MattiasBuelens commented on this pull request.



> +   1. Perform ! [$ReadableByteStreamControllerError$](|branch2|.[=ReadableStream/[[controller]]=],
+      |r|).
+   1. If |canceled1| is false or |canceled2| is false, [=resolve=] |cancelPromise| with undefined.
+ 1. Let |pullWithDefaultReader| be the following steps:
+  1. If |reader| [=implements=] {{ReadableStreamBYOBReader}},
+   1. Assert: |reader|.[=ReadableStreamBYOBReader/[[readIntoRequests]]=] is [=list/is empty|empty=].
+   1. Perform ! [$ReadableStreamReaderGenericRelease$](|reader|).
+   1. Set |reader| to ! [$AcquireReadableStreamDefaultReader$](|stream|).
+   1. Perform |forwardReaderError|, given |reader|.
+  1. Let |readRequest| be a [=read request=] with the following [=struct/items=]:
+   : [=read request/chunk steps=], given |chunk|
+   ::
+    1. [=Queue a microtask=] to perform the following steps:
+     1. Set |reading| to false.
+     1. Let |chunk1| and |chunk2| be |chunk|.
+     1. If |canceled1| is false and |canceled2| is false, set |chunk2| to ?

> Also related: I think the `? ReadableStreamDefaultControllerEnqueue` steps inside the chunk steps should be `!`. (Maybe all call sites for it should be??)

Indeed, it will only throw if `strategy.size()` throws, but that can never happen since we're using the default strategy. This is the same reasoning as we had for `ReadableStream.from()` in [#1083](https://github.com/whatwg/streams/pull/1083#discussion_r520886364).

> Anyway, what is the correct behavior? I think another option would be to only error branch 2. WDYT?

Well, we shouldn't ignore the failure like we did previously, since that means both branches get "stuck" (never receive a new chunk, or become closed or errored).

Only erroring branch 2 seems too risky. One place where we'll want to tee a readable byte stream is when Fetch clones the request to pass to the service worker (step 5.1 of [HTTP fetch](https://fetch.spec.whatwg.org/commit-snapshots/3babdcb8d1e564040b30bc824420ade870709e4f/#concept-http-fetch)). If the service worker reads the request body but does *not* provide a response (i.e. its `fetch` event handler does not call `event.respondWith()`), then we'll skip step 5.5 and we'll continue the fetch with the original request. However, if we've *failed to clone* a chunk, then the service worker would have successfully read the chunk (because its branch was providing the BYOB request), but our original request's body would have become errored, breaking the fetch.

To me, it seems better to error both branches if we can't clone the chunk.

-- 
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/pull/1114#discussion_r646882315