Runtime caching conflicts with precache

[jaspermdegroot]

When runtime caching is applied to a page that is precached, that page won't work offline anymore.

This affects the workbox-sw library. Tested on Chrome 61 on MacOS.

The Progressive Web AMP codelab at https://codelabs.developers.google.com/codelabs/amp-pwa-workbox/ illustrates the issue. The homepage is precached, but if you navigate from any of the articles to the homepage while offline the homepage won't be displayed and the console shows the following error:

The FetchEvent for "http://localhost:8081/" resulted in a network error response: the promise was rejected.
Promise rejected (async)
_isListenerRegistered.self.addEventListener @ workbox-sw.dev.v2.0.0.js:71

If you visit the homepage after the SW is installed and before going offline, so the page is added to the runtime cache, the page will be displayed.

I also included a more basic test case without app shell:

workbox-issue.zip

Is this indeed a bug or is this expected behaviour and is it up to the developer to keep precached files in mind when adding routes?

[jaspermdegroot]

I just saw the Request precached.txt example at https://workboxjs.org/examples/workbox-sw/ which shows the following message:

Requesting precached.txt...
...the response is 'I'm included in the manifest passed to workboxSW.precache([]), so I'll be added to the cache automatically whenever the service worker is registered. Additionally, I'll always be served with a cache-first strategy, even though there's also a runtime caching route that matches /\.txt$/. The cache-first route that .precache() sets up always takes precedence over any runtime caching routes. '.

Based on this I suppose the behaviour I noticed is a bug.

[gauntface]

@jaspermdegroot this definitely sounds like a bug to me. Will take a look later today. Thanks for raising the bug.

[jeffposnick]

This sounds like something that went amiss when we changed the code to make registering the event listener for the Router class opt-in.

The code from workbox-sw that currently reads:

if (handleFetch) {
  this._router.addFetchListener();
}

this._registerDefaultRoutes(ignoreUrlParametersMatching, directoryIndex);

needs to be switched around so that the default routes are registered first.

[jeffposnick]

On further inspection, it looks like this might date back to #382, where we moved to a single fetch handler per Router instance, combined with the fact that the last-registered Route takes precedence over earlier routes.

I'm going to give some thought as to how to get this specific use case fixed without introducing breaking changes to the order in which we evaluate Routes (since changes there would definitely be considered a breaking change).

In the meantime, @jaspermdegroot, you could call

workboxSW.router.setDefaultHandler(workbox.strategies.networkFirst());

to accomplish what you've intended. setDefaultHandler() registers something that will only run if there's a same-origin request that doesn't match any existing Route.

[gauntface]

What a short term solution be to register a two fetch handlers - one for precaching and one for routing?

We can't reliably add precaching to be the last route registered :(

[jeffposnick]

Yeah, that would probably be the best short-term solution. I can put something together for that in v2.

See #845 about some changes we could make in v3.

[jaspermdegroot]

Thanks all for the quick response and for fixing it!

@jeffposnick - Thanks for providing a temporary workaround. However, when setting the default handler I got an error saying the handler parameter is undefined instead of a function (I did change workbox.strategies to workboxSW.strategies).

Anyway, I only precache the homepage so I will use something like this until next release:

workboxSW.router.registerRoute('/*', args => {
	if (args.url.pathname!='/' && args.url.pathname!='/index.html') {
		return workboxSW.strategies.networkFirst().handle(args)
	}
})

[jeffposnick]

Sorry, that example should have been:

workboxSW.router.setDefaultHandler({handler: workbox.strategies.networkFirst()});

(We're in the process of moving from objects with named properties to positional properties for this sort of thing in v3, and I got confused.)