Re: [w3c/ServiceWorker] Abort with fetch controller's error in Handle Fetch (PR #1655)

@noamr commented on this pull request.



> @@ -3057,7 +3057,10 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
                       1. If |navigationPreloadResponse|'s [=response/type=] is "`error`", reject |preloadResponse| with a `TypeError` and terminate these substeps.
                       1. Associate |preloadResponseObject| with |navigationPreloadResponse|.
                       1. Resolve |preloadResponse| with |preloadResponseObject|.
-              1. [=If aborted=], then [=fetch controller/abort=] |preloadFetchController|.
+              1. [=If aborted=], then:
+                  1. If |controller|'s [=fetch controller/serialized abort reason=] is non-null, let |deserializedError| be the result of calling [$StructuredDeserialize$] with |controller|'s [=fetch controller/serialized abort reason=] and |workerRealm|. If that threw an exception or returns undefined, then let |deserializedError| be a "{{AbortError}}" {{DOMException}}.
+                  1. Otherwise, let |deserializedError| be a "{{AbortError}}" {{DOMException}}.

I don't think you can do "Otherwise let".
I suggest to have this row before the previous one as the default, and then `if ... set` without an otherwise.

> @@ -3081,7 +3084,7 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
           1. Set |eventHandled| to [=a new promise=] in |workerRealm|.
           1. [=Queue a task=] |task| to run the following substeps:
               1. Let |e| be the result of <a>creating an event</a> with {{FetchEvent}}.
-              1. Let |requestObject| be a new {{Request}} object associated with |request| and a new associated {{Headers}} object whose [=guard=] is "`immutable`".
+              1. Let |requestObject| be the result of <a href="https://fetch.spec.whatwg.org/#request-create">creating</a> a {{Request}} object, given |request|, a new {{Headers}} object's [=guard=] which is "`immutable`", and |workerRealm|.

[=Request/create=]

> @@ -3103,7 +3106,10 @@ spec: storage; urlPrefix: https://storage.spec.whatwg.org/
                   1. Else, [=ReadableStream/cancel=] |request|'s [=request/body=] with undefined.
               1. If |response| is not null, then set |response|'s [=response/service worker timing info=] to |timingInfo|.
               1. If |e|'s <a>canceled flag</a> is set, set |eventCanceled| to true.
-              1. If |controller| [=fetch controller/state=] is "<code>terminated</code>" or "<code>aborted</code>", then [=queue a task=] to [=AbortSignal/signal abort=] on |requestObject|'s {{Request/signal}}.
+              1. If |controller| [=fetch controller/state=] is "<code>terminated</code>" or "<code>aborted</code>", then:
+                  1. If |controller|'s [=fetch controller/serialized abort reason=] is non-null, let |deserializedError| be the result of calling [$StructuredDeserialize$] with |controller|'s [=fetch controller/serialized abort reason=] and |workerRealm|. If that threw an exception or returns undefined, then let |deserializedError| be a "{{AbortError}}" {{DOMException}}.

Same comment about otherwise/let

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

Message ID: <w3c/ServiceWorker/pull/1655/review/1072442301@github.com>