diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index a7513bc886826..3c44568657bd6 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -742,11 +742,16 @@ export class Driver implements Debuggable, UpdateSource { // This particular AppVersion is broken. First, find the manifest hash. 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); // TODO: notify affected apps. @@ -759,17 +764,12 @@ export class Driver implements Debuggable, UpdateSource { this.state = DriverReadyState.EXISTING_CLIENTS_ONLY; this.stateMessage = `Degraded due to: ${errorToString(err)}`; - // Cancel the binding for these clients. - Array.from(this.clientVersionMap.keys()) - .forEach(clientId => this.clientVersionMap.delete(clientId)); + // Cancel the binding for the affected clients. + affectedClients.forEach(clientId => this.clientVersionMap.delete(clientId)); } else { - // The current version is viable, but this older version isn't. The only + // The latest version is viable, but this older version isn't. The only // possible remedy is to stop serving the older version and go to the network. - // Figure out which clients are affected and put them on the latest. - const affectedClients = - Array.from(this.clientVersionMap.keys()) - .filter(clientId => this.clientVersionMap.get(clientId) ! === brokenHash); - // Push the affected clients onto the latest version. + // Put the affected clients on the latest version. affectedClients.forEach(clientId => this.clientVersionMap.set(clientId, this.latestHash !)); } diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index e28777e3bde1e..c9d550ac20472 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -52,7 +52,10 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; .addUnhashedFile('/ignored/dir/file2', 'this is not handled by the SW either') .build(); - const brokenFs = new MockFileSystemBuilder().addFile('/foo.txt', 'this is foo').build(); + const brokenFs = new MockFileSystemBuilder() + .addFile('/foo.txt', 'this is foo (broken)') + .addFile('/bar.txt', 'this is bar (broken)') + .build(); const brokenManifest: Manifest = { configVersion: 1, @@ -72,6 +75,35 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; hashTable: tmpHashTableForFs(brokenFs, {'/foo.txt': true}), }; + const brokenLazyManifest: Manifest = { + configVersion: 1, + timestamp: 1234567890123, + index: '/foo.txt', + assetGroups: [ + { + name: 'assets', + installMode: 'prefetch', + updateMode: 'prefetch', + urls: [ + '/foo.txt', + ], + patterns: [], + }, + { + name: 'lazy-assets', + installMode: 'lazy', + updateMode: 'lazy', + urls: [ + '/bar.txt', + ], + patterns: [], + }, + ], + dataGroups: [], + navigationUrls: processNavigationUrls(''), + hashTable: tmpHashTableForFs(brokenFs, {'/bar.txt': true}), + }; + // Manifest without navigation urls to test backward compatibility with // versions < 6.0.0. interface ManifestV5 { @@ -226,6 +258,11 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; const brokenServer = new MockServerStateBuilder().withStaticFiles(brokenFs).withManifest(brokenManifest).build(); + const brokenLazyServer = new MockServerStateBuilder() + .withStaticFiles(brokenFs) + .withManifest(brokenLazyManifest) + .build(); + const server404 = new MockServerStateBuilder().withStaticFiles(dist).build(); const manifestHash = sha1(JSON.stringify(manifest)); @@ -1150,7 +1187,7 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; (scope.registration as any).scope = 'http://site.com'; driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); - expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo (broken)'); }); it('enters degraded mode when update has a bad index', async() => { @@ -1193,6 +1230,45 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; server.assertSawRequestFor('/foo.txt'); }); + it('enters degraded mode when something goes wrong with the latest version', async() => { + await driver.initialized; + + // Two clients on initial version. + expect(await makeRequest(scope, '/foo.txt', 'client1')).toBe('this is foo'); + expect(await makeRequest(scope, '/foo.txt', 'client2')).toBe('this is foo'); + + // Install a broken version (`bar.txt` has invalid hash). + scope.updateServerState(brokenLazyServer); + await driver.checkForUpdate(); + + // Update `client1` but not `client2`. + await makeNavigationRequest(scope, '/', 'client1'); + server.clearRequests(); + brokenLazyServer.clearRequests(); + + expect(await makeRequest(scope, '/foo.txt', 'client1')).toBe('this is foo (broken)'); + expect(await makeRequest(scope, '/foo.txt', 'client2')).toBe('this is foo'); + server.assertNoOtherRequests(); + brokenLazyServer.assertNoOtherRequests(); + + // Trying to fetch `bar.txt` (which has an invalid hash) should invalidate the latest + // version, enter degraded mode and "forget" clients that are on that version (i.e. + // `client1`). + expect(await makeRequest(scope, '/bar.txt', 'client1')).toBe('this is bar (broken)'); + expect(driver.state).toBe(DriverReadyState.EXISTING_CLIENTS_ONLY); + brokenLazyServer.sawRequestFor('/bar.txt'); + brokenLazyServer.clearRequests(); + + // `client1` should not be served from the network. + expect(await makeRequest(scope, '/foo.txt', 'client1')).toBe('this is foo (broken)'); + brokenLazyServer.sawRequestFor('/foo.txt'); + + // `client2` should still be served from the old version (since it never updated). + expect(await makeRequest(scope, '/foo.txt', 'client2')).toBe('this is foo'); + server.assertNoOtherRequests(); + brokenLazyServer.assertNoOtherRequests(); + }); + it('ignores invalid `only-if-cached` requests ', async() => { const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) => makeRequest(scope, '/foo.txt', undefined, {cache, mode}); @@ -1234,7 +1310,7 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; expect(requestUrls2).toContain(httpsRequestUrl); }); - describe('Backwards compatibility with v5', () => { + describe('backwards compatibility with v5', () => { beforeEach(() => { const serverV5 = new MockServerStateBuilder() .withStaticFiles(dist) @@ -1246,7 +1322,7 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; }); // Test this bug: https://github.com/angular/angular/issues/27209 - it('Fill previous versions of manifests with default navigation urls for backwards compatibility', + it('fills previous versions of manifests with default navigation urls for backwards compatibility', async() => { expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); await driver.initialized;