Skip to content

Commit

Permalink
fix(service-worker): don't crash if SW not supported
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
alxhub committed Dec 1, 2017
1 parent 65f4fad commit b9a91a5
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 7 deletions.
1 change: 1 addition & 0 deletions packages/service-worker/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',

Expand Down
8 changes: 5 additions & 3 deletions packages/service-worker/src/low_level.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -86,9 +86,9 @@ export class NgswCommChannel {
*/
readonly events: Observable<IncomingEvent>;

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 =
<Observable<any>>(obs_fromEvent(serviceWorker, 'controllerchange'));
Expand Down Expand Up @@ -176,4 +176,6 @@ export class NgswCommChannel {
}));
return op_toPromise.call(mapErrorAndValue);
}

get isEnabled(): boolean { return !!this.serviceWorker; }
}
23 changes: 20 additions & 3 deletions packages/service-worker/src/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -34,6 +34,11 @@ export class SwPush {
new Subject<PushSubscription|null>();

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);

Expand All @@ -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<PushSubscription> {
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));
Expand All @@ -64,6 +78,9 @@ export class SwPush {
}

unsubscribe(): Promise<void> {
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 => {
Expand Down
20 changes: 19 additions & 1 deletion packages/service-worker/src/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';


/**
Expand All @@ -26,16 +27,33 @@ export class SwUpdate {
readonly activated: Observable<UpdateActivatedEvent>;

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<void> {
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<void> {
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);
}
Expand Down
35 changes: 35 additions & 0 deletions packages/service-worker/test/comm_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(); });
});
});
});
});
}
2 changes: 2 additions & 0 deletions tools/public_api_guard/service-worker/service-worker.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export declare class ServiceWorkerModule {

/** @experimental */
export declare class SwPush {
readonly isEnabled: boolean;
readonly messages: Observable<object>;
readonly subscription: Observable<PushSubscription | null>;
constructor(sw: NgswCommChannel);
Expand All @@ -21,6 +22,7 @@ export declare class SwPush {
export declare class SwUpdate {
readonly activated: Observable<UpdateActivatedEvent>;
readonly available: Observable<UpdateAvailableEvent>;
readonly isEnabled: boolean;
constructor(sw: NgswCommChannel);
activateUpdate(): Promise<void>;
checkForUpdate(): Promise<void>;
Expand Down

0 comments on commit b9a91a5

Please sign in to comment.