From 28f6c907085b0a672f63298a6d480abe9e05b0f5 Mon Sep 17 00:00:00 2001
From: Domenic Denicola <d@domenic.me>
Date: Mon, 25 Jun 2018 14:13:45 -0400
Subject: [PATCH] Fix some bugs in service worker code

Fixes #211, by not treating opaque responses as errors. Also logs warnings for failures to refresh the cache, to make this easier to debug in the future, and switches the "needs to be fresh" check to return true for all navigations.
---
 resources.whatwg.org/standard-service-worker.js | 14 ++++++++------
 resources.whatwg.org/website-service-worker.js  |  9 ++++++---
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/resources.whatwg.org/standard-service-worker.js b/resources.whatwg.org/standard-service-worker.js
index 2177842ea..3a795f204 100644
--- a/resources.whatwg.org/standard-service-worker.js
+++ b/resources.whatwg.org/standard-service-worker.js
@@ -8,7 +8,7 @@
 
 const standardShortname = location.host.split(".")[0];
 
-const cacheKey = "v4";
+const cacheKey = "v5";
 const toCache = [
   location.origin + "/",
   "https://resources.whatwg.org/spec.css",
@@ -46,11 +46,14 @@ self.onfetch = e => {
       caches.match(e.request).then(cachedResponse => {
         const networkFetchPromise = fetch(e.request);
 
-        // Ignore network fetch or caching errors; they just mean we won't be able to refresh the cache.
+        // Only warn on network fetch or caching errors; they just mean we won't be able to refresh
+        // the cache. (But, don't ignore them, because that could hide coding errors.)
         e.waitUntil(
           networkFetchPromise
             .then(res => refreshCacheFromNetworkResponse(e.request, res))
-            .catch(() => {})
+            .catch(e => {
+              console.warn(`Could not refresh the cache for ${e.request.url}`, e);
+            })
         );
 
         return cachedResponse || networkFetchPromise;
@@ -66,7 +69,7 @@ self.onactivate = e => {
 };
 
 function refreshCacheFromNetworkResponse(req, res) {
-  if (!res.ok) {
+  if (res.type !== "opaque" && !res.ok) {
     throw new Error(`${res.url} is responding with ${res.status}`);
   }
 
@@ -76,6 +79,5 @@ function refreshCacheFromNetworkResponse(req, res) {
 }
 
 function needsToBeFresh(req) {
-  const requestURL = new URL(req.url);
-  return requestURL.origin === location.origin && requestURL.pathname === "/";
+  return req.mode === "navigate";
 }
diff --git a/resources.whatwg.org/website-service-worker.js b/resources.whatwg.org/website-service-worker.js
index 40660f4a2..ede846092 100644
--- a/resources.whatwg.org/website-service-worker.js
+++ b/resources.whatwg.org/website-service-worker.js
@@ -35,11 +35,14 @@ self.onfetch = e => {
     caches.match(e.request).then(cachedResponse => {
       const networkFetchPromise = fetch(e.request);
 
-      // Ignore network fetch or caching errors; they just mean we won't be able to refresh the cache.
+      // Only warn on network fetch or caching errors; they just mean we won't be able to refresh
+      // the cache. (But, don't ignore them, because that could hide coding errors.)
       e.waitUntil(
         networkFetchPromise
           .then(res => refreshCacheFromNetworkResponse(e.request, res))
-          .catch(() => {})
+          .catch(e => {
+            console.warn(`Could not refresh the cache for ${e.request.url}`, e);
+          })
       );
 
       return cachedResponse || networkFetchPromise;
@@ -48,7 +51,7 @@ self.onfetch = e => {
 };
 
 function refreshCacheFromNetworkResponse(req, res) {
-  if (!res.ok) {
+  if (res.type !== "opaque" && !res.ok) {
     throw new Error(`${res.url} is responding with ${res.status}`);
   }