From b9a91a5e7490d499dcc4c698b4201962a01aff77 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 30 Nov 2017 08:22:41 -0800 Subject: [PATCH] fix(service-worker): don't crash if SW not supported Currently a bug exists where attempting to inject SwPush crashes the application if Service Workers are unsupported. This happens because SwPush doesn't properly detect that navigator.serviceWorker isn't set. This change ensures that all passive observation of SwPush and SwUpdate doesn't cause crashes, and that calling methods to perform actions on them results in rejected Promises. It's up to applications to detect when those services are not available, and refrain from attempting to use them. To that end, this change also adds an `isSupported` getter to both services, so users don't have to rely on feature detection directly with browser APIs. Currently this simply detects whether the SW API is present, but in the future it will be expanded to detect whether a particular browser supports specific APIs (such as push notifications, for example). --- packages/service-worker/rollup.config.js | 1 + packages/service-worker/src/low_level.ts | 8 +++-- packages/service-worker/src/push.ts | 23 ++++++++++-- packages/service-worker/src/update.ts | 20 ++++++++++- packages/service-worker/test/comm_spec.ts | 35 +++++++++++++++++++ .../service-worker/service-worker.d.ts | 2 ++ 6 files changed, 82 insertions(+), 7 deletions(-) diff --git a/packages/service-worker/rollup.config.js b/packages/service-worker/rollup.config.js index 00d9ea89e0..6242de44c3 100644 --- a/packages/service-worker/rollup.config.js +++ b/packages/service-worker/rollup.config.js @@ -21,6 +21,7 @@ const globals = { 'rxjs/observable/defer': 'Rx.Observable', 'rxjs/observable/fromEvent': 'Rx.Observable', 'rxjs/observable/merge': 'Rx.Observable', + 'rxjs/observable/never': 'Rx.Observable', 'rxjs/observable/of': 'Rx.Observable', 'rxjs/observable/throw': 'Rx.Observable', diff --git a/packages/service-worker/src/low_level.ts b/packages/service-worker/src/low_level.ts index 1632ca6bbd..6d46b7264a 100644 --- a/packages/service-worker/src/low_level.ts +++ b/packages/service-worker/src/low_level.ts @@ -24,7 +24,7 @@ import {switchMap as op_switchMap} from 'rxjs/operator/switchMap'; import {take as op_take} from 'rxjs/operator/take'; import {toPromise as op_toPromise} from 'rxjs/operator/toPromise'; -const ERR_SW_NOT_SUPPORTED = 'Service workers are not supported by this browser'; +export const ERR_SW_NOT_SUPPORTED = 'Service workers are disabled or not supported by this browser'; export interface Version { hash: string; @@ -86,9 +86,9 @@ export class NgswCommChannel { */ readonly events: Observable; - constructor(serviceWorker: ServiceWorkerContainer|undefined) { + constructor(private serviceWorker: ServiceWorkerContainer|undefined) { if (!serviceWorker) { - this.worker = this.events = errorObservable(ERR_SW_NOT_SUPPORTED); + this.worker = this.events = this.registration = errorObservable(ERR_SW_NOT_SUPPORTED); } else { const controllerChangeEvents = >(obs_fromEvent(serviceWorker, 'controllerchange')); @@ -176,4 +176,6 @@ export class NgswCommChannel { })); return op_toPromise.call(mapErrorAndValue); } + + get isEnabled(): boolean { return !!this.serviceWorker; } } \ No newline at end of file diff --git a/packages/service-worker/src/push.ts b/packages/service-worker/src/push.ts index 845a886207..7300376433 100644 --- a/packages/service-worker/src/push.ts +++ b/packages/service-worker/src/push.ts @@ -9,15 +9,15 @@ import {Injectable} from '@angular/core'; import {Observable} from 'rxjs/Observable'; import {Subject} from 'rxjs/Subject'; - import {merge as obs_merge} from 'rxjs/observable/merge'; - +import {never as obs_never} from 'rxjs/observable/never'; import {map as op_map} from 'rxjs/operator/map'; import {switchMap as op_switchMap} from 'rxjs/operator/switchMap'; import {take as op_take} from 'rxjs/operator/take'; import {toPromise as op_toPromise} from 'rxjs/operator/toPromise'; -import {NgswCommChannel} from './low_level'; +import {ERR_SW_NOT_SUPPORTED, NgswCommChannel} from './low_level'; + /** * Subscribe and listen to push notifications from the Service Worker. @@ -34,6 +34,11 @@ export class SwPush { new Subject(); constructor(private sw: NgswCommChannel) { + if (!sw.isEnabled) { + this.messages = obs_never(); + this.subscription = obs_never(); + return; + } this.messages = op_map.call(this.sw.eventsOfType('PUSH'), (message: {data: object}) => message.data); @@ -46,7 +51,16 @@ export class SwPush { this.subscription = obs_merge(workerDrivenSubscriptions, this.subscriptionChanges); } + /** + * Returns true if the Service Worker is enabled (supported by the browser and enabled via + * ServiceWorkerModule). + */ + get isEnabled(): boolean { return this.sw.isEnabled; } + requestSubscription(options: {serverPublicKey: string}): Promise { + if (!this.sw.isEnabled) { + return Promise.reject(new Error(ERR_SW_NOT_SUPPORTED)); + } const pushOptions: PushSubscriptionOptionsInit = {userVisibleOnly: true}; let key = atob(options.serverPublicKey.replace(/_/g, '/').replace(/-/g, '+')); let applicationServerKey = new Uint8Array(new ArrayBuffer(key.length)); @@ -64,6 +78,9 @@ export class SwPush { } unsubscribe(): Promise { + if (!this.sw.isEnabled) { + return Promise.reject(new Error(ERR_SW_NOT_SUPPORTED)); + } const unsubscribe = op_switchMap.call(this.subscription, (sub: PushSubscription | null) => { if (sub !== null) { return sub.unsubscribe().then(success => { diff --git a/packages/service-worker/src/update.ts b/packages/service-worker/src/update.ts index 349f526fe8..d2f1a80e84 100644 --- a/packages/service-worker/src/update.ts +++ b/packages/service-worker/src/update.ts @@ -9,9 +9,10 @@ import {Injectable} from '@angular/core'; import {Observable} from 'rxjs/Observable'; import {defer as obs_defer} from 'rxjs/observable/defer'; +import {never as obs_never} from 'rxjs/observable/never'; import {map as op_map} from 'rxjs/operator/map'; -import {NgswCommChannel, UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; +import {ERR_SW_NOT_SUPPORTED, NgswCommChannel, UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; /** @@ -26,16 +27,33 @@ export class SwUpdate { readonly activated: Observable; constructor(private sw: NgswCommChannel) { + if (!sw.isEnabled) { + this.available = obs_never(); + this.activated = obs_never(); + return; + } this.available = this.sw.eventsOfType('UPDATE_AVAILABLE'); this.activated = this.sw.eventsOfType('UPDATE_ACTIVATED'); } + /** + * Returns true if the Service Worker is enabled (supported by the browser and enabled via + * ServiceWorkerModule). + */ + get isEnabled(): boolean { return this.sw.isEnabled; } + checkForUpdate(): Promise { + if (!this.sw.isEnabled) { + return Promise.reject(new Error(ERR_SW_NOT_SUPPORTED)); + } const statusNonce = this.sw.generateNonce(); return this.sw.postMessageWithStatus('CHECK_FOR_UPDATES', {statusNonce}, statusNonce); } activateUpdate(): Promise { + if (!this.sw.isEnabled) { + return Promise.reject(new Error(ERR_SW_NOT_SUPPORTED)); + } const statusNonce = this.sw.generateNonce(); return this.sw.postMessageWithStatus('ACTIVATE_UPDATE', {statusNonce}, statusNonce); } diff --git a/packages/service-worker/test/comm_spec.ts b/packages/service-worker/test/comm_spec.ts index c1c0d75c80..9788c9b1a7 100644 --- a/packages/service-worker/test/comm_spec.ts +++ b/packages/service-worker/test/comm_spec.ts @@ -71,6 +71,24 @@ export function main() { }); expect(() => TestBed.get(SwPush)).not.toThrow(); }); + describe('with no SW', () => { + beforeEach(() => { comm = new NgswCommChannel(undefined); }); + it('can be instantiated', () => { push = new SwPush(comm); }); + it('does not crash on subscription to observables', () => { + push = new SwPush(comm); + push.messages.toPromise().catch(err => fail(err)); + push.subscription.toPromise().catch(err => fail(err)); + }); + it('gives an error when registering', done => { + push = new SwPush(comm); + push.requestSubscription({serverPublicKey: 'test'}).catch(err => { done(); }); + }); + it('gives an error when unsubscribing', done => { + + push = new SwPush(comm); + push.unsubscribe().catch(err => { done(); }); + }); + }); }); describe('SwUpdate', () => { let update: SwUpdate; @@ -147,6 +165,23 @@ export function main() { }); expect(() => TestBed.get(SwUpdate)).not.toThrow(); }); + describe('with no SW', () => { + beforeEach(() => { comm = new NgswCommChannel(undefined); }); + it('can be instantiated', () => { update = new SwUpdate(comm); }); + it('does not crash on subscription to observables', () => { + update = new SwUpdate(comm); + update.available.toPromise().catch(err => fail(err)); + update.activated.toPromise().catch(err => fail(err)); + }); + it('gives an error when checking for updates', done => { + update = new SwUpdate(comm); + update.checkForUpdate().catch(err => { done(); }); + }); + it('gives an error when activating updates', done => { + update = new SwUpdate(comm); + update.activateUpdate().catch(err => { done(); }); + }); + }); }); }); } diff --git a/tools/public_api_guard/service-worker/service-worker.d.ts b/tools/public_api_guard/service-worker/service-worker.d.ts index c70daacd72..e05787c01a 100644 --- a/tools/public_api_guard/service-worker/service-worker.d.ts +++ b/tools/public_api_guard/service-worker/service-worker.d.ts @@ -8,6 +8,7 @@ export declare class ServiceWorkerModule { /** @experimental */ export declare class SwPush { + readonly isEnabled: boolean; readonly messages: Observable; readonly subscription: Observable; constructor(sw: NgswCommChannel); @@ -21,6 +22,7 @@ export declare class SwPush { export declare class SwUpdate { readonly activated: Observable; readonly available: Observable; + readonly isEnabled: boolean; constructor(sw: NgswCommChannel); activateUpdate(): Promise; checkForUpdate(): Promise;