Skip to content

Commit

Permalink
fix(service-worker): Don't stay locked in EXISTING_CLIENTS_ONLY if co…
Browse files Browse the repository at this point in the history
…rrupted data (angular#37453)

**Problem**

After angular#31109 and angular#31865, it's still possible to get locked in state
`EXISTING_CLIENTS_ONLY`, without any possibility to get out (even by
pushing new updates on the server).
More specifically, if control doc `/latest` of `ngsw:/:db:control` once
gets a bad value, then the service worker will fail early, and won't be
able to overwrite `/latest` with new, valid values (the ones from future
updates).

For example, once in this state, URL `/ngsw/state` will show:

    NGSW Debug Info:
    Driver state: EXISTING_CLIENTS_ONLY (Degraded due to failed initialization: Invariant violated (initialize): latest hash 8b75… has no known manifest
    Error: Invariant violated (initialize): latest hash 8b75… has no known manifest
        at Driver.<anonymous> (https://my.app/ngsw-worker.js:2302:27)
        at Generator.next (<anonymous>)
        at fulfilled (https://my.app/ngsw-worker.js:175:62))
    Latest manifest hash: 8b75…
    Last update check: 22s971u

... with hash `8b75…` corresponding to no installed version.

**Solution**

Currently, when such a case happens, the service worker [simply fails
with an assertion][1]. Because this failure happens early, and is not
handled, the service worker is not able to update `/latest` to new
installed app versions.

I propose to detect this corrupted case (a `latest` hash that doesn't
match any installed version) a few lines above, so that the service
worker can correctly call its [already existing cleaning code][2].

[1]: https://github.com/angular/angular/blob/3569fdf/packages/service-worker/worker/src/driver.ts#L559-L563
[2]: https://github.com/angular/angular/blob/3569fdf/packages/service-worker/worker/src/driver.ts#L505-L519

This change successfully fixes the problem described above.

Unit test written with the help of George Kalpakas. Thank you!

PR Close angular#37453
  • Loading branch information
adrienverge authored and ngwattcos committed Jun 25, 2020
1 parent 165bbe4 commit 9088200
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
9 changes: 9 additions & 0 deletions packages/service-worker/worker/src/driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,15 @@ export class Driver implements Debuggable, UpdateSource {
table.read<LatestEntry>('latest'),
]);

// Make sure latest manifest is correctly installed. If not (e.g. corrupted data),
// it could stay locked in EXISTING_CLIENTS_ONLY or SAFE_MODE state.
if (!this.versions.has(latest.latest) && !manifests.hasOwnProperty(latest.latest)) {
this.debugger.log(
`Missing manifest for latest version hash ${latest.latest}`,
'initialize: read from DB');
throw new Error(`Missing manifest for latest hash ${latest.latest}`);
}

// Successfully loaded from saved state. This implies a manifest exists, so
// the update check needs to happen in the background.
this.idle.schedule('init post-load (update, cleanup)', async () => {
Expand Down
24 changes: 24 additions & 0 deletions packages/service-worker/worker/test/happy_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,30 @@ describe('Driver', () => {
expect(driver.state).toBe(DriverReadyState.NORMAL);
});

it('should not enter degraded mode if manifest for latest hash is missing upon initialization',
async () => {
// Initialize the SW.
scope.handleMessage({action: 'INITIALIZE'}, null);
await driver.initialized;
expect(driver.state).toBe(DriverReadyState.NORMAL);

// Ensure the data has been stored in the DB.
const db: MockCache = await scope.caches.open('ngsw:/:db:control') as any;
const getLatestHashFromDb = async () => (await (await db.match('/latest')).json()).latest;
expect(await getLatestHashFromDb()).toBe(manifestHash);

// Change the latest hash to not correspond to any manifest.
await db.put('/latest', new MockResponse('{"latest": "wrong-hash"}'));
expect(await getLatestHashFromDb()).toBe('wrong-hash');

// Re-initialize the SW and ensure it does not enter a degraded mode.
driver.initialized = null;
scope.handleMessage({action: 'INITIALIZE'}, null);
await driver.initialized;
expect(driver.state).toBe(DriverReadyState.NORMAL);
expect(await getLatestHashFromDb()).toBe(manifestHash);
});

it('ignores invalid `only-if-cached` requests ', async () => {
const requestFoo = (cache: RequestCache|'only-if-cached', mode: RequestMode) =>
makeRequest(scope, '/foo.txt', undefined, {cache, mode});
Expand Down

0 comments on commit 9088200

Please sign in to comment.