Re: [whatwg/fetch] Attach timing info and URL to network errors, and report for fetch API (#1311)

@annevk commented on this pull request.

Generally this seems to be heading in the right direction. Thanks for tackling this!

I haven't checked in detail whether this overlooks any network error creation.

I think the creation algorithm needs some slight tweaks.

And I think we want to change things such that processResponseDone also runs in the event of a network error.

> @@ -7417,8 +7479,14 @@ method steps are:
    with <var>p</var>, <var>request</var>, and <var>responseObject</var>, and terminate these
    substeps.
 
-   <li><p>If <var>response</var> is a <a>network error</a>, then <a for=/>reject</a> <var>p</var>
-   with a {{TypeError}} and terminate these substeps.
+   <li><p>If <var>response</var> is a <a>network error</a>, then:
+   <ol>
+    <li><p><a for=/>Reject</a> <var>p</var> with a {{TypeError}}.
+
+    <li><p>Call <var>handleFetchDone</var> with <var>response</var>.
+
+    <li><p>terminate these substeps.
+   </ol>

I think this is wrong. At least, in my mental model processResponseDone should run when you get a network error.

> @@ -2011,6 +2017,14 @@ known as an <dfn export id=concept-aborted-network-error>aborted network error</
 <a for=response>header list</a> is always empty, and
 <a for=response>body</a> is always null.
 
+<p>To <dfn for="network error" lt="create|creating">create</dfn> a <a>network error</a> given
+<a for=/>fetch params</a> <var>fetchParams</var>, return a new <a>network error</a> with its
+<a for=response>timing info</a> set to the result of <a>creating an opaque timing info</a> with
+<var>fetchParams</var>'s <a for="fetch params">timing info</a>, and its
+<a for=response>URL list</a> set to a <a for=/>list</a> containing <var>fetchParams</var>'s
+<a for="fetch params">request</a>'s <a for=request>URL</a>.
+
+

Nit: single newline here.

> @@ -2011,6 +2017,14 @@ known as an <dfn export id=concept-aborted-network-error>aborted network error</
 <a for=response>header list</a> is always empty, and
 <a for=response>body</a> is always null.
 
+<p>To <dfn for="network error" lt="create|creating">create</dfn> a <a>network error</a> given
+<a for=/>fetch params</a> <var>fetchParams</var>, return a new <a>network error</a> with its
+<a for=response>timing info</a> set to the result of <a>creating an opaque timing info</a> with
+<var>fetchParams</var>'s <a for="fetch params">timing info</a>, and its
+<a for=response>URL list</a> set to a <a for=/>list</a> containing <var>fetchParams</var>'s
+<a for="fetch params">request</a>'s <a for=request>URL</a>.

Nit: it seems that the network error is an argument to this algorithm currently although not an explicit one. E.g., sometimes this is passed an aborted network error rather than a network error. As such, it seems more like this algorithm initializes a network error rather than creating one.

-- 
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/1311#pullrequestreview-765382152