diff --git a/packages/service-worker/worker/src/app-version.ts b/packages/service-worker/worker/src/app-version.ts index 5db2d6d3a4..434b1ad038 100644 --- a/packages/service-worker/worker/src/app-version.ts +++ b/packages/service-worker/worker/src/app-version.ts @@ -11,6 +11,7 @@ import {CacheState, UpdateCacheStatus, UpdateSource} from './api'; import {AssetGroup, LazyAssetGroup, PrefetchAssetGroup} from './assets'; import {DataGroup} from './data'; import {Database} from './database'; +import {DebugHandler} from './debug'; import {IdleScheduler} from './idle'; import {Manifest} from './manifest'; @@ -60,7 +61,8 @@ export class AppVersion implements UpdateSource { constructor( private scope: ServiceWorkerGlobalScope, private adapter: Adapter, private database: Database, - private idle: IdleScheduler, readonly manifest: Manifest, readonly manifestHash: string) { + private idle: IdleScheduler, private debugHandler: DebugHandler, readonly manifest: Manifest, + readonly manifestHash: string) { // The hashTable within the manifest is an Object - convert it to a Map for easier lookups. Object.keys(this.manifest.hashTable).forEach(url => { this.hashTable.set(url, this.manifest.hashTable[url]); @@ -85,11 +87,12 @@ export class AppVersion implements UpdateSource { }); // Process each `DataGroup` declared in the manifest. - this.dataGroups = (manifest.dataGroups || []) - .map( - config => new DataGroup( - this.scope, this.adapter, config, this.database, - `${adapter.cacheNamePrefix}:${config.version}:data`)); + this.dataGroups = + (manifest.dataGroups || []) + .map( + config => new DataGroup( + this.scope, this.adapter, config, this.database, this.debugHandler, + `${adapter.cacheNamePrefix}:${config.version}:data`)); // This keeps backwards compatibility with app versions without navigation urls. // Fix: https://github.com/angular/angular/issues/27209 diff --git a/packages/service-worker/worker/src/data.ts b/packages/service-worker/worker/src/data.ts index 9ba08f029f..4d9fdd7242 100644 --- a/packages/service-worker/worker/src/data.ts +++ b/packages/service-worker/worker/src/data.ts @@ -8,6 +8,7 @@ import {Adapter, Context} from './adapter'; import {Database, Table} from './database'; +import {DebugHandler} from './debug'; import {DataGroupConfig} from './manifest'; /** @@ -243,7 +244,8 @@ export class DataGroup { constructor( private scope: ServiceWorkerGlobalScope, private adapter: Adapter, - private config: DataGroupConfig, private db: Database, private prefix: string) { + private config: DataGroupConfig, private db: Database, private debugHandler: DebugHandler, + private prefix: string) { this.patterns = this.config.patterns.map(pattern => new RegExp(pattern)); this.cache = this.scope.caches.open(`${this.prefix}:dynamic:${this.config.name}:cache`); this.lruTable = this.db.open(`${this.prefix}:dynamic:${this.config.name}:lru`); @@ -356,7 +358,7 @@ export class DataGroup { ctx.waitUntil(this.safeCacheResponse(req, networkFetch, lru)); } else { // The request completed in time, so cache it inline with the response flow. - await this.cacheResponse(req, res, lru); + await this.safeCacheResponse(req, res, lru); } return res; @@ -385,7 +387,7 @@ export class DataGroup { const fromCache = await this.loadFromCache(req, lru); res = (fromCache !== null) ? fromCache.res : null; } else { - await this.cacheResponse(req, res, lru, true); + await this.safeCacheResponse(req, res, lru, true); } // Either the network fetch didn't time out, or the cache yielded a usable response. @@ -442,7 +444,10 @@ export class DataGroup { } catch (err) { // Saving the API response failed. This could be a result of a full storage. // Since this data is cached lazily and temporarily, continue serving clients as usual. - // TODO: Log error + this.debugHandler.log( + err, + `DataGroup(${this.config.name}@${this.config.version}).safeCacheResponse(${req.url}, status: ${res.status})`); + // TODO: Better detect/handle full storage; e.g. using // [navigator.storage](https://developer.mozilla.org/en-US/docs/Web/API/NavigatorStorage/storage). } diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index 09b966aeed..afa0eb71b0 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -533,7 +533,8 @@ export class Driver implements Debuggable, UpdateSource { // created for it. if (!this.versions.has(hash)) { this.versions.set( - hash, new AppVersion(this.scope, this.adapter, this.db, this.idle, manifest, hash)); + hash, new AppVersion( + this.scope, this.adapter, this.db, this.idle, this.debugger, manifest, hash)); } }); @@ -783,7 +784,8 @@ export class Driver implements Debuggable, UpdateSource { } private async setupUpdate(manifest: Manifest, hash: string): Promise { - const newVersion = new AppVersion(this.scope, this.adapter, this.db, this.idle, manifest, hash); + const newVersion = + new AppVersion(this.scope, this.adapter, this.db, this.idle, this.debugger, manifest, hash); // Firstly, check if the manifest version is correct. if (manifest.configVersion !== SUPPORTED_CONFIG_VERSION) { diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index aa6a673473..0bb0a93005 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -34,6 +34,9 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; .addFile('/lazy/unchanged2.txt', 'this is unchanged (2)') .addUnhashedFile('/unhashed/a.txt', 'this is unhashed', {'Cache-Control': 'max-age=10'}) .addUnhashedFile('/unhashed/b.txt', 'this is unhashed b', {'Cache-Control': 'no-cache'}) + .addUnhashedFile('/api/foo', 'this is api foo', {'Cache-Control': 'no-cache'}) + .addUnhashedFile( + '/api-static/bar', 'this is static api bar', {'Cache-Control': 'no-cache'}) .build(); const distUpdate = @@ -172,11 +175,21 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; version: 42, maxAge: 3600000, maxSize: 100, - strategy: 'performance', + strategy: 'freshness', patterns: [ '/api/.*', ], }, + { + name: 'api-static', + version: 43, + maxAge: 3600000, + maxSize: 100, + strategy: 'performance', + patterns: [ + '/api-static/.*', + ], + }, ], navigationUrls: processNavigationUrls(''), hashTable: tmpHashTableForFs(dist), @@ -829,6 +842,9 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; `ngsw:${baseHref}:42:data:dynamic:api:cache`, `ngsw:${baseHref}:db:ngsw:${baseHref}:42:data:dynamic:api:lru`, `ngsw:${baseHref}:db:ngsw:${baseHref}:42:data:dynamic:api:age`, + `ngsw:${baseHref}:43:data:dynamic:api-static:cache`, + `ngsw:${baseHref}:db:ngsw:${baseHref}:43:data:dynamic:api-static:lru`, + `ngsw:${baseHref}:db:ngsw:${baseHref}:43:data:dynamic:api-static:age`, ]; const getClientAssignments = async(sw: SwTestHarness, baseHref: string) => { @@ -1230,6 +1246,48 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; server.assertSawRequestFor('/foo.txt'); }); + it('keeps serving api requests with freshness strategy when failing to write to cache', + async() => { + // Initialize the SW. + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + await driver.initialized; + server.clearRequests(); + + // Make the caches unwritable. + spyOn(MockCache.prototype, 'put').and.throwError('Can\'t touch this'); + spyOn(driver.debugger, 'log'); + + expect(await makeRequest(scope, '/api/foo')).toEqual('this is api foo'); + expect(driver.state).toBe(DriverReadyState.NORMAL); + // Since we are swallowing an error here, make sure it is at least properly logged + expect(driver.debugger.log) + .toHaveBeenCalledWith( + new Error('Can\'t touch this'), + 'DataGroup(api@42).safeCacheResponse(/api/foo, status: 200)'); + server.assertSawRequestFor('/api/foo'); + }); + + it('keeps serving api requests with performance strategy when failing to write to cache', + async() => { + // Initialize the SW. + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + await driver.initialized; + server.clearRequests(); + + // Make the caches unwritable. + spyOn(MockCache.prototype, 'put').and.throwError('Can\'t touch this'); + spyOn(driver.debugger, 'log'); + + expect(await makeRequest(scope, '/api-static/bar')).toEqual('this is static api bar'); + expect(driver.state).toBe(DriverReadyState.NORMAL); + // Since we are swallowing an error here, make sure it is at least properly logged + expect(driver.debugger.log) + .toHaveBeenCalledWith( + new Error('Can\'t touch this'), + 'DataGroup(api-static@43).safeCacheResponse(/api-static/bar, status: 200)'); + server.assertSawRequestFor('/api-static/bar'); + }); + it('enters degraded mode when something goes wrong with the latest version', async() => { await driver.initialized;