-
Notifications
You must be signed in to change notification settings - Fork 831
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
Handling of quota exceeded errors in workbox-strategies #1308
Comments
I'm going to add that based on my reading of the code flow, I'm not sure whether our current implementation would necessarily lead to failures to get a valid response to the page. In the quota exceeded scenario, we'd still end up passing a valid response to But even if making this change is not strictly necessary, it would be cleaner to put our own logging in via |
@jeffposnick couple of points you've raised:
Could you explain what is happening today for users of Workbox and what you want the actual experience to be. |
@jeffposnick If we can reproduce the bad behavior (i.e. runtime cache HTML pages, load page in incognito and cache a large number of opaque requests - breaking the 120MB quota and then ensuring the page still loads). |
@gauntface, thanks for clarifying what the expected behavior of passing a rejected promise to But, approaching it from a different direction, let's say that a developer ends up in a situation where they've got runtime caching set up and they eventually bump against the quota limit. What, if anything, could Workbox do to help recover from that state automatically? If the runtime strategy is configured with cache expiration and workbox/packages/workbox-core/_private/cacheWrapper.mjs Lines 62 to 73 in aa95c44
If we were to look for a solution outside of the current model, I could imagine that anytime a |
Why don't we add an plugin error callback. call the functions and throw and error? |
Also just thinking more holistically - quota is going to be a pretty bad situation and doing it on a per route basis feels error prone. Should we have a custom callback that gets called when quota is reached and we clear out all caches we are aware of (apart from pre-caching)? |
For the initial installation, if there isn't enough storage for precaching assets, then service worker installation will fail. Which is fine, since it is not actionable if the initial allocated storage is smaller. If an update is found for a newer version of service worker script, but this time around it fails again not because of the storage allocation problem but the aggregated runtime caches are taking up the space. What do you think about deleting runtime caches in favor of freeing up space for precached assets? Quoted from @jeffposnick
This sounds like a reasonable strategy for runtime caches that are configured with expiration strategy. Do you think it'd be useful to have a default strategy in place when quota limit is hit? For example, we can start with the oldest cache, or the same file with different fingerprint etc.. Any more followup from this topic? I have been running into this issue as well, and have been contemplating on a good strategy for quota limit 🤔 |
Library Affected:
workbox-strategies
There's be an uptick of issues related to
DOMException: Quota exceeded.
reported recently, both from Workbox users and from the larger development community. A few bugs in Chrome, along with what appears to be a stricter enforcement of per-origin quota limitations (especially when opaque responses are cached) have all brought this to the forefront.This is likely to remain an issue if Safari goes ahead with a maximum limit of 50 MiB of cache storage when they ship service workers, as they've indicated.
We could do more to ensure that, when appropriate, Workbox will catch
DOMException: Quota exceeded.
that occur when writing to caches, and ensure that they lead to failures in passing along a valid response to the client page.I think it makes sense to address this at the
workbox-strategies
level, rather than in the underlyingcacheWrapper
fromworkbox-core
, because we want the flexibility to treat acache.put()
as a fatal exception for some use cases. A failure to write to caches withinworkbox-precaching
should still be fatal, and cause theinstall
event to fail, for instance.I'm thinking changing code like
workbox/packages/workbox-strategies/CacheFirst.mjs
Lines 142 to 149 in aa95c44
to be:
would help, along with corresponding changes in the other
workbox-strategies
.What do you think, @gauntface & co.?
The text was updated successfully, but these errors were encountered: