Re: [whatwg/fetch] Formalize reading bodies (#1172)

@domenic approved this pull request.

LGTM with editorial suggestions

> +algorithm <var>processBodyChunk</var>, an algorithm <var>processEndOfBody</var>, an algorithm
+<var>processBodyError</var>, and an optional null, <a for=/>parallel queue</a>, or
+<a for=/>global object</a> <var>taskDestination</var> (default null), run these steps.
+<var>processBodyChunk</var> must be an algorithm accepting a <a for=/>byte sequence</a>.
+<var>processEndOfBody</var> must be an algorithm accepting no arguments. <var>processBodyError</var>
+must be an algorithm accepting an exception.
+
+<ol>
+ <li><p>If <var>taskDestination</var> is null, then set <var>taskDestination</var> to the result of
+ <a>starting a new parallel queue</a>.
+
+ <li>
+  <p>Let <var>reader</var> be the result of <a for=ReadableStream>getting a reader</a> for
+  <var>body</var>'s <a for=body>stream</a>.
+
+  <p class=note>This operation ought not to throw an exception.

out not to -> will not?

> +  <dl>
+   <dt><a for="read request">chunk steps</a>, given <var>chunk</var>
+   <dd>
+    <ol>
+     <li><p>Let <var>continueAlgorithm</var> be null.
+
+     <li><p>If <var>chunk</var> is not a {{Uint8Array}} object, then set
+     <var>continueAlgorithm</var> to this step: run <var>processBodyError</var> given a
+     {{TypeError}}.
+
+     <li>
+      <p>Otherwise:
+
+      <ol>
+       <li><p>Let <var>bytes</var> be a <a lt="get a copy of the buffer source">copy of</a>
+       <var>chunk</var>.

Maybe note that this copy can often be elided if the user agent is working on its own streams, or e.g. a stream supplied from a service worker where the copy has already been made while crossing the realm boundary? I dunno, maybe not worth it...

> @@ -3241,9 +3226,11 @@ an optional algorithm <dfn export for=fetch id=process-response><var>processResp
 optional algorithm
 <dfn export for=fetch id=process-response-end-of-body oldids=process-response-end-of-file><var>processResponseEndOfBody</var></dfn>,
 and an optional boolean <dfn export for=fetch><var>useParallelQueue</var></dfn> (default false), run
-the steps below. If given, <var>processRequestBody</var> and <var>processRequestEndOfBody</var> must
-be an algorithm accepting a <a for=/>request</a>; <var>processResponse</var> and
-<var>processResponseEndOfBody</var> must be an algorithm accepting a <a for=/>response</a>.
+the steps below. If given, <var>processRequestBody</var> must be an algorithm accepting an integer.

"an integer representing the number of bytes transmitted"? I had to click around a lot to figure out what this was for.

> @@ -3241,9 +3226,11 @@ an optional algorithm <dfn export for=fetch id=process-response><var>processResp
 optional algorithm
 <dfn export for=fetch id=process-response-end-of-body oldids=process-response-end-of-file><var>processResponseEndOfBody</var></dfn>,
 and an optional boolean <dfn export for=fetch><var>useParallelQueue</var></dfn> (default false), run
-the steps below. If given, <var>processRequestBody</var> and <var>processRequestEndOfBody</var> must
-be an algorithm accepting a <a for=/>request</a>; <var>processResponse</var> and
-<var>processResponseEndOfBody</var> must be an algorithm accepting a <a for=/>response</a>.
+the steps below. If given, <var>processRequestBody</var> must be an algorithm accepting an integer.
+If given, <var>processRequestEndOfBody</var> must be an algorithm accepting no arguments.
+<var>processResponse</var> must be an algorithm accepting a <a for=/>response</a>.

"If given" for both of these

> @@ -3284,15 +3271,9 @@ the request.
  <a for="fetch params">process response end-of-body</a> is <var>processResponseEndOfBody</var>, and
  <a for="fetch params">task destination</a> is <var>taskDestination</var>.
 
- <li>
-  <p>If <var>request</var>'s <a for=request>body</a> is a <a for=/>byte sequence</a>, then:
-
-  <ol>
-   <li><p>Let <var>body</var> and <var ignore>ignoreType</var> be the result of
-   <a for=BodyInit>safely extracting</a> <var>request</var>'s <a for=request>body</a>.
-
-   <li><p>Set <var>request</var>'s <a for=request>body</a> to <var>body</var>.
-  </ol>
+ <li><p>If <var>request</var>'s <a for=request>body</a> is a <a for=/>byte sequence</a>, then set
+ <var>request</var>'s <a for=request>body</a> to the first return value of

"the first return value" seems pretty unusual; why not just "the result of"? Here and below.

>  
   <ol>
-   <li><p>Set <var>request</var>'s <a>done flag</a>.
+   <li><p>Let <var>processBody</var> given <var>nullOrFailureOrBytes</var> be this step: run
+   <var>fetchParams</var>'s <a for="fetch params">process response end-of-body</a> given
+   <var>response</var> and <var>nullOrFailureOrBytes</var>.
+
+   <li><p>Let <var>processBodyError</var> be to run <var>processBody</var> given failure.

Although this is clever, I think it'd be easier to understand if it directly called to the output algorithm, i.e.

> Let _processBodyError_ be this step: run _fetchParam_'s process response end-of-body given _response_ and failure.

>  
-   <li><p>If <var>fetchParams</var>'s <a for="fetch params">process response end-of-body</a> is
-   non-null, then run <var>fetchParams</var>'s
-   <a for="fetch params">process response end-of-body</a> given <var>response</var>.
+   <li><p>If <var>response</var>'s <a for=response>body</a> is null, then <a>queue a fetch task</a>
+   to <var>processBody</var> given null, with <var>fetchParams</var>'s

```suggestion
   to run <var>processBody</var> given null, with <var>fetchParams</var>'s
```

-- 
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/fetch/pull/1172#pullrequestreview-594590967