Re: [w3c/ServiceWorker] Reuse network response on both network and fetch handler failed (PR #1777)

@domenic commented on this pull request.



> @@ -3263,16 +3263,21 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
                   1. Let |queue| be an empty [=queue=] of [=race result=].
                   1. Let |raceFetchController| be null.
                   1. Let |raceResponse| be a [=race response=] whose [=race response/value=] is "<code>pending</code>".
+                  1. Let |networkFetchCompleted| be false.

I feel "completed" is a bit confusing, because it is not clear when a fetch is "completed". Currently it is set to true when the fetch is started, which is long before the response comes back.

Maybe it should be set inside _processResponseCallback_, i.e., intented one level?

> @@ -3263,16 +3263,21 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
                   1. Let |queue| be an empty [=queue=] of [=race result=].
                   1. Let |raceFetchController| be null.
                   1. Let |raceResponse| be a [=race response=] whose [=race response/value=] is "<code>pending</code>".
+                  1. Let |networkFetchCompleted| be false.
+                  1. Let |networkFetchResult| be null.
                   1. Run the following substeps [=in parallel=], but [=abort when=] |fetchController|'s [=fetch controller/state=] is "<code>terminated</code>" or "<code>aborted</code>":

The existing setup seems a bit fragile to me. The definition of [abort when](https://infra.spec.whatwg.org/#algorithm-conditional-abort) only operates between steps, and there are only two steps nested under "If aborted"... In fact there is only one step before this PR.

Probably there should be a check on fetchController's state inside the _processResponse_ callback?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1777#pullrequestreview-2919299527
You are receiving this because you are subscribed to this thread.

Message ID: <w3c/ServiceWorker/pull/1777/review/2919299527@github.com>