Fix the perform the fetch steps in Update algorithm #1175
Conversation
This fixes the peform the fetch steps so as for it not to synchronously return true on success but to asynchronously coplete with response. This factors the promise rejection of the fail cases out of the perform the fetch steps so the fetch a * worker script algorithm's remaining step will invoke the Reject Job Promise with provided error data.
| 1. Invoke [=Reject Job Promise=] with |job| and "{{SecurityError}}" {{DOMException}}. | ||
| 1. Asynchronously complete these steps with a [=network error=]. | ||
| 1. Let |mimeType| be the result of [=extracting a MIME type=] from the |response|'s [=response/header list=]. | ||
| 1. If |mimeType| (ignoring parameters) is not a [=JavaScript MIME type=], asynchronously complete these steps with a [=network error=] and "{{SecurityError}}" {{DOMException}}, and abort these steps. |
There was a problem hiding this comment.
@domenic, I'm not sure if this change makes it better. My intention is to remove the Reject Job Promise call (here and other fail cases) within the perform the fetch steps and uniformly handle them in https://github.com/w3c/ServiceWorker/pull/1175/files#diff-27b79860afe28f01aed4f1f6228367faL2688. For this change, we need to change HTML as well to pass |error data| from the perform the fetch steps to fetch a * worker script to the post fetch steps.
There was a problem hiding this comment.
Another option is to call Reject Job Promise here (and other fail cases) as-is and run https://github.com/w3c/ServiceWorker/pull/1175/files#diff-27b79860afe28f01aed4f1f6228367faL2690 conditionally when the job promise is still pending. Which would be better?
There was a problem hiding this comment.
Note: Passing |error data| may be tricky for the synchronous fetch case where the response is the single return value.
There was a problem hiding this comment.
Hmm, yeah, this is pretty invasive. I think the "another option" is probably better.
However note that rejecting a promise that's already resolved (including being rejected) is a no-op. So maybe things are already fine? Or is it important to avoid the equivalentJob stuff too?
|
FYI I am looking forwarding to reviewing and helping with this but am at TC39 for the rest of the week so might not be able to get to it that soon :). |
|
Thanks for letting me know. Good meeting :). |
|
I think the code here has changed since this PR, eg at #1380 . So I'll close this as obsolete. |
This fixes the peform the fetch steps so as for it not to synchronously
return true on success but to asynchronously coplete with response. This
factors the promise rejection of the fail cases out of the perform the
fetch steps so the fetch a * worker script algorithm's remaining step
will invoke the Reject Job Promise with provided error data.