Skip to content

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

Closed
gkalpak wants to merge 1 commit into
angular:masterfrom
gkalpak:fix-sw-unwritable-cache
Closed

fix(service-worker): do not blow up when caches are unwritable#26042
gkalpak wants to merge 1 commit into
angular:masterfrom
gkalpak:fix-sw-unwritable-cache

Conversation

@gkalpak

@gkalpak gkalpak commented Sep 20, 2018

Copy link
Copy Markdown
Member

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 IgorMinar and alxhub September 20, 2018 18:22
@mary-poppins

mary-poppins commented Sep 20, 2018

Copy link
Copy Markdown

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

JoostK
JoostK reviewed Sep 20, 2018
View reviewed changes
Comment thread packages/service-worker/worker/test/happy_spec.ts Outdated

@JoostK JoostK Sep 20, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

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

@gkalpak
fix(service-worker): do not blow up when caches are unwritable
4dff1de 
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

mary-poppins commented Sep 20, 2018

Copy link
Copy Markdown

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

IgorMinar
IgorMinar approved these changes Sep 24, 2018
View reviewed changes

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

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

IgorMinar commented Sep 24, 2018

Copy link
Copy Markdown
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
@gkalpak @kara
fix(service-worker): do not blow up when caches are unwritable (#26042)
a169743 
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
@gkalpak gkalpak mentioned this pull request Sep 24, 2018
Closed
@gkalpak gkalpak mentioned this pull request Oct 6, 2018
Closed
@gkalpak gkalpak mentioned this pull request Oct 17, 2018
Closed
@patrickmichalina

patrickmichalina commented Nov 2, 2018

Copy link
Copy Markdown

Will this be merged in? It looks closed.

@kara

kara commented Nov 2, 2018

Copy link
Copy Markdown
Contributor

@patrickmichalina It is in master. See 2bd767c.

@patrickmichalina

patrickmichalina commented Nov 5, 2018

Copy link
Copy Markdown

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

@gkalpak

gkalpak commented Nov 5, 2018

Copy link
Copy Markdown
Member Author

@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

patrickmichalina commented Nov 5, 2018

Copy link
Copy Markdown

@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

angular-automatic-lock-bot Bot commented Sep 14, 2019

Copy link
Copy Markdown

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.

Reviewers

@JoostK JoostK JoostK left review comments
+1 more reviewer
@IgorMinar IgorMinar IgorMinar approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@gkalpak @mary-poppins @IgorMinar @patrickmichalina @kara @JoostK @googlebot