Repository navigation
Commit 37710b9
fix(service-worker): handle error with console.error (#40236)
This commit reverts commit [_fix(service-worker): handle error with
ErrorHandler_](552419d).
With Angular v11.0.4 and commit [_fix(service-worker): handle error with
ErrorHandler_](552419d)
Angular start to send all service worker registration errors to the Angular
standard `ErrorHandler#handleError()` interface, instead of logging them in the
console.
But users existing `ErrorHandler#handleError()` implementations are not adapted
to service worker registration errors and it might result in broken apps or
bad UI.
Passing to `ErrorHandler` is desirable for some and undesirable for others and
the same is true for passing to `console.error()`.
But `console.error()` was used for a long time and thus it is preferable to keep
it as long as a good solution is not found with `ErrorHandler`.
Right now it's hard to define a good solution for `ErrorHandler` because:
1. Given the nature of the SW registration errors (usually outside the control
of the developer, different error messages on each browser/version, often
quite generic error messages, etc.), passing them to the `ErrorHandler` is
not particularly helpful.
2. While `ErrorHandler#handleError()` accepts an argument of type `any` (so
theoretically we could pass any object without changing the public API), most
apps expect an `Error` instance, so many apps could break if we changed the
shape.
3. Ideally, the Angular community want to re-think the `ErrorHandler` API
and add support for being able to pass additional metadata for each error
(such as the source of the error or some identifier, etc.). This change,
however, could potentially affect many apps out there, so the community must
put some thought into it and design it in a way that accounts for the needs
of all packages (not just the SW).
4. Given that we want to more holistically revisit the `ErrorHandler` API, any
changes we make in the short term to address the issue just for the SW will
make it more difficult/breaky for people to move to a new API in the future.
To see the whole explanation see GitHub PR #40236.
PR Close #402361 parent 3124e42 commit 37710b9
2 files changed
Lines changed: 9 additions & 14 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -7,7 +7,7 @@ | |||
| 7 | 7 | */ | |
| 8 | 8 | ||
| 9 | 9 | import {isPlatformBrowser} from '@angular/common'; | |
| 10 | - import {APP_INITIALIZER, ApplicationRef, ErrorHandler, InjectionToken, Injector, ModuleWithProviders, NgModule, NgZone, PLATFORM_ID} from '@angular/core'; | ||
| 10 | + import {APP_INITIALIZER, ApplicationRef, InjectionToken, Injector, ModuleWithProviders, NgModule, NgZone, PLATFORM_ID} from '@angular/core'; | ||
| 11 | 11 | import {merge, Observable, of} from 'rxjs'; | |
| 12 | 12 | import {delay, filter, take} from 'rxjs/operators'; | |
| 13 | 13 | ||
@@ -128,10 +128,9 @@ export function ngswAppInitializer( | |||
| 128 | 128 | const ngZone = injector.get(NgZone); | |
| 129 | 129 | ngZone.runOutsideAngular( | |
| 130 | 130 | () => readyToRegister$.pipe(take(1)).subscribe( | |
| 131 | - () => navigator.serviceWorker.register(script, {scope: options.scope}).catch(err => { | ||
| 132 | - const errorHandler = injector.get(ErrorHandler); | ||
| 133 | - errorHandler.handleError(err); | ||
| 134 | - }))); | ||
| 131 | + () => | ||
| 132 | + navigator.serviceWorker.register(script, {scope: options.scope}) | ||
| 133 | + .catch(err => console.error('Service worker registration failed with:', err)))); | ||
| 135 | 134 | }; | |
| 136 | 135 | return initializer; | |
| 137 | 136 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -6,7 +6,7 @@ | |||
| 6 | 6 | * found in the LICENSE file at https://angular.io/license | |
| 7 | 7 | */ | |
| 8 | 8 | ||
| 9 | - import {ApplicationRef, ErrorHandler, PLATFORM_ID} from '@angular/core'; | ||
| 9 | + import {ApplicationRef, PLATFORM_ID} from '@angular/core'; | ||
| 10 | 10 | import {fakeAsync, flushMicrotasks, TestBed, tick} from '@angular/core/testing'; | |
| 11 | 11 | import {Subject} from 'rxjs'; | |
| 12 | 12 | import {filter, take} from 'rxjs/operators'; | |
@@ -21,7 +21,6 @@ describe('ServiceWorkerModule', () => { | |||
| 21 | 21 | return; | |
| 22 | 22 | } | |
| 23 | 23 | ||
| 24 | - let errorHandlerSpy: jasmine.Spy; | ||
| 25 | 24 | let swRegisterSpy: jasmine.Spy; | |
| 26 | 25 | ||
| 27 | 26 | const untilStable = () => { | |
@@ -35,14 +34,9 @@ describe('ServiceWorkerModule', () => { | |||
| 35 | 34 | ||
| 36 | 35 | describe('register()', () => { | |
| 37 | 36 | const configTestBed = async (opts: SwRegistrationOptions) => { | |
| 38 | - const errorHandler = {handleError: () => {}}; | ||
| 39 | - errorHandlerSpy = spyOn(errorHandler, 'handleError'); | ||
| 40 | 37 | TestBed.configureTestingModule({ | |
| 41 | 38 | imports: [ServiceWorkerModule.register('sw.js', opts)], | |
| 42 | - providers: [ | ||
| 43 | - {provide: ErrorHandler, useValue: errorHandler}, | ||
| 44 | - {provide: PLATFORM_ID, useValue: 'browser'}, | ||
| 45 | - ], | ||
| 39 | + providers: [{provide: PLATFORM_ID, useValue: 'browser'}], | ||
| 46 | 40 | }); | |
| 47 | 41 | ||
| 48 | 42 | await untilStable(); | |
@@ -77,10 +71,12 @@ describe('ServiceWorkerModule', () => { | |||
| 77 | 71 | }); | |
| 78 | 72 | ||
| 79 | 73 | it('catches and a logs registration errors', async () => { | |
| 74 | + const consoleErrorSpy = spyOn(console, 'error'); | ||
| 80 | 75 | swRegisterSpy.and.returnValue(Promise.reject('no reason')); | |
| 81 | 76 | ||
| 82 | 77 | await configTestBed({enabled: true, scope: 'foo'}); | |
| 83 | - expect(errorHandlerSpy).toHaveBeenCalledWith('no reason'); | ||
| 78 | + expect(consoleErrorSpy) | ||
| 79 | + .toHaveBeenCalledWith('Service worker registration failed with:', 'no reason'); | ||
| 84 | 80 | }); | |
| 85 | 81 | }); | |
| 86 | 82 | ||
0 commit comments