diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index 9e7fff4b3c207..6b7f7373f57a6 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -9,6 +9,7 @@ import {Adapter, Context} from './adapter'; import {CacheState, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {Database, Table} from './database'; +import {SwCriticalError} from './error'; import {IdleScheduler} from './idle'; import {AssetGroupConfig} from './manifest'; import {sha1Binary} from './sha1'; @@ -345,7 +346,7 @@ export abstract class AssetGroup { if ((res as any)['redirected'] && !!res.url) { // If the redirect limit is exhausted, fail with an error. if (redirectLimit === 0) { - throw new Error( + throw new SwCriticalError( `Response hit redirect limit (fetchFromNetwork): request redirected too many times, next is ${res.url}`); } @@ -409,7 +410,7 @@ export abstract class AssetGroup { // If the response was unsuccessful, there's nothing more that can be done. if (!cacheBustedResult.ok) { - throw new Error( + throw new SwCriticalError( `Response not Ok (cacheBustedFetchFromNetwork): cache busted request for ${req.url} returned response ${cacheBustedResult.status} ${cacheBustedResult.statusText}`); } @@ -419,7 +420,7 @@ export abstract class AssetGroup { // If the cache-busted version doesn't match, then the manifest is not an accurate // representation of the server's current set of files, and the SW should give up. if (canonicalHash !== cacheBustedHash) { - throw new Error( + throw new SwCriticalError( `Hash mismatch (cacheBustedFetchFromNetwork): ${req.url}: expected ${canonicalHash}, got ${cacheBustedHash} (after cache busting)`); } diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index 755e9e4a297da..65c1d3c9f1edf 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -11,6 +11,7 @@ import {CacheState, DebugIdleState, DebugState, DebugVersion, Debuggable, Update import {AppVersion} from './app-version'; import {Database, Table} from './database'; import {DebugHandler} from './debug'; +import {SwCriticalError} from './error'; import {IdleScheduler} from './idle'; import {Manifest, ManifestHash, hashManifest} from './manifest'; import {MsgAny, isMsgActivateUpdate, isMsgCheckForUpdates} from './msg'; @@ -38,7 +39,7 @@ interface LatestEntry { latest: string; } -enum DriverReadyState { +export enum DriverReadyState { // The SW is operating in a normal mode, responding to all traffic. NORMAL, @@ -57,7 +58,7 @@ export class Driver implements Debuggable, UpdateSource { * Tracks the current readiness condition under which the SW is operating. This controls * whether the SW attempts to respond to some or all requests. */ - private state: DriverReadyState = DriverReadyState.NORMAL; + state: DriverReadyState = DriverReadyState.NORMAL; private stateMessage: string = '(nominal)'; /** @@ -351,8 +352,22 @@ export class Driver implements Debuggable, UpdateSource { return this.safeFetch(event.request); } - // Handle the request. First try the AppVersion. If that doesn't work, fall back on the network. - const res = await appVersion.handleFetch(event.request, event); + let res: Response|null = null; + try { + // Handle the request. First try the AppVersion. If that doesn't work, fall back on the + // network. + res = await appVersion.handleFetch(event.request, event); + } catch (err) { + if (err.isCritical) { + // Something went wrong with the activation of this version. + await this.versionFailed(appVersion, err, this.latestHash === appVersion.manifestHash); + + event.waitUntil(this.idle.trigger()); + return this.safeFetch(event.request); + } + throw err; + } + // The AppVersion will only return null if the manifest doesn't specify what to do about this // request. In that case, just fall back on the network. @@ -482,7 +497,7 @@ export class Driver implements Debuggable, UpdateSource { // Attempt to schedule or initialize this version. If this operation is // successful, then initialization either succeeded or was scheduled. If // it fails, then full initialization was attempted and failed. - await this.scheduleInitialization(this.versions.get(hash) !); + await this.scheduleInitialization(this.versions.get(hash) !, this.latestHash === hash); } catch (err) { this.debugger.log(err, `initialize: schedule init of ${hash}`); return false; @@ -623,13 +638,13 @@ export class Driver implements Debuggable, UpdateSource { * when the SW is not busy and has connectivity. This returns a Promise which must be * awaited, as under some conditions the AppVersion might be initialized immediately. */ - private async scheduleInitialization(appVersion: AppVersion): Promise { + private async scheduleInitialization(appVersion: AppVersion, latest: boolean): Promise { const initialize = async() => { try { await appVersion.initializeFully(); } catch (err) { this.debugger.log(err, `initializeFully for ${appVersion.manifestHash}`); - await this.versionFailed(appVersion, err); + await this.versionFailed(appVersion, err, latest); } }; // TODO: better logic for detecting localhost. @@ -639,7 +654,7 @@ export class Driver implements Debuggable, UpdateSource { this.idle.schedule(`initialization(${appVersion.manifestHash})`, initialize); } - private async versionFailed(appVersion: AppVersion, err: Error): Promise { + private async versionFailed(appVersion: AppVersion, err: Error, latest: boolean): Promise { // This particular AppVersion is broken. First, find the manifest hash. const broken = Array.from(this.versions.entries()).find(([hash, version]) => version === appVersion); @@ -653,7 +668,7 @@ export class Driver implements Debuggable, UpdateSource { // The action taken depends on whether the broken manifest is the active (latest) or not. // If so, the SW cannot accept new clients, but can continue to service old ones. - if (this.latestHash === brokenHash) { + if (this.latestHash === brokenHash || latest) { // The latest manifest is broken. This means that new clients are at the mercy of the // network, but caches continue to be valid for previous versions. This is // unfortunate but unavoidable. @@ -711,9 +726,10 @@ export class Driver implements Debuggable, UpdateSource { } async checkForUpdate(): Promise { + let hash: string = '(unknown)'; try { const manifest = await this.fetchLatestManifest(); - const hash = hashManifest(manifest); + hash = hashManifest(manifest); // Check whether this is really an update. if (this.versions.has(hash)) { @@ -723,7 +739,12 @@ export class Driver implements Debuggable, UpdateSource { await this.setupUpdate(manifest, hash); return true; - } catch (_) { + } catch (err) { + this.debugger.log(err, `Error occurred while updating to manifest ${hash}`); + + this.state = DriverReadyState.EXISTING_CLIENTS_ONLY; + this.stateMessage = `Degraded due to failed initialization: ${errorToString(err)}`; + return false; } } diff --git a/packages/service-worker/worker/src/error.ts b/packages/service-worker/worker/src/error.ts new file mode 100644 index 0000000000000..fa445acabd2a9 --- /dev/null +++ b/packages/service-worker/worker/src/error.ts @@ -0,0 +1,9 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +export class SwCriticalError extends Error { readonly isCritical: boolean = true; } diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 6e128249de83b..bf0d71fd2a6ae 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -7,7 +7,7 @@ */ import {CacheDatabase} from '../src/db-cache'; -import {Driver} from '../src/driver'; +import {Driver, DriverReadyState} from '../src/driver'; import {Manifest} from '../src/manifest'; import {sha1} from '../src/sha1'; import {MockRequest} from '../testing/fetch'; @@ -36,6 +36,24 @@ const distUpdate = .addUnhashedFile('/unhashed/a.txt', 'this is unhashed v2', {'Cache-Control': 'max-age=10'}) .build(); +const brokenFs = new MockFileSystemBuilder().addFile('/foo.txt', 'this is foo').build(); + +const brokenManifest: Manifest = { + configVersion: 1, + index: '/foo.txt', + assetGroups: [{ + name: 'assets', + installMode: 'prefetch', + updateMode: 'prefetch', + urls: [ + '/foo.txt', + ], + patterns: [], + }], + dataGroups: [], + hashTable: tmpHashTableForFs(brokenFs, {'/foo.txt': true}), +}; + const manifest: Manifest = { configVersion: 1, appData: { @@ -132,6 +150,9 @@ const serverUpdate = .withRedirect('/redirected.txt', '/redirect-target.txt', 'this was a redirect') .build(); +const brokenServer = + new MockServerStateBuilder().withStaticFiles(brokenFs).withManifest(brokenManifest).build(); + const server404 = new MockServerStateBuilder().withStaticFiles(dist).build(); const scope = new SwTestHarnessBuilder().withServerState(server).build(); @@ -673,6 +694,34 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate)); server.assertSawRequestFor('/baz'); }); }); + + describe('bugs', () => { + async_it('does not crash with bad index hash', async() => { + scope = new SwTestHarnessBuilder().withServerState(brokenServer).build(); + (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'); + }); + + async_it('enters degraded mode when update has a bad index', async() => { + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + await driver.initialized; + server.clearRequests(); + + scope = new SwTestHarnessBuilder() + .withCacheState(scope.caches.dehydrate()) + .withServerState(brokenServer) + .build(); + driver = new Driver(scope, scope, new CacheDatabase(scope, scope)); + await driver.checkForUpdate(); + + scope.advance(12000); + await driver.idle.empty; + + expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY); + }); + }); }); })(); diff --git a/packages/service-worker/worker/testing/mock.ts b/packages/service-worker/worker/testing/mock.ts index 9ceadaf7a3bde..4ab6502464322 100644 --- a/packages/service-worker/worker/testing/mock.ts +++ b/packages/service-worker/worker/testing/mock.ts @@ -194,12 +194,16 @@ export function tmpManifestSingleAssetGroup(fs: MockFileSystem): Manifest { }; } -export function tmpHashTableForFs(fs: MockFileSystem): {[url: string]: string} { +export function tmpHashTableForFs( + fs: MockFileSystem, breakHashes: {[url: string]: boolean} = {}): {[url: string]: string} { const table: {[url: string]: string} = {}; fs.list().forEach(path => { const file = fs.lookup(path) !; if (file.hashThisFile) { table[path] = file.hash; + if (breakHashes[path]) { + table[path] = table[path].split('').reverse().join(''); + } } }); return table;