Skip to content

fix(service-worker): handle error with ErrorHandler#39990

Closed
chrisguttandin wants to merge 1 commit intoangular:masterfrom
chrisguttandin:handle-service-worker-error-with-error-handler
Closed

fix(service-worker): handle error with ErrorHandler#39990
chrisguttandin wants to merge 1 commit intoangular:masterfrom
chrisguttandin:handle-service-worker-error-with-error-handler

Conversation

@chrisguttandin
Copy link
Contributor

@chrisguttandin chrisguttandin commented Dec 5, 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: #39913

What is the new behavior?

Errors thrown when trying to register a Service Worker are now passed to the global ErrorHandler.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla google-cla bot added the cla: yes label Dec 5, 2020
@pullapprove pullapprove bot requested a review from IgorMinar December 5, 2020 16:39
@chrisguttandin chrisguttandin mentioned this pull request Dec 5, 2020
Closed
@gkalpak gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer area: service-worker Issues related to the @angular/service-worker package target: patch This PR is targeted for the next patch release type: bug/fix labels Dec 5, 2020
@ngbot ngbot bot modified the milestone: Backlog Dec 5, 2020
gkalpak
gkalpak suggested changes Dec 5, 2020
View reviewed changes
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 for working on this, @chrisguttandin 👍

I've left a couple of minor comments. Could you also add a test for the new behavior in service-worker/test/module_spec.ts?

Also, let's add a short description of the motivation for the change in the commit message body and also add Fixes #39913 at the bottom (per our commit message guidelines).

@chrisguttandin
Copy link
Contributor Author

chrisguttandin commented Dec 5, 2020

Hi @gkalpak, thanks for your feedback. I made the changes and updated the test. Please let me know if there is anything else I should change.

@gkalpak gkalpak closed this Dec 6, 2020
@gkalpak gkalpak reopened this Dec 6, 2020
gkalpak
gkalpak approved these changes Dec 6, 2020
View reviewed changes
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.

One super-minor nit. Otherwise lgtm (as long as CI is happy 😃)
Thx again, @chrisguttandin ✨

@gkalpak gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 6, 2020
@gkalpak gkalpak removed the request for review from IgorMinar December 6, 2020 11:30
@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Dec 6, 2020
@chrisguttandin @mhevery
fix(service-worker): handle error with ErrorHandler
552419d 
Errors thrown by calling serviceWorker.register() are now passed to the global ErrorHandler.

Fixes #39913
@mhevery mhevery closed this in 74e42cf Dec 8, 2020
mhevery pushed a commit that referenced this pull request Dec 8, 2020
@chrisguttandin @mhevery
fix(service-worker): handle error with ErrorHandler (#39990)
588dbd3 
Errors thrown by calling serviceWorker.register() are now passed to the global ErrorHandler.

Fixes #39913

PR Close #39990
@gkalpak gkalpak mentioned this pull request Jan 8, 2021
Closed
14 tasks
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Jan 8, 2021

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 Jan 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@gkalpak gkalpak gkalpak 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 target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

2 participants

@chrisguttandin @gkalpak