Skip to content

fix(service-worker): add UNRECOVERABLE_STATE handler #36847

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

Conversation

sonukapoor
Copy link
Contributor

@sonukapoor sonukapoor commented Apr 29, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #36539

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@sonukapoor sonukapoor force-pushed the service-worker-invalid-resource branch 2 times, most recently from 1d2fa37 to 6f833e7 Compare April 29, 2020 02:23
@sonukapoor sonukapoor changed the title Service worker invalid resource fix(service-worker): add RESOURCE_REMOVED handler Apr 29, 2020
@AndrewKushnir AndrewKushnir added the area: service-worker Issues related to the @angular/service-worker package label Apr 29, 2020
@ngbot ngbot bot added this to the needsTriage milestone Apr 29, 2020
@sonukapoor sonukapoor force-pushed the service-worker-invalid-resource branch 3 times, most recently from a9d8a4b to 9d1f214 Compare April 30, 2020 10:56
@sonukapoor sonukapoor force-pushed the service-worker-invalid-resource branch 6 times, most recently from 0396e7a to fd1ab83 Compare May 15, 2020 20:34
@sonukapoor sonukapoor changed the title fix(service-worker): add RESOURCE_REMOVED handler fix(service-worker): add UNRECOVERABLE_STATE handler May 15, 2020
@sonukapoor sonukapoor force-pushed the service-worker-invalid-resource branch from fd1ab83 to 9843c4f Compare May 15, 2020 21:33
@IgorMinar IgorMinar requested review from IgorMinar and gkalpak June 3, 2020 21:01
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.

I like where this is heading, but I suggest that we use terminology different from unrecovered because it gets awkward and confusing to use it. How about using defunkt, terminalError, or something of that sort?

@gkalpak
Copy link
Member

gkalpak commented Jun 16, 2020

@IgorMinar, I was the one that proposed "unrecoverable"/"unrecoverableState" (I assume "unrecovered" is a typo). I find "unrecoverable" more descriptive than "defunkt" or "terminalError" (but not a hill I'm going to die on 😁).

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Thx, @sonukapoor! I've suggested some changes. LMK what you think.

Note to self: We need to document this in the appropriate guides (with code examples).

@sonukapoor
Copy link
Contributor Author

@gkalpak Please review again. I have made some of the suggested changes. I will address unit tests once we are OK with the changes.

@sonukapoor sonukapoor force-pushed the service-worker-invalid-resource branch from 01a67a7 to ab76f91 Compare August 31, 2020 10:13
@mary-poppins
Copy link

You can preview ab76f91 at https://pr36847-ab76f91.ngbuilds.io/.

Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@sonukapoor sonukapoor 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 Aug 31, 2020
@gkalpak gkalpak added target: minor This PR is targeted for the next minor release and removed target: major This PR is targeted for the next major release labels Aug 31, 2020
josephperrott pushed a commit that referenced this pull request Aug 31, 2020
#36847)

Previously, the condition to make the cache busted was executing although
the network request was successful. However, this is not valid. The cache
should only be marked as busted when the request failed. This commit fixes
the invalid condition.

PR Close #36847
josephperrott pushed a commit that referenced this pull request Aug 31, 2020
In several occasions it has been observed when the browser has evicted
eagerly cached assets from the cache and which can also not be found on the
server anymore. This can lead to broken state where only parts of the application
will load and others will fail.

This commit fixes this issue by checking for the missing asset in the cache
and on the server. If this condition is true, the broken client will be
notified about the current state through the `UnrecoverableStateError`.

Closes #36539

PR Close #36847
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
…gular#36847)

This commit adds a helper method to remove individual cached items.

PR Close angular#36847
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
angular#36847)

Previously, the condition to make the cache busted was executing although
the network request was successful. However, this is not valid. The cache
should only be marked as busted when the request failed. This commit fixes
the invalid condition.

PR Close angular#36847
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
In several occasions it has been observed when the browser has evicted
eagerly cached assets from the cache and which can also not be found on the
server anymore. This can lead to broken state where only parts of the application
will load and others will fail.

This commit fixes this issue by checking for the missing asset in the cache
and on the server. If this condition is true, the broken client will be
notified about the current state through the `UnrecoverableStateError`.

Closes angular#36539

PR Close angular#36847
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
@govi2010
Copy link

How can I get this fix?
I am following this PR which is closed.
I am using PWA with angular 9.
How can I get this Fix ?

@sonukapoor sonukapoor deleted the service-worker-invalid-resource branch September 14, 2020 10:14
@gkalpak
Copy link
Member

gkalpak commented Sep 14, 2020

@govi2010, this feature was introduced in v11.0.0-next.0. The first stable version that will have it is 11.0.0.

@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 Oct 15, 2020
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 feature Issue that requests a new feature target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants