Skip to content

fix(service-worker): do not blow up when caches are unwritable #26042

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

Closed
wants to merge 1 commit into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Sep 20, 2018

PR Checklist

PR Type

[x] Bugfix

What is the current behavior?

In some cases, example when the user clears the caches in DevTools but the SW remains active on another tab and keeps references to the deleted caches, trying to write to the cache throws errors (e.g. Entry was not found).

When this happens, the SW can no longer work correctly and should enter a degraded mode allowing requests to be served from the network.

Possibly related:

What is the new behavior?

In such situations, the SW will enter the degraded EXISTING_CLIENTS_ONLY mode and forward requests to the network.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@gkalpak gkalpak added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: service-worker Issues related to the @angular/service-worker package labels Sep 20, 2018
@gkalpak gkalpak requested review from alxhub and IgorMinar September 20, 2018 18:22
@mary-poppins
Copy link

You can preview 20e23e2 at https://pr26042-20e23e2.ngbuilds.io/.


// Clear the caches and make them unwritable.
await clearAllCaches(scope.caches);
spyOn(MockCache.prototype, 'put').and.throwError('Can\'t touch this');
Copy link
Member

@JoostK JoostK Sep 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hammer time?

I'll show myself out

In some cases, example when the user clears the caches in DevTools but
the SW remains active on another tab and keeps references to the deleted
caches, trying to write to the cache throws errors (e.g.
`Entry was not found`).

When this happens, the SW can no longer work correctly and should enter
a degraded mode allowing requests to be served from the network.

Possibly related:
- GoogleChrome/workbox#792
- https://bugs.chromium.org/p/chromium/issues/detail?id=639034

This commits remedies this situation, by ensuring the SW can enter the
degraded `EXISTING_CLIENTS_ONLY` mode and forward requests to the
network.
@gkalpak gkalpak force-pushed the fix-sw-unwritable-cache branch from 20e23e2 to 4dff1de Compare September 20, 2018 19:10
@mary-poppins
Copy link

You can preview 4dff1de at https://pr26042-4dff1de.ngbuilds.io/.

Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing this one!

@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 24, 2018
@IgorMinar IgorMinar removed the request for review from alxhub September 24, 2018 16:17
@IgorMinar IgorMinar added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Sep 24, 2018
@IgorMinar
Copy link
Contributor

caretaker note: no g3 impact because this code is not synced into g3

kara pushed a commit that referenced this pull request Sep 24, 2018
In some cases, example when the user clears the caches in DevTools but
the SW remains active on another tab and keeps references to the deleted
caches, trying to write to the cache throws errors (e.g.
`Entry was not found`).

When this happens, the SW can no longer work correctly and should enter
a degraded mode allowing requests to be served from the network.

Possibly related:
- GoogleChrome/workbox#792
- https://bugs.chromium.org/p/chromium/issues/detail?id=639034

This commits remedies this situation, by ensuring the SW can enter the
degraded `EXISTING_CLIENTS_ONLY` mode and forward requests to the
network.

PR Close #26042
@kara kara closed this in 2bd767c Sep 24, 2018
@gkalpak gkalpak deleted the fix-sw-unwritable-cache branch September 24, 2018 17:34
@patrickmichalina
Copy link

Will this be merged in? It looks closed.

@kara
Copy link
Contributor

kara commented Nov 2, 2018

@patrickmichalina It is in master. See 2bd767c.

@patrickmichalina
Copy link

Safari still failing with #26500 with latest version 7.1.0-beta.1

@gkalpak
Copy link
Member Author

gkalpak commented Nov 5, 2018

@patrickmichalina, can you open a new issue including all necesary details (per the issue template).
(You can skip stackblitz, since it doesn't work for SW, but we need some kind of minimal reproduction (exact steps/repo/etc.).)

@patrickmichalina
Copy link

@gkalpak I should have an repo and production deployment that can show the issue. It might just take a week or so.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: service-worker Issues related to the @angular/service-worker package cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants