diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index b5dc2a9f04..dedbceff3a 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, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {Database, Table} from './database'; -import {SwCriticalError} from './error'; +import {SwCriticalError, errorToString} from './error'; import {IdleScheduler} from './idle'; import {AssetGroupConfig} from './manifest'; import {sha1Binary} from './sha1'; @@ -317,24 +317,33 @@ export abstract class AssetGroup { `Response not Ok (fetchAndCacheOnce): request for ${req.url} returned response ${res.status} ${res.statusText}`); } - // This response is safe to cache (as long as it's cloned). Wait until the cache operation - // is complete. - const cache = await this.scope.caches.open(`${this.prefix}:${this.config.name}:cache`); - await cache.put(req, res.clone()); + try { + // This response is safe to cache (as long as it's cloned). Wait until the cache operation + // is complete. + const cache = await this.scope.caches.open(`${this.prefix}:${this.config.name}:cache`); + await cache.put(req, res.clone()); - // If the request is not hashed, update its metadata, especially the timestamp. This is needed - // for future determination of whether this cached response is stale or not. - if (!this.hashes.has(req.url)) { - // Metadata is tracked for requests that are unhashed. - const meta: UrlMetadata = {ts: this.adapter.time, used}; - const metaTable = await this.metadata; - await metaTable.write(req.url, meta); + // If the request is not hashed, update its metadata, especially the timestamp. This is + // needed for future determination of whether this cached response is stale or not. + if (!this.hashes.has(req.url)) { + // Metadata is tracked for requests that are unhashed. + const meta: UrlMetadata = {ts: this.adapter.time, used}; + const metaTable = await this.metadata; + await metaTable.write(req.url, meta); + } + + return res; + } catch (err) { + // Among other cases, this can happen when the user clears all data through the DevTools, + // but the SW is still running and serving another tab. In that case, trying to write to the + // caches throws an `Entry was not found` error. + // If this happens the SW can no longer work correctly. This situation is unrecoverable. + throw new SwCriticalError( + `Failed to update the caches for request to '${req.url}' (fetchAndCacheOnce): ${errorToString(err)}`); } - - return res; } finally { // Finally, it can be removed from `inFlightRequests`. This might result in a double-remove - // if some other chain was already making this request too, but that won't hurt anything. + // if some other chain was already making this request too, but that won't hurt anything. this.inFlightRequests.delete(req.url); } } diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index 1dc99259b3..86afc43d86 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -6,12 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {Adapter, Context} from './adapter'; +import {Adapter} from './adapter'; import {CacheState, DebugIdleState, DebugState, DebugVersion, Debuggable, UpdateCacheStatus, UpdateSource} from './api'; import {AppVersion} from './app-version'; -import {Database, Table} from './database'; +import {Database} from './database'; import {DebugHandler} from './debug'; -import {SwCriticalError} from './error'; +import {errorToString} from './error'; import {IdleScheduler} from './idle'; import {Manifest, ManifestHash, hashManifest} from './manifest'; import {MsgAny, isMsgActivateUpdate, isMsgCheckForUpdates} from './msg'; @@ -709,7 +709,7 @@ export class Driver implements Debuggable, UpdateSource { // network, but caches continue to be valid for previous versions. This is // unfortunate but unavoidable. this.state = DriverReadyState.EXISTING_CLIENTS_ONLY; - this.stateMessage = `Degraded due to failed initialization: ${errorToString(err)}`; + this.stateMessage = `Degraded due to: ${errorToString(err)}`; // Cancel the binding for these clients. Array.from(this.clientVersionMap.keys()) @@ -724,7 +724,14 @@ export class Driver implements Debuggable, UpdateSource { // Push the affected clients onto the latest version. affectedClients.forEach(clientId => this.clientVersionMap.set(clientId, this.latestHash !)); } - await this.sync(); + + try { + await this.sync(); + } catch (err2) { + // We are already in a bad state. No need to make things worse. + // Just log the error and move on. + this.debugger.log(err2, `Driver.versionFailed(${err.message || err})`); + } } private async setupUpdate(manifest: Manifest, hash: string): Promise { @@ -999,11 +1006,3 @@ export class Driver implements Debuggable, UpdateSource { } } } - -function errorToString(error: any): string { - if (error instanceof Error) { - return `${error.message}\n${error.stack}`; - } else { - return `${error}`; - } -} diff --git a/packages/service-worker/worker/src/error.ts b/packages/service-worker/worker/src/error.ts index fa445acabd..25ecdadc0d 100644 --- a/packages/service-worker/worker/src/error.ts +++ b/packages/service-worker/worker/src/error.ts @@ -7,3 +7,11 @@ */ export class SwCriticalError extends Error { readonly isCritical: boolean = true; } + +export function errorToString(error: any): string { + if (error instanceof Error) { + return `${error.message}\n${error.stack}`; + } else { + return `${error}`; + } +} diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 4ca3969b4a..0e43108da0 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -11,6 +11,7 @@ import {CacheDatabase} from '../src/db-cache'; import {Driver, DriverReadyState} from '../src/driver'; import {Manifest} from '../src/manifest'; import {sha1} from '../src/sha1'; +import {MockCache, clearAllCaches} from '../testing/cache'; import {MockRequest} from '../testing/fetch'; import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock'; import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; @@ -851,6 +852,28 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate)); expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY); }); + async_it('enters degraded mode when failing to write to cache', async() => { + // Initialize the SW. + await makeRequest(scope, '/foo.txt'); + await driver.initialized; + expect(driver.state).toBe(DriverReadyState.NORMAL); + + server.clearRequests(); + + // Operate normally. + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo'); + server.assertNoOtherRequests(); + + // Clear the caches and make them unwritable. + await clearAllCaches(scope.caches); + spyOn(MockCache.prototype, 'put').and.throwError('Can\'t touch this'); + + // Enter degraded mode and serve from network. + expect(await makeRequest(scope, '/foo.txt')).toBe('this is foo'); + expect(driver.state).toBe(DriverReadyState.EXISTING_CLIENTS_ONLY); + server.assertSawRequestFor('/foo.txt'); + }); + async_it('ignores invalid `only-if-cached` requests ', async() => { const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) => makeRequest(scope, '/foo.txt', undefined, {cache, mode}); diff --git a/packages/service-worker/worker/testing/cache.ts b/packages/service-worker/worker/testing/cache.ts index b24147e2f6..5d9f3c2fee 100644 --- a/packages/service-worker/worker/testing/cache.ts +++ b/packages/service-worker/worker/testing/cache.ts @@ -164,3 +164,20 @@ export class MockCache { return dehydrated; } } + +// This can be used to simulate a situation (bug?), where the user clears the caches from DevTools, +// while the SW is still running (e.g. serving another tab) and keeps references to the deleted +// caches. +export async function clearAllCaches(caches: CacheStorage): Promise { + const cacheNames = await caches.keys(); + const cacheInstances = await Promise.all(cacheNames.map(name => caches.open(name))); + + // Delete all cache instances from `CacheStorage`. + await Promise.all(cacheNames.map(name => caches.delete(name))); + + // Delete all entries from each cache instance. + await Promise.all(cacheInstances.map(async cache => { + const keys = await cache.keys(); + await Promise.all(keys.map(key => cache.delete(key))); + })); +}