From 036a2faf02e4dafad80a777f32d6b7c2303139c3 Mon Sep 17 00:00:00 2001 From: Sonu Kapoor Date: Tue, 28 Apr 2020 07:45:18 -0400 Subject: [PATCH] feat(service-worker): add `UnrecoverableStateError` (#36847) 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 --- .../service-worker/service-worker.d.ts | 6 + packages/service-worker/src/index.ts | 2 +- packages/service-worker/src/low_level.ts | 18 +- packages/service-worker/src/update.ts | 11 +- packages/service-worker/test/comm_spec.ts | 9 + packages/service-worker/worker/src/assets.ts | 17 +- packages/service-worker/worker/src/driver.ts | 23 +++ packages/service-worker/worker/src/error.ts | 4 + .../service-worker/worker/test/happy_spec.ts | 163 ++++++++++++++++++ 9 files changed, 245 insertions(+), 8 deletions(-) diff --git a/goldens/public-api/service-worker/service-worker.d.ts b/goldens/public-api/service-worker/service-worker.d.ts index 097cbe2339..946a370fdd 100644 --- a/goldens/public-api/service-worker/service-worker.d.ts +++ b/goldens/public-api/service-worker/service-worker.d.ts @@ -29,11 +29,17 @@ export declare class SwUpdate { readonly activated: Observable; readonly available: Observable; get isEnabled(): boolean; + readonly unrecoverable: Observable; constructor(sw: ɵangular_packages_service_worker_service_worker_a); activateUpdate(): Promise; checkForUpdate(): Promise; } +export declare interface UnrecoverableStateEvent { + reason: string; + type: 'UNRECOVERABLE_STATE'; +} + export declare interface UpdateActivatedEvent { current: { hash: string; diff --git a/packages/service-worker/src/index.ts b/packages/service-worker/src/index.ts index 24b9a0ad6d..38a540dc2b 100644 --- a/packages/service-worker/src/index.ts +++ b/packages/service-worker/src/index.ts @@ -14,7 +14,7 @@ * found in the LICENSE file at https://angular.io/license */ -export {UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; +export {UnrecoverableStateEvent, UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; export {ServiceWorkerModule, SwRegistrationOptions} from './module'; export {SwPush} from './push'; export {SwUpdate} from './update'; diff --git a/packages/service-worker/src/low_level.ts b/packages/service-worker/src/low_level.ts index 00c5e7cf0b..220e631cd6 100644 --- a/packages/service-worker/src/low_level.ts +++ b/packages/service-worker/src/low_level.ts @@ -33,6 +33,22 @@ export interface UpdateActivatedEvent { current: {hash: string, appData?: Object}; } +/** + * An event emitted when the version of the app used by the service worker to serve this client is + * in a broken state that cannot be recovered from and a full page reload is required. + * + * For example, the service worker may not be able to retrieve a required resource, neither from the + * cache nor from the server. This could happen if a new version is deployed to the server and the + * service worker cache has been partially cleaned by the browser, removing some files of a previous + * app version but not all. + * + * @publicApi + */ +export interface UnrecoverableStateEvent { + type: 'UNRECOVERABLE_STATE'; + reason: string; +} + /** * An event emitted when a `PushEvent` is received by the service worker. */ @@ -41,7 +57,7 @@ export interface PushEvent { data: any; } -export type IncomingEvent = UpdateAvailableEvent|UpdateActivatedEvent; +export type IncomingEvent = UpdateAvailableEvent|UpdateActivatedEvent|UnrecoverableStateEvent; export interface TypedEvent { type: string; diff --git a/packages/service-worker/src/update.ts b/packages/service-worker/src/update.ts index bfd070e544..f9a8ad802a 100644 --- a/packages/service-worker/src/update.ts +++ b/packages/service-worker/src/update.ts @@ -9,7 +9,7 @@ import {Injectable} from '@angular/core'; import {NEVER, Observable} from 'rxjs'; -import {ERR_SW_NOT_SUPPORTED, NgswCommChannel, UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; +import {ERR_SW_NOT_SUPPORTED, NgswCommChannel, UnrecoverableStateEvent, UpdateActivatedEvent, UpdateAvailableEvent} from './low_level'; @@ -31,6 +31,13 @@ export class SwUpdate { */ readonly activated: Observable; + /** + * Emits an `UnrecoverableStateEvent` event whenever the version of the app used by the service + * worker to serve this client is in a broken state that cannot be recovered from without a full + * page reload. + */ + readonly unrecoverable: Observable; + /** * True if the Service Worker is enabled (supported by the browser and enabled via * `ServiceWorkerModule`). @@ -43,10 +50,12 @@ export class SwUpdate { if (!sw.isEnabled) { this.available = NEVER; this.activated = NEVER; + this.unrecoverable = NEVER; return; } this.available = this.sw.eventsOfType('UPDATE_AVAILABLE'); this.activated = this.sw.eventsOfType('UPDATE_ACTIVATED'); + this.unrecoverable = this.sw.eventsOfType('UNRECOVERABLE_STATE'); } checkForUpdate(): Promise { diff --git a/packages/service-worker/test/comm_spec.ts b/packages/service-worker/test/comm_spec.ts index cc4d69a5e8..efbc3d55e0 100644 --- a/packages/service-worker/test/comm_spec.ts +++ b/packages/service-worker/test/comm_spec.ts @@ -435,6 +435,14 @@ import {MockPushManager, MockPushSubscription, MockServiceWorkerContainer, MockS }, }); }); + it('processes unrecoverable notifications when sent', done => { + update.unrecoverable.subscribe(event => { + expect(event.reason).toEqual('Invalid Resource'); + expect(event.type).toEqual('UNRECOVERABLE_STATE'); + done(); + }); + mock.sendMessage({type: 'UNRECOVERABLE_STATE', reason: 'Invalid Resource'}); + }); it('processes update activation notifications when sent', done => { update.activated.subscribe(event => { expect(event.previous).toEqual({hash: 'A'}); @@ -500,6 +508,7 @@ import {MockPushManager, MockPushSubscription, MockServiceWorkerContainer, MockS update = new SwUpdate(comm); update.available.toPromise().catch(err => fail(err)); update.activated.toPromise().catch(err => fail(err)); + update.unrecoverable.toPromise().catch(err => fail(err)); }); it('gives an error when checking for updates', done => { update = new SwUpdate(comm); diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index bfb8a66413..c2f4222bdd 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -9,7 +9,7 @@ import {Adapter, Context} from './adapter'; import {CacheState, NormalizedUrl, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {Database, Table} from './database'; -import {errorToString, SwCriticalError} from './error'; +import {errorToString, SwCriticalError, SwUnrecoverableStateError} from './error'; import {IdleScheduler} from './idle'; import {AssetGroupConfig} from './manifest'; import {sha1Binary} from './sha1'; @@ -145,6 +145,7 @@ export abstract class AssetGroup { return cachedResponse; } } + // No already-cached response exists, so attempt a fetch/cache operation. The original request // may specify things like credential inclusion, but for assets these are not honored in order // to avoid issues with opaque responses. The SW requests the data itself. @@ -414,10 +415,16 @@ export abstract class AssetGroup { // If the response was unsuccessful, there's nothing more that can be done. if (!cacheBustedResult.ok) { - throw new SwCriticalError( - `Response not Ok (cacheBustedFetchFromNetwork): cache busted request for ${ - req.url} returned response ${cacheBustedResult.status} ${ - cacheBustedResult.statusText}`); + if (cacheBustedResult.status === 404) { + throw new SwUnrecoverableStateError( + `Failed to retrieve hashed resource from the server. (AssetGroup: ${ + this.config.name} | URL: ${url})`); + } else { + throw new SwCriticalError( + `Response not Ok (cacheBustedFetchFromNetwork): cache busted request for ${ + req.url} returned response ${cacheBustedResult.status} ${ + cacheBustedResult.statusText}`); + } } // Hash the contents. diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index c0875e0c66..006cb2cce5 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -436,6 +436,9 @@ export class Driver implements Debuggable, UpdateSource { // network. res = await appVersion.handleFetch(event.request, event); } catch (err) { + if (err.isUnrecoverableState) { + await this.notifyClientsAboutUnrecoverableState(appVersion, err.message); + } if (err.isCritical) { // Something went wrong with the activation of this version. await this.versionFailed(appVersion, err); @@ -1009,6 +1012,26 @@ export class Driver implements Debuggable, UpdateSource { }; } + async notifyClientsAboutUnrecoverableState(appVersion: AppVersion, reason: string): + Promise { + const broken = + Array.from(this.versions.entries()).find(([hash, version]) => version === appVersion); + if (broken === undefined) { + // This version is no longer in use anyway, so nobody cares. + return; + } + + const brokenHash = broken[0]; + const affectedClients = Array.from(this.clientVersionMap.entries()) + .filter(([clientId, hash]) => hash === brokenHash) + .map(([clientId]) => clientId); + + affectedClients.forEach(async clientId => { + const client = await this.scope.clients.get(clientId); + client.postMessage({type: 'UNRECOVERABLE_STATE', reason}); + }); + } + async notifyClientsAboutUpdate(next: AppVersion): Promise { await this.initialized; diff --git a/packages/service-worker/worker/src/error.ts b/packages/service-worker/worker/src/error.ts index dc6f10cff5..3bb7ac4276 100644 --- a/packages/service-worker/worker/src/error.ts +++ b/packages/service-worker/worker/src/error.ts @@ -17,3 +17,7 @@ export function errorToString(error: any): string { return `${error}`; } } + +export class SwUnrecoverableStateError extends SwCriticalError { + readonly isUnrecoverableState: boolean = true; +} diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index c54b04ea95..e4cdaaa320 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -1738,6 +1738,169 @@ describe('Driver', () => { expect(requestUrls2).toContain(httpsRequestUrl); }); + describe('unrecoverable state', () => { + const generateMockServerState = (fileSystem: MockFileSystem) => { + const manifest: Manifest = { + configVersion: 1, + timestamp: 1234567890123, + index: '/index.html', + assetGroups: [{ + name: 'assets', + installMode: 'prefetch', + updateMode: 'prefetch', + urls: fileSystem.list(), + patterns: [], + cacheQueryOptions: {ignoreVary: true}, + }], + dataGroups: [], + navigationUrls: processNavigationUrls(''), + hashTable: tmpHashTableForFs(fileSystem), + }; + + return { + serverState: new MockServerStateBuilder() + .withManifest(manifest) + .withStaticFiles(fileSystem) + .build(), + manifest, + }; + }; + + it('notifies affected clients', async () => { + const {serverState: serverState1} = generateMockServerState( + new MockFileSystemBuilder() + .addFile('/index.html', '') + .addFile('/foo.hash.js', 'console.log("FOO");') + .build()); + + const {serverState: serverState2, manifest: manifest2} = generateMockServerState( + new MockFileSystemBuilder() + .addFile('/index.html', '') + .addFile('/bar.hash.js', 'console.log("BAR");') + .build()); + + const {serverState: serverState3} = generateMockServerState( + new MockFileSystemBuilder() + .addFile('/index.html', '') + .addFile('/baz.hash.js', 'console.log("BAZ");') + .build()); + + // Create initial server state and initialize the SW. + scope = new SwTestHarnessBuilder().withServerState(serverState1).build(); + driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); + + // Verify that all three clients are able to make the request. + expect(await makeRequest(scope, '/foo.hash.js', 'client1')).toBe('console.log("FOO");'); + expect(await makeRequest(scope, '/foo.hash.js', 'client2')).toBe('console.log("FOO");'); + expect(await makeRequest(scope, '/foo.hash.js', 'client3')).toBe('console.log("FOO");'); + + await driver.initialized; + serverState1.clearRequests(); + + // Verify that the `foo.hash.js` file is cached. + expect(await makeRequest(scope, '/foo.hash.js')).toBe('console.log("FOO");'); + serverState1.assertNoRequestFor('/foo.hash.js'); + + // Update the ServiceWorker to the second version. + scope.updateServerState(serverState2); + expect(await driver.checkForUpdate()).toEqual(true); + + // Update the first two clients to the latest version, keep `client3` as is. + const [client1, client2] = + await Promise.all([scope.clients.get('client1'), scope.clients.get('client2')]); + + await Promise.all([driver.updateClient(client1), driver.updateClient(client2)]); + + // Update the ServiceWorker to the latest version + scope.updateServerState(serverState3); + expect(await driver.checkForUpdate()).toEqual(true); + + // Remove `bar.hash.js` from the cache to emulate the browser evicting files from the cache. + await removeAssetFromCache(scope, manifest2, '/bar.hash.js'); + + // Get all clients and verify their messages + const mockClient1 = scope.clients.getMock('client1')!; + const mockClient2 = scope.clients.getMock('client2')!; + const mockClient3 = scope.clients.getMock('client3')!; + + // Try to retrieve `bar.hash.js`, which is neither in the cache nor on the server. + // This should put the SW in an unrecoverable state and notify clients. + expect(await makeRequest(scope, '/bar.hash.js', 'client1')).toBeNull(); + serverState2.assertSawRequestFor('/bar.hash.js'); + const unrecoverableMessage = { + type: 'UNRECOVERABLE_STATE', + reason: + 'Failed to retrieve hashed resource from the server. (AssetGroup: assets | URL: /bar.hash.js)' + }; + + expect(mockClient1.messages).toContain(unrecoverableMessage); + expect(mockClient2.messages).toContain(unrecoverableMessage); + expect(mockClient3.messages).not.toContain(unrecoverableMessage); + + // Because `client1` failed, `client1` and `client2` have been moved to the latest version. + // Verify that by retrieving `baz.hash.js`. + expect(await makeRequest(scope, '/baz.hash.js', 'client1')).toBe('console.log("BAZ");'); + serverState2.assertNoRequestFor('/baz.hash.js'); + expect(await makeRequest(scope, '/baz.hash.js', 'client2')).toBe('console.log("BAZ");'); + serverState2.assertNoRequestFor('/baz.hash.js'); + + // Ensure that `client3` remains on the first version and can request `foo.hash.js`. + expect(await makeRequest(scope, '/foo.hash.js', 'client3')).toBe('console.log("FOO");'); + serverState2.assertNoRequestFor('/foo.hash.js'); + }); + + it('enters degraded mode', async () => { + const originalFiles = new MockFileSystemBuilder() + .addFile('/index.html', '') + .addFile('/foo.hash.js', 'console.log("FOO");') + .build(); + + const updatedFiles = new MockFileSystemBuilder() + .addFile('/index.html', '') + .addFile('/bar.hash.js', 'console.log("BAR");') + .build(); + + const {serverState: originalServer, manifest} = generateMockServerState(originalFiles); + const {serverState: updatedServer} = generateMockServerState(updatedFiles); + + // Create initial server state and initialize the SW. + scope = new SwTestHarnessBuilder().withServerState(originalServer).build(); + driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); + + expect(await makeRequest(scope, '/foo.hash.js')).toBe('console.log("FOO");'); + await driver.initialized; + originalServer.clearRequests(); + + // Verify that the `foo.hash.js` file is cached. + expect(await makeRequest(scope, '/foo.hash.js')).toBe('console.log("FOO");'); + originalServer.assertNoRequestFor('/foo.hash.js'); + + // Update the server state to emulate deploying a new version (where `foo.hash.js` does not + // exist any more). Keep the cache though. + scope = new SwTestHarnessBuilder() + .withCacheState(scope.caches.dehydrate()) + .withServerState(updatedServer) + .build(); + driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); + + // The SW is still able to serve `foo.hash.js` from the cache. + expect(await makeRequest(scope, '/foo.hash.js')).toBe('console.log("FOO");'); + updatedServer.assertNoRequestFor('/foo.hash.js'); + + // Remove `foo.hash.js` from the cache to emulate the browser evicting files from the cache. + await removeAssetFromCache(scope, manifest, '/foo.hash.js'); + + // Try to retrieve `foo.hash.js`, which is neither in the cache nor on the server. + // This should put the SW in an unrecoverable state and notify clients. + expect(await makeRequest(scope, '/foo.hash.js')).toBeNull(); + updatedServer.assertSawRequestFor('/foo.hash.js'); + + // This should also enter the `SW` into degraded mode, because the broken version was the + // latest one. + expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY); + }); + }); + describe('backwards compatibility with v5', () => { beforeEach(() => { const serverV5 = new MockServerStateBuilder()