fix(service-worker): do not blow up when caches are unwritable (#26042)

In some cases, example when the user clears the caches in DevTools but
the SW remains active on another tab and keeps references to the deleted
caches, trying to write to the cache throws errors (e.g.
`Entry was not found`).

When this happens, the SW can no longer work correctly and should enter
a degraded mode allowing requests to be served from the network.

Possibly related:
- https://github.com/GoogleChrome/workbox/issues/792
- https://bugs.chromium.org/p/chromium/issues/detail?id=639034

This commits remedies this situation, by ensuring the SW can enter the
degraded `EXISTING_CLIENTS_ONLY` mode and forward requests to the
network.

PR Close #26042
This commit is contained in:
George Kalpakas 2018-09-20 21:04:37 +03:00 committed by Kara Erickson
parent 1e02cd9961
commit 2bd767c4a6
5 changed files with 84 additions and 28 deletions

View File

@ -9,7 +9,7 @@
import {Adapter, Context} from './adapter'; import {Adapter, Context} from './adapter';
import {CacheState, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {CacheState, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api';
import {Database, Table} from './database'; import {Database, Table} from './database';
import {SwCriticalError} from './error'; import {SwCriticalError, errorToString} from './error';
import {IdleScheduler} from './idle'; import {IdleScheduler} from './idle';
import {AssetGroupConfig} from './manifest'; import {AssetGroupConfig} from './manifest';
import {sha1Binary} from './sha1'; 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}`); `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 try {
// is complete. // This response is safe to cache (as long as it's cloned). Wait until the cache operation
const cache = await this.scope.caches.open(`${this.prefix}:${this.config.name}:cache`); // is complete.
await cache.put(req, res.clone()); 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 // If the request is not hashed, update its metadata, especially the timestamp. This is
// for future determination of whether this cached response is stale or not. // needed for future determination of whether this cached response is stale or not.
if (!this.hashes.has(req.url)) { if (!this.hashes.has(req.url)) {
// Metadata is tracked for requests that are unhashed. // Metadata is tracked for requests that are unhashed.
const meta: UrlMetadata = {ts: this.adapter.time, used}; const meta: UrlMetadata = {ts: this.adapter.time, used};
const metaTable = await this.metadata; const metaTable = await this.metadata;
await metaTable.write(req.url, meta); 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 {
// Finally, it can be removed from `inFlightRequests`. This might result in a double-remove // 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); this.inFlightRequests.delete(req.url);
} }
} }

View File

@ -6,12 +6,12 @@
* found in the LICENSE file at https://angular.io/license * 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 {CacheState, DebugIdleState, DebugState, DebugVersion, Debuggable, UpdateCacheStatus, UpdateSource} from './api';
import {AppVersion} from './app-version'; import {AppVersion} from './app-version';
import {Database, Table} from './database'; import {Database} from './database';
import {DebugHandler} from './debug'; import {DebugHandler} from './debug';
import {SwCriticalError} from './error'; import {errorToString} from './error';
import {IdleScheduler} from './idle'; import {IdleScheduler} from './idle';
import {Manifest, ManifestHash, hashManifest} from './manifest'; import {Manifest, ManifestHash, hashManifest} from './manifest';
import {MsgAny, isMsgActivateUpdate, isMsgCheckForUpdates} from './msg'; 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 // network, but caches continue to be valid for previous versions. This is
// unfortunate but unavoidable. // unfortunate but unavoidable.
this.state = DriverReadyState.EXISTING_CLIENTS_ONLY; 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. // Cancel the binding for these clients.
Array.from(this.clientVersionMap.keys()) Array.from(this.clientVersionMap.keys())
@ -724,7 +724,14 @@ export class Driver implements Debuggable, UpdateSource {
// Push the affected clients onto the latest version. // Push the affected clients onto the latest version.
affectedClients.forEach(clientId => this.clientVersionMap.set(clientId, this.latestHash !)); 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<void> { private async setupUpdate(manifest: Manifest, hash: string): Promise<void> {
@ -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}`;
}
}

View File

@ -7,3 +7,11 @@
*/ */
export class SwCriticalError extends Error { readonly isCritical: boolean = true; } 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}`;
}
}

View File

@ -11,6 +11,7 @@ import {CacheDatabase} from '../src/db-cache';
import {Driver, DriverReadyState} from '../src/driver'; import {Driver, DriverReadyState} from '../src/driver';
import {Manifest} from '../src/manifest'; import {Manifest} from '../src/manifest';
import {sha1} from '../src/sha1'; import {sha1} from '../src/sha1';
import {MockCache, clearAllCaches} from '../testing/cache';
import {MockRequest} from '../testing/fetch'; import {MockRequest} from '../testing/fetch';
import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock'; import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock';
import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
@ -851,6 +852,28 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
expect(driver.state).toEqual(DriverReadyState.EXISTING_CLIENTS_ONLY); 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() => { async_it('ignores invalid `only-if-cached` requests ', async() => {
const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) => const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) =>
makeRequest(scope, '/foo.txt', undefined, {cache, mode}); makeRequest(scope, '/foo.txt', undefined, {cache, mode});

View File

@ -164,3 +164,20 @@ export class MockCache {
return dehydrated; 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<void> {
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)));
}));
}