Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Occasional responseToCache undefined #1368

Closed
djeeg opened this issue Mar 15, 2018 · 7 comments
Closed

Occasional responseToCache undefined #1368

djeeg opened this issue Mar 15, 2018 · 7 comments

Comments

@djeeg
Copy link

djeeg commented Mar 15, 2018

Library Affected:

https://github.com/GoogleChrome/workbox/blob/master/packages/workbox-core/_private/cacheWrapper.mjs#L174
Getting TypeError: Cannot read property 'ok' of undefined

Browser & Platform:
"workbox-sw": "^3.0.0",
"workbox-webpack-plugin": "^3.0.0"
Chrome 64

Issue or Feature Request Description:

How do i cause this .. possibly by deleting application in chrome/refreshing service worker a couple of times in a row. Sometimes deleting the application and a new tab helps. Sometimes waiting a bit clears it.

Maybe just another if (!responseToCache) check is needed (like a couple of lines above it)

@webmaxru
Copy link

webmaxru commented Mar 15, 2018

@gauntface
Copy link

Eek - annoyingly sporadic.

Thanks for the demo @webmaxru. I can raise a PR that checks for a response, but would have really liked a clearer explanation of why the response is undefined while it's being put in the cache and the PR could bury it..

@gauntface
Copy link

@webmaxru could you try your demo without skipWaiting and clientsClaim and see if you can reproduce the error?

@djeeg were you using skipWaiting and clientsClaim in your service worker?

@djeeg
Copy link
Author

djeeg commented Mar 16, 2018

No I'm not using either.

is actually an issue with precaching going from temp -> final cache

This is about the only time I the -temp cache under Cache Storage in Chrome

image

@gauntface
Copy link

@djeeg is there anything in the caches for v1 or v1-temp or are they empty?

@djeeg
Copy link
Author

djeeg commented Mar 16, 2018

I have only ever seen temp empty

image

And precache only has 1 or a couple of files in it

image

@gauntface
Copy link

Done plenty of digging into this with @jeffposnick and we have a theory (albeit difficult to prove).

At the moment the process is that during install we add files to a temporary cache and during activation these requests and responses are copied over to the final cache.

Using the debugger we saw that the install would cache the requests but when the activate step occured the temporary cache would be empty.

The reasoning behind may be explained by this subtle behavior of cache deletion: w3c/ServiceWorker#538

Specifically:

Per IRC and other issue discussions it seems that there is an expectation the underlying Cache data will continue to live until the last reference to the Cache object is dropped. At that point the underlying storage should be removed.

This means that any old SW with a reference to the temp cache will cause the cache NOT to delete until after it's closed, which won't happen until the new service worker claims all clients (from an activate event). This basically causes a really weird condition to work with.

I'm putting together a PR that instead of deleting the temporary cache, it simply clears it in the install step, meaning from install to activate we can rely on the entries existing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants