From 72eba7745fee1eacb4b4d076ce9f81dcb5d510ea Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Fri, 6 Sep 2019 23:57:21 -0400 Subject: [PATCH] fix(service-worker): ensure initialization before handling messages (#32525) - resolves "Invariant violated (initialize): latest hash null has no known manifest" - Thanks to @gkalpak and @hsta for helping test and investigate this fix Fixes #25611 PR Close #32525 --- packages/service-worker/worker/src/driver.ts | 89 +++++++++---------- .../service-worker/worker/test/happy_spec.ts | 42 +++++++++ 2 files changed, 85 insertions(+), 46 deletions(-) diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts index afa0eb71b0b53..2091bd8d95cde 100644 --- a/packages/service-worker/worker/src/driver.ts +++ b/packages/service-worker/worker/src/driver.ts @@ -250,34 +250,23 @@ export class Driver implements Debuggable, UpdateSource { return; } - // Initialization is the only event which is sent directly from the SW to itself, - // and thus `event.source` is not a Client. Handle it here, before the check - // for Client sources. - if (data.action === 'INITIALIZE') { - // Only initialize if not already initialized (or initializing). - if (this.initialized === null) { - // Initialize the SW. - this.initialized = this.initialize(); - - // Wait until initialization is properly scheduled, then trigger idle - // events to allow it to complete (assuming the SW is idle). - event.waitUntil((async() => { - await this.initialized; - await this.idle.trigger(); - })()); + event.waitUntil((async() => { + // Initialization is the only event which is sent directly from the SW to itself, and thus + // `event.source` is not a `Client`. Handle it here, before the check for `Client` sources. + if (data.action === 'INITIALIZE') { + return this.ensureInitialized(event); } - return; - } - - // Only messages from true clients are accepted past this point (this is essentially - // a typecast). - if (!this.adapter.isClient(event.source)) { - return; - } + // Only messages from true clients are accepted past this point. + // This is essentially a typecast. + if (!this.adapter.isClient(event.source)) { + return; + } - // Handle the message and keep the SW alive until it's handled. - event.waitUntil(this.handleMessage(data, event.source)); + // Handle the message and keep the SW alive until it's handled. + await this.ensureInitialized(event); + await this.handleMessage(data, event.source); + })()); } private onPush(msg: PushEvent): void { @@ -295,6 +284,32 @@ export class Driver implements Debuggable, UpdateSource { event.waitUntil(this.handleClick(event.notification, event.action)); } + private async ensureInitialized(event: ExtendableEvent): Promise { + // Since the SW may have just been started, it may or may not have been initialized already. + // `this.initialized` will be `null` if initialization has not yet been attempted, or will be a + // `Promise` which will resolve (successfully or unsuccessfully) if it has. + if (this.initialized !== null) { + return this.initialized; + } + + // Initialization has not yet been attempted, so attempt it. This should only ever happen once + // per SW instantiation. + try { + this.initialized = this.initialize(); + await this.initialized; + } catch (error) { + // If initialization fails, the SW needs to enter a safe state, where it declines to respond + // to network requests. + this.state = DriverReadyState.SAFE_MODE; + this.stateMessage = `Initialization failed due to error: ${errorToString(error)}`; + + throw error; + } finally { + // Regardless if initialization succeeded, background tasks still need to happen. + event.waitUntil(this.idle.trigger()); + } + } + private async handleMessage(msg: MsgAny&{action: string}, from: Client): Promise { if (isMsgCheckForUpdates(msg)) { const action = (async() => { await this.checkForUpdate(); })(); @@ -383,28 +398,10 @@ export class Driver implements Debuggable, UpdateSource { } private async handleFetch(event: FetchEvent): Promise { - // Since the SW may have just been started, it may or may not have been initialized already. - // this.initialized will be `null` if initialization has not yet been attempted, or will be a - // Promise which will resolve (successfully or unsuccessfully) if it has. - if (this.initialized === null) { - // Initialization has not yet been attempted, so attempt it. This should only ever happen once - // per SW instantiation. - this.initialized = this.initialize(); - } - - // If initialization fails, the SW needs to enter a safe state, where it declines to respond to - // network requests. try { - // Wait for initialization. - await this.initialized; - } catch (e) { - // Initialization failed. Enter a safe state. - this.state = DriverReadyState.SAFE_MODE; - this.stateMessage = `Initialization failed due to error: ${errorToString(e)}`; - - // Even though the driver entered safe mode, background tasks still need to happen. - event.waitUntil(this.idle.trigger()); - + // Ensure the SW instance has been initialized. + await this.ensureInitialized(event); + } catch { // Since the SW is already committed to responding to the currently active request, // respond with a network fetch. return this.safeFetch(event.request); diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 0bb0a93005a72..54b313509f66d 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -366,6 +366,48 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; server.assertNoOtherRequests(); }); + it('initializes the service worker on fetch if it has not yet been initialized', async() => { + // Driver is initially uninitialized. + expect(driver.initialized).toBeNull(); + expect(driver['latestHash']).toBeNull(); + + // Making a request initializes the driver (fetches assets). + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + expect(driver['latestHash']).toEqual(jasmine.any(String)); + server.assertSawRequestFor('ngsw.json'); + server.assertSawRequestFor('/foo.txt'); + server.assertSawRequestFor('/bar.txt'); + server.assertSawRequestFor('/redirected.txt'); + + // Once initialized, cached resources are served without network requests. + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + expect(await makeRequest(scope, '/bar.txt')).toEqual('this is bar'); + server.assertNoOtherRequests(); + }); + + it('initializes the service worker on message if it has not yet been initialized', async() => { + // Driver is initially uninitialized. + expect(driver.initialized).toBeNull(); + expect(driver['latestHash']).toBeNull(); + + // Pushing a message initializes the driver (fetches assets). + await scope.handleMessage({action: 'foo'}, 'someClient'); + expect(driver['latestHash']).toEqual(jasmine.any(String)); + server.assertSawRequestFor('ngsw.json'); + server.assertSawRequestFor('/foo.txt'); + server.assertSawRequestFor('/bar.txt'); + server.assertSawRequestFor('/redirected.txt'); + + // Once initialized, pushed messages are handled without re-initializing. + await scope.handleMessage({action: 'bar'}, 'someClient'); + server.assertNoOtherRequests(); + + // Once initialized, cached resources are served without network requests. + expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); + expect(await makeRequest(scope, '/bar.txt')).toEqual('this is bar'); + server.assertNoOtherRequests(); + }); + it('handles non-relative URLs', async() => { expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo'); await driver.initialized;