Re: [whatwg/fetch] Set body with byte reading support (PR #1593)

@saschanaz commented on this pull request.



> +
+   <li>Let |extractSize| be the smaller value of |available| and |desiredSize|.
+
+   <li>Let <var>bytes</var> be the result of extracting |extractSize| of bytes from
+   <var>buffer</var>.
+
+   <li>If |stream|s [=ReadableStream/current BYOB request view=] is non-null, then
+   [=ArrayBufferView/write=] |bytes| into |stream|'s [=ReadableStream/current BYOB request view=],
+   and set |view| to |stream|'s [=ReadableStream/current BYOB request view=]. <li>Otherwise, set
+   |view| to the result of [=ArrayBufferView/create|creating=] a {{Uint8Array}} from |bytes| in
+   |stream|'s [=relevant Realm=].
+
+   <li>[=ReadableStream/Enqueue=] |view| into |stream|.
+
+   <li><p>If |stream| is [=ReadableStream/errored=], then [=fetch controller/terminate=]
+   |fetchParams|'s [=fetch params/controller=].

I'm not quite sure either and honestly just copied this from the previous steps which was also right after the enqueueing step.

Looking at the [Gecko call diagram](https://searchfox.org/mozilla-central/query/default?q=calls-to%3A%27mozilla%3A%3Adom%3A%3AReadableByteStreamControllerError%27%20depth%3A4), it seems `ReadableByteStreamControllerRespond` can lead to `ReadableByteStreamControllerEnqueueClonedChunkToQueue` which can error the stream on a buffer clone failure. It's complex enough, I'd be happy if the spec can list the possible error reasons.

>I think it needs to be separate, something like, if the network blows up, terminate the controller and error the stream.

Yes, but I think that's a lil bit out of scope here as I see no relevant existing step, right?

> +   <li>Let <var>bytes</var> be the result of extracting |extractSize| of bytes from
+   <var>buffer</var>.
+
+   <li>If |stream|s [=ReadableStream/current BYOB request view=] is non-null, then
+   [=ArrayBufferView/write=] |bytes| into |stream|'s [=ReadableStream/current BYOB request view=],
+   and set |view| to |stream|'s [=ReadableStream/current BYOB request view=]. <li>Otherwise, set
+   |view| to the result of [=ArrayBufferView/create|creating=] a {{Uint8Array}} from |bytes| in
+   |stream|'s [=relevant Realm=].
+
+   <li>[=ReadableStream/Enqueue=] |view| into |stream|.
+
+   <li><p>If |stream| is [=ReadableStream/errored=], then [=fetch controller/terminate=]
+   |fetchParams|'s [=fetch params/controller=].
+
+   <li><p>If |stream| doesn't [=ReadableStream/need more data=], ask the user agent to
+   [=fetch/suspend=] the ongoing fetch.

Hmm, I was lazy and hoping I could just reuse the existing things... but you're right, since the HWM is now zero the desired size cannot be a positive number. I'll try adding some notes below `buffer`.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1593#discussion_r1084044848
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1593/review/1265576043@github.com>