Re: [whatwg/fetch] Editorial: Reword how-to section to explain how to use callbacks & controller (PR #1614)

@jyasskin commented on this pull request.

This looks great, thanks! I think we should also run it by a couple of the more novice specification authors who have wondered how to integrate with Fetch recently, to see if it answers their questions. I'll send it to a couple once you've had a chance to integrate or disagree with my comments here.

> -  is primarily useful for standards that want to operate on <a for=/>response</a>'s
-  <a for=response>body</a>'s <a for=body>stream</a> directly.
-
-  <p>If the <a for=/>request</a>'s <a for=request>mode</a> is "<code>navigate</code>" and its
-  <a for=request>redirect mode</a> is "<code>manual</code>", then callers need to follow a very
-  specific flow with this algorithm to get the intended behavior. They should compute the
-  appropriate <a for=response>location URL</a>, and if it is non-null or failure, then they should
-  call <a for="fetch controller">process the next manual redirect</a>. This will result in
-  <a for=fetch><i>processResponse</i></a> being called again, with the next <a for=/>response</a>
-  in the redirect chain.
-
- <dt><a for=fetch><i>processResponseEndOfBody</i></a>
- <dd><p>Takes an algorithm that will be passed a <a for=/>response</a>. Indicates the network is
- done transmitting the response. This does not read <a for=/>response</a>'s
- <a for=response>body</a>.
+  <p>This is the most common way in whochclients handle a <a for=/>response</a>, for example scripts

```suggestion
  <p>This is the most common way in which clients handle a <a for=/>response</a>, for example scripts
```

> +  <p>This is the most common way in whochclients handle a <a for=/>response</a>, for example scripts
+  and styles. The <a for=/>response</a>'s <a for=response>body</a> is downloaded in its entirety

Would it make sense to link "scripts" and "styles" to the algorithm that uses fetch in this way?

> +  algorithm accepts a <a for=/>response</a> and the final contents, which could be null if there was
+  a <a>network error</a>, failure if there was an error while downloading, or a <a>byte sequence</a>
+  representing the successfully downloaded <a for=response>body</a>.

I stumbled over this sentence the first time I read it. I'm not sure if we should have a `<ul>` or `<dl class=switch>` of the possible final contents, or if just splitting the sentence will work:

```suggestion
  algorithm accepts a <a for=/>response</a> and the final contents.
  The final contents could be null if there was
  a <a>network error</a>, failure if there was an error while downloading, or a <a>byte sequence</a>
  representing the successfully downloaded <a for=response>body</a>.
```

> +  <a for=fetch><i>processResponseEndOfBody</i></a> argument, which would be called once the response
+  is fully downloaded. Note that unlike <a for=fetch><i>processResponseConsumeBody</i></a>, passing

The passive voice here felt inconsistent with the bit about "callers are responsible to download it themselves". Maybe:

```suggestion
  <a for=fetch><i>processResponseEndOfBody</i></a> argument, which would be called once you have finished
  downloading the response. Note that unlike <a for=fetch><i>processResponseConsumeBody</i></a>, passing
```

> +  <a for=fetch><i>processResponse</i></a> argument of <a for/>fetch</a>. The given
+  algorithm accepts a <a for=/>response</a>. For convenience, you may also pass an algorithm to the

How about something like:

```suggestion
  <a for=fetch><i>processResponse</i></a> argument of <a for/>fetch</a>. The given
  algorithm is passed a <a for=/>response</a> when the response's headers have been
  received and is responsible for reading the <a for=/>response</a>'s 
  <a for=response>body</a>'s <a for=body>stream</a> in order to download the rest
  of the response. For convenience, you may also pass an algorithm to the
```

Moving the "callers are responsible" bit up to the description of the algorithm.

> -   <dd><a for=body>Fully reading</a> the contents of the <a for=/>response</a>'s
-   <a for=response>body</a> succeeded.
-  </dl>
-
-  <p class=warning>A standard that uses this argument cannot operate on <a for=/>response</a>'s
-  <a for=response>body</a> itself as providing this argument will cause it to be read and it can be
-  read only once.
+  <p>In some cases, for example when displaying video or progressively loaded images, clients might
+  want to stream the response, and process it one chunk at a time. The <a for=/>response</a> is
+  handed over to the <a for=request>client</a> once the <a for=response>header list</a> is processed, and
+  the client continues from there.
+
+  <p>To process a <a for=/>response</a> chunk-by-chunk, pass an algorithm to the
+  <a for=fetch><i>processResponse</i></a> argument of <a for/>fetch</a>. The given
+  algorithm accepts a <a for=/>response</a>. For convenience, you may also pass an algorithm to the
+  <a for=fetch><i>processResponseEndOfBody</i></a> argument, which would be called once the response

```suggestion
  <a for=fetch><i>processResponseEndOfBody</i></a> argument, which is called once the response
```

> -
-  <p>If the <a for=/>request</a>'s <a for=request>mode</a> is "<code>navigate</code>" and its
-  <a for=request>redirect mode</a> is "<code>manual</code>", then callers need to follow a very
-  specific flow with this algorithm to get the intended behavior. They should compute the
-  appropriate <a for=response>location URL</a>, and if it is non-null or failure, then they should
-  call <a for="fetch controller">process the next manual redirect</a>. This will result in
-  <a for=fetch><i>processResponse</i></a> being called again, with the next <a for=/>response</a>
-  in the redirect chain.
-
- <dt><a for=fetch><i>processResponseEndOfBody</i></a>
- <dd><p>Takes an algorithm that will be passed a <a for=/>response</a>. Indicates the network is
- done transmitting the response. This does not read <a for=/>response</a>'s
- <a for=response>body</a>.
+  <p>This is the most common way in whochclients handle a <a for=/>response</a>, for example scripts
+  and styles. The <a for=/>response</a>'s <a for=response>body</a> is downloaded in its entirety
+  into a <a>byte sequence</a>, and then processed by the <a for=request>client</a>.

The "client" in this section isn't the _request's_ [client](https://fetch.spec.whatwg.org/#concept-request-client); it's the web browser, or, for the purpose of this section, the specification using Fetch. I'm not sure if it's important to replace the "client" term with "specification" or similar, but I'm pretty sure it shouldn't link to the request's client.

> -   <dd>The <a for=/>response</a>'s <a for=response>body</a> is null, due to the response being a
-   <a for=/>network error</a> or having a <a>null body status</a>.
-
-   <dt>failure
-   <dd>Attempting to <a for=body>fully read</a> the contents of the <a for=/>response</a>'s
-   <a for=response>body</a> failed, e.g., due to an I/O error.
-
-   <dt>a <a>byte sequence</a>
-   <dd><a for=body>Fully reading</a> the contents of the <a for=/>response</a>'s
-   <a for=response>body</a> succeeded.
-  </dl>
-
-  <p class=warning>A standard that uses this argument cannot operate on <a for=/>response</a>'s
-  <a for=response>body</a> itself as providing this argument will cause it to be read and it can be
-  read only once.
+  <p>In some cases, for example when displaying video or progressively loaded images, clients might

```suggestion
  <p>In some cases, for example when displaying video or progressively loading images, clients might
```

And consider whether you can link to existing specs that do this well.

> +  the client continues from there.
+
+  <p>To process a <a for=/>response</a> chunk-by-chunk, pass an algorithm to the
+  <a for=fetch><i>processResponse</i></a> argument of <a for/>fetch</a>. The given
+  algorithm accepts a <a for=/>response</a>. For convenience, you may also pass an algorithm to the
+  <a for=fetch><i>processResponseEndOfBody</i></a> argument, which would be called once the response
+  is fully downloaded. Note that unlike <a for=fetch><i>processResponseConsumeBody</i></a>, passing
+  the <a for=fetch><i>processResponse</i></a> or  <a for=fetch><i>processResponseEndOfBody</i></a>
+  does not guarantee that the response will be fully downloaded, and callers are responsible to
+  download it themselves by reading the <a for=/>response</a>'s <a for=response>body</a>'s
+  <a for=body>stream</a>.
+
+  <div id=example-callback-chunk-by-chunk class=example>
+   <ol>
+    <li><p>Let <var>request</var> be a <a for=/>request</a> whose <a for=request>URL</a> is
+    "https://stream.example.com", and whose <a for=request>client</a> is <a>this</a>'s

```suggestion
    "<code>https://stream.example.com</code>", and whose <a for=request>client</a> is <a>this</a>'s
```
To match https://infra.spec.whatwg.org/#string.

> +      <a for=response>body</a>'s <a for=body>stream</a>, and process it using the result of
+      <a>extracting a MIME type</a> from <var>response</var>'s <a for=response>headers list</a>.

This read as if "extracting a MIME type" were going to return an algorithm that could process the response, but I think you mean to imply that "process" would be a link to one of the specification's algorithms. Maybe:

```suggestion
      <a for=response>body</a>'s <a for=body>stream</a>, and process in an appropriate way for
      the MIME type identified by
      <a>extracting a MIME type</a> from <var>response</var>'s <a for=response>headers list</a>.
```

> +  <a for=/>fetch</a> is optional, so omitting the callback would <a for=/>fetch</a> without
+  expecting a response.

Does this cancel the response stream appropriately, in the case that the server sends a big response body that we're not going to use? I think it'll make potential callers more comfortable to see that written.

>  
- <dt><a for=fetch><i>useParallelQueue</i></a>
- <dd><p>Takes a <a for=/>boolean</a> that defaults to false. Indicates where the algorithms passed
- as arguments will be invoked. Hopefully most standards will not need this.
+ <dt>Ignore the response

Is there also a common case where we'd want the headers but not the body?

>  
- <dt><a for=fetch><i>useParallelQueue</i></a>
- <dd><p>Takes a <a for=/>boolean</a> that defaults to false. Indicates where the algorithms passed
- as arguments will be invoked. Hopefully most standards will not need this.
+ <dt>Ignore the response
+ <dd>
+  <p>In some cases, there is no need for a <a for=/>response</a> at all, e.g. in the case of
+  <code>navigator.sendBeacon()</code>. Processing a response and passing callbacks to
+  <a for=/>fetch</a> is optional, so omitting the callback would <a for=/>fetch</a> without
+  expecting a response.
+
+  <p id=example-no-callback class=example><a for=/>Fetch</a> a <a for=/>request</a> whose
+  <a for=request>URL</a> is "https://fire-and-forget.example.com", <a for=request>method</a> is

```suggestion
  <a for=request>URL</a> is "<code>https://fire-and-forget.example.com</code>", <a for=request>method</a> is
```

>  </dl>
 
-<p>When invoked, the <a for=/>fetch</a> operation returns a <a for=/>fetch controller</a>. The
-controller is used for performing actions on a fetch operation that has already started, such as
-<a for="fetch controller" lt=abort>aborting</a> the operation by the user or page logic, or
-<a for="fetch controller" lt=terminate>terminating</a> it due to a browser-internal circumstance.
-
-
+<p>Apart from the callbacks to handle responses, <a for=/>fetch</a> accepts additional callbacks
+for advanced cases.<a for=fetch><i>processEarlyHintsResponse</i></a> is intended specifically for
+<a for=/>responses</a> whose <a for=response>status</a> is 103, and is currently handled only by
+navigations. <a for=fetch><i>processRequestBodyChunkLength</i></a> and
+<a for=fetch><i>processRequestEndOfBody</i></a> notify the client of request body uploading
+progress.
+
+<p>Note that the <a for=/>fetch</a> operation is reentrant: it runs <a>in parallel</a>, and posts

I don't think mentioning "reentrant" is likely to help spec authors here:
```suggestion
<p>Note that the <a for=/>fetch</a> operation runs <a>in parallel</a>, and it posts
```

> -controller is used for performing actions on a fetch operation that has already started, such as
-<a for="fetch controller" lt=abort>aborting</a> the operation by the user or page logic, or
-<a for="fetch controller" lt=terminate>terminating</a> it due to a browser-internal circumstance.
-
-
+<p>Apart from the callbacks to handle responses, <a for=/>fetch</a> accepts additional callbacks
+for advanced cases.<a for=fetch><i>processEarlyHintsResponse</i></a> is intended specifically for
+<a for=/>responses</a> whose <a for=response>status</a> is 103, and is currently handled only by
+navigations. <a for=fetch><i>processRequestBodyChunkLength</i></a> and
+<a for=fetch><i>processRequestEndOfBody</i></a> notify the client of request body uploading
+progress.
+
+<p>Note that the <a for=/>fetch</a> operation is reentrant: it runs <a>in parallel</a>, and posts
+the aforementioned callbacks to a given <a for=/>event loop</a> which is usually the
+<a for=request>client</a>'s <a for="environment settings object">global object</a>. To process
+resposnes <a>in parallel</a> and handle reentrancy by yourself, <a for=/>fetch</a> with

```suggestion
responses <a>in parallel</a> and handle interactions with the main thread by yourself, <a for=/>fetch</a> with
```

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

Message ID: <whatwg/fetch/pull/1614/review/1327626190@github.com>

Received on Tuesday, 7 March 2023 21:03:10 UTC