fix(service-worker): ensure initialization before handling messages

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<void> {
+    // 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<void> {
     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<Response> {
-    // 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);

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;