Allow to configure redirect mode for fetch

[friedger]

For our site template project (for Google Developer Group events), we would like to allow add the settings redirect: 'follow' to the fetch method in the 'install' handler.

This should solve our issue with wrong redirects, as described for our site here:
gdg-x/hoverboard#149
and on stackoverflow
http://stackoverflow.com/a/40277730/243599

Not sure how the configuration could look like in the sw-precache file though. Maybe

fetchInit = {redirect : 'follow'}

It is only considered if handleFetch is true.

[jeffposnick]

As per https://fetch.spec.whatwg.org/#concept-request-redirect-mode, the default value for redirect is 'follow'. Since there's nothing in the service worker code that explicitly sets a value for the redirect mode in any of the fetch() or cache.add() calls, my expectation is that it's already effectively doing what you're asking for.

So... can you confirm that you actually see buggy behavior that is fixed when you change

return cache.add(new Request(cacheKey, {credentials: 'same-origin'}));

to

return cache.add(new Request(cacheKey, {credentials: 'same-origin', redirect: 'follow'}));

inside the install handler of the generated service worker? If you do see a difference in behavior, what browsers are you seeing it in?

Or if that's not the code that you're talking about changing, where are you asking for {redirect: 'follow'} to be set?

If you have a specific site that's currently exhibiting problems with the default behavior, I'm happy to investigate that as well.

[friedger]

Inside the install handler I would like to changed

var request = new Request(urlWithCacheBusting,
              {credentials: 'same-origin'});
            return fetch(request).then(function(response) {
              if (response.ok) {

to

var request = new Request(urlWithCacheBusting,
              {credentials: 'same-origin', redirect: 'follow'});
            return fetch(request).then(function(response) {
              if (response.ok) {

Looks like this is an older version.

However, I can't reproduce at the moment that this change will fix the behaviour.

This is only experienced with firefox both on desktop and android. See gdg-x/hoverboard#149

The site devfest.be and https://devfest.gdg.org.ua/ still shows this behaviour. Source code for both are available at https://github.com/BruGTUG/DevFest2016 and https://github.com/GDG-Ukraine/hoverboard

[jeffposnick]

So you're using a service worker generated by an older version of sw-precache, which means the code doesn't match up. You could just try updating to the latest sw-precache release and see if that makes a difference, though I'm not sure it will.

If you visit https://devfest.gdg.org.ua/ in Firefox v49.0.1 without an existing service worker registration (delete ~/Library/Application Support/Firefox/ on a Mac, or the appropriate directory for your operating system), then you'll see the following in the Console:

App IndexedDB Client connecting..schedule-page.html-1.js:1:970
SyntaxError: expected expression, got '<'schedule-page.html-1.js:1:1333
Polymer.AppIndexedDBMirrorClient.prototype.connect/this[e]</<()schedule-page.html-1.js:1

I don't know what's causing this error in Firefox and not in Chrome, but I'm thinking that whatever it is, it's resulting in unexpected content getting stored in the service worker's cache, and then that's affecting subsequent navigations.

Since the default when creating a new Request object should already be a redirect mode of 'follow', I don't know that your proposed change would help. I'd recommend tracking down what's going on with that initial error related to IndexedDB, fix that, and see if it helps.

[wireforce]

Hi! I think we have the same problem as described in this issue. We have a service-worker that caches the root url "/", this url redirects to the users languageCode, e.g "/en/". This works fine in Chrome, but in Firefox 49+ the site does not work at all... Firefox displays a general error page saying something about a "network violation", and in the console you see: "A service worker has handed over a "redirect" response to a FetchEvent.respondWith() although the RedirectMode was not 'follow'"

Unfortunately, cache.add(new Request(cacheKey, {credentials: 'same-origin', redirect: 'follow'}));, did not solve the problem... I had to completely disable the cache for this url, for firefox users. Any ideas?

Also, while the statement above: "Since the default when creating a new Request object should already be a redirect mode of 'follow'", seems to be true. The browsers probably don't honor this fully. See here:
https://developer.mozilla.org/en-US/docs/Web/API/Request/Request
"The redirect mode to use: follow, error, or manual. In Chrome the default is manual before Chrome 47 and follow starting with Chrome 47."
And here:
https://developer.mozilla.org/en-US/docs/Web/API/GlobalFetch/fetch
"In Chrome the default was follow before Chrome 47 and manual starting with Chrome 47."

I'm a bit confused :) Any ideas are appreciated.

[sophieH29]

Same problem for me. @wireforce you are right, that's a bit confusing.
After I've added manually {redirect : 'follow'} to generated SW and deployed, it worked for me.

[jeffposnick]

I've filed #233 to explicitly set redirect: 'follow', since that's what I had assumed the value would be by default, and it sounds like some browsers vary from that. #220 (comment) is encouraging that this should fix at least some of the occurences of this type of issue.

@wireforce, I'd love to see a reproduction of an issue that isn't resolved when redirect: 'follow' is used.

[jeffposnick]

The change in #233 has been deployed to the 4.3.0 release of sw-precache. If folks who were previously seeing this issue could update and regenerate their service worker file, that would be appreciated.

Please let me know if any issues remain related to this when using 4.3.0.

[ozasadnyy]

@jeffposnick seems like it doesn't work, still all requests are with redirect: "manual"

Firefox doesn't work due to error:
A ServiceWorker passed a redirected Response to FetchEvent.respondWith() while RedirectMode is not ‘follow’.

Demo: https://hoverboard-experimental.firebaseapp.com/
Environment:
sw-precache: 4.3.0
firefox: 50.1
os: macOS Sierra 10.12.2

[jeffposnick]

@ozasadnyy, thanks for passing along those steps to reproduce and screenshots. It's a lot clearer what's happening now, and setting mode: 'follow' on the precaching request definitely isn't the right fix.

I've found related discussions on the various standards GitHub repos and browser bug trackers. As per https://bugs.chromium.org/p/chromium/issues/detail?id=669363&desc=2#c1 there are at least two possible workarounds.

I think the workaround that makes the most sense would be similar the first one, in which sw-precache checks the response for a precaching request to see if it's redirected, and if so, manually creates a synthetic cache entry using that redirect response's body. That should, as far as I can tell, be backwards compatible. I'll loop in some folks who work on the service worker specification to double-check my thinking.

Firefox was the first browser to enforce this new security restriction, which is why it stopped working there first. Chrome is going to start enforcing it in version 59 in a few months.

[jeffposnick]

I've got a potential fix checked in to a branch. I'm going to ask some more folks to look at it, but in the meantime, @ozasadnyy, would you mind trying to either manually edit your generated service worker to match the changes in https://github.com/GoogleChrome/sw-precache/compare/handle-redirected-responses, or alternatively, pull in a local instance of sw-precache pinned to that branch via something like

npm install https://github.com/googlechrome/sw-precache#handle-redirected-responses

and then regenerate your service worker?

[ozasadnyy]

@jeffposnick I have deployed updated version using npm install https://github.com/googlechrome/sw-precache#handle-redirected-responses
but it didn't help: https://hoverboard-experimental.firebaseapp.com/

[jeffposnick]

@ozasadnyy Sorry about that—I've added a couple of extra commits to https://github.com/GoogleChrome/sw-precache/compare/handle-redirected-responses and that might help.

I bumped the cache version string to ensure that the entirety of the cache is repopulated when deploying this update. You may have been hitting an older cache entry which didn't have the correct redirected property.

In addition to that, Firefox wasn't happy even when starting with an empty cache, because I wasn't copying over the response headers and it didn't automatically interpret the response as text/html, so I'm now copying that over.

Can you update to the latest code in that branch and give it another go?

[ozasadnyy]

@jeffposnick well, now I don't have SW error but site doesn't work after refresh https://hoverboard-experimental.firebaseapp.com/

[jeffposnick]

Hmm, so that's interesting 🙁

I'm seeing difference in how Firefox handles this particular situation compared to Chrome, so I'm going to do a quick loop-in of @wanderview to see if he has any ideas. Here's what I'm seeing in the Firefox console; when the body for the synthetically created response is read from the cache, either via a navigation request or just via fetch(), it comes back empty:

The same works fine from Chrome 58 Canary:

Also, rather than asking you to keep redeploying your site, can I just use https://github.com/gdg-x/hoverboard deployed to a Firebase hosting instance? Or do I need to do anything special to reproduce locally?

[ozasadnyy]

@jeffposnick feel free to use develop branch https://github.com/gdg-x/hoverboard/tree/develop
just run bower install, npm install and gulp to build the project

[wanderview]

Firefox does not have the Response.body attribute yet. So you will get a null body if you try to do this:

+                  new Response(response.body, {
+                    headers: response.headers,
+                    status: response.status,
+                    statusText: response.statusText
+                  })

[wanderview]

You can do:

function launder(redirectedResponse) {
  return redirectedResponse.text().then(text => {
    return new Response(text, {
      headers: redirectedResponse.headers,
      status: redirectedResponse.status,
      statusText: redirectedResponse.statusText
    });
  });
}

It would be nice if the opaque redirect exposed the target redirect URL. Not sure why it doesn't. Then you could just do the redirect manually in your SW and come out with a "clean" Response.

[wanderview]

Alternatively, could you just cache the opaque-redirect in Cache API and return it to the outer page? Why do you want to follow it yourself here?

[wanderview]

For example, your install event handler could do something like this:

addEventListener('fetch', evt => {
  // index.html redirects to "/".  Therefore use  cache.put() to offline the
  // the opaque redirect.
  fetch('index.html', { redirect: 'manual' }).then(resp => {
    return caches.open('db').then(cache => {
      return cache.put(evt.request, resp);
    });
  });
});

You can then just return the cache.match(evt.request) for index.html and the opaque redirect will be passed back to the page which then should redirect correctly.

[jeffposnick]

Thanks, @wanderview!

I think it's more backwards-compatible to launder and cache the actual redirected response body instead of caching the redirect itself, since that's what was happening prior to this additional security restriction.

That also has the advantage of being a generic solution without having to specifically know ahead of time whether a given response was going to someday be used to fulfill a redirect === 'manual' request. All the caching is happening in the install handler, not inside a fetch.

Finally, I need to put some logic in place to version each URL so that I know when to update cache entries, and that gets more complicated if there isn't a 1:1 mapping between original URL and cached response.

I'm going to go with an approach that first attempts to use response.body and then falls back to the text() promise if it's not supported.

[wanderview]

Ok. Note, you only really need to do all this for responses that will be served to navigation requests. Request.mode === 'navigation'

[jeffposnick]

@wanderview Gotcha. But since the precaching all happens during install, I don't want to try to get too clever and infer how a response is going to be used ahead of time. Something that "just works" the same way as it previously did is preferable in this case.

[wanderview]

Ok. Just keep in mind its very memory inefficient to do it this way compared with putting the opaque redirect into the Cache API. Doing it for every resource in a large site simultaneously could go badly on mobile.

[jeffposnick]

Well, it will only be done when .redirected is true. Otherwise I'll just continue caching the original response as-is.

I'm assuming that it's only a small fraction of the request URLs in a precache list, if any, that result in a HTTP 3xx redirect.

[ozasadnyy]

Hello, @jeffposnick . It is broken again :(
Firefox 51.0.1 (64-bit)

Chrome 60.0.3091.0 (Official Build) canary (64-bit)

[jeffposnick]

@ozasadnyy, can you please check the version of sw-precache that you're using? sw-precache v5.0.0 or higher should contain the correct code.

The PR to "clean" redirected responses also bumped the sw-precache VERSION constant from v2 to v3.

The deployment that you linked to is still using caches named with the v2 identifier, which leads me to believe that it's not using the correct sw-precache release:

[scraly]

Here the dependencies we have and we have corrupted error on Firefox :-(.

[wanderview]

Here the dependencies we have and we have corrupted error on Firefox :-(.

But not in chrome canary? Any messages in the web console when it fails in FF?

[scraly]

We have an error log in chrome:

Message in webconsole on Firefox:

[scraly]

We have an error too in chrome canary:

[jeffposnick]

https://2017.devfesttoulouse.fr/ is still using an old version of sw-precache as well. I can tell from the v2 in the cache identifier string:

If and old sw-precache is being pulled in from another dependency or build tool, then using npm's shrinkwrap configuration could force the latest release to be pulled in: https://docs.npmjs.com/cli/shrinkwrap

[davinkevin]

The dependency which requires sw-precache@^4.2.0 is the polymer-build. No update has migrated to the sw-precache@^5.0.0, so I don't have a simple solution.

If I understand well, you advice me to generate the shrinkwrap file and then modify it to replace the dependency I need to upgrade ?

So, for polymer-build (I have removed all useless information for example):

{
    "polymer-build": {
      "version": "0.7.1",
      "from": "polymer-build@>=0.7.0 <0.8.0",
      "resolved": "https://registry.npmjs.org/polymer-build/-/polymer-build-0.7.1.tgz",
      "dev": true,
      "dependencies": {
        "sw-precache": {
          "version": "4.3.0",
          "from": "sw-precache@>=4.2.0 <5.0.0",
          "resolved": "https://registry.npmjs.org/sw-precache/-/sw-precache-4.3.0.tgz",
          "dev": true
        }
      }
    }

should became :

{
    "polymer-build": {
      "version": "0.7.1",
      "from": "polymer-build@>=0.7.0 <0.8.0",
      "resolved": "https://registry.npmjs.org/polymer-build/-/polymer-build-0.7.1.tgz",
      "dev": true,
      "dependencies": {
        "sw-precache": {
          "version": "5.0.0",
          "from": "sw-precache@>=5.0.0 <6.0.0",
          "resolved": "https://registry.npmjs.org/sw-precache/-/sw-precache-5.0.0.tgz",
          "dev": true
        }
      }
    }

I would like to do a PR for the hoverboard project, but it seems to be an hacky solution...

This modification doesn't seem to work, because polymer-build is a dev dependency...

I've done the same thing with yarn and it's ok... Do you have any clue to solve it with npm ?

[FredKSchott]

@davinkevin we'll upgrade the dependency within polymer-build so that you don't need to monkey around in your node_modules folder.

@jeffposnick can we mark <v5 of sw-precache as "deprecated" on npm so that it shows a warning on install? Most users probably are not aware that this will break in Firefox / Chrome 59 (I know I wasn't until @davinkevin / @ozasadnyy / @Splaktar reported it).

[jeffposnick]

Yup, thanks for the suggestion—I'll mark < 5.0.0 as deprecated.

[farrukhsheikh248]