From 9e9b8dd494cac7f1bd3a6f0effded7a15023ce1a Mon Sep 17 00:00:00 2001
From: George Kalpakas <kalpakas.g@gmail.com>
Date: Tue, 20 Mar 2018 00:40:32 +0200
Subject: [PATCH] fix(service-worker): do not enter degraded mode when offline
 (#22883)

Previously, when trying to fetch `ngsw.json` (e.g. during
`checkForUpdate()`) while either the client or the server were offline,
the ServiceWorker would enter a degrade mode, where only existing
clients would be served. This essentially meant that the ServiceWorker
didn't work offline.
This commit fixes it by differentiating offline errors and not entering
degraded mode. The ServiceWorker will remain in the current mode until
connectivity to the server is restored.

Fixes #21636

PR Close #22883
---
 packages/service-worker/worker/src/driver.ts  | 22 +++++++++++++++----
 .../service-worker/worker/test/happy_spec.ts  | 11 ++++++++++
 .../service-worker/worker/testing/mock.ts     |  6 +++++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts
index b4e046539f..19fc0dded6 100644
--- a/packages/service-worker/worker/src/driver.ts
+++ b/packages/service-worker/worker/src/driver.ts
@@ -607,16 +607,22 @@ export class Driver implements Debuggable, UpdateSource {
 
   /**
    * Retrieve a copy of the latest manifest from the server.
+   * Return `null` if `ignoreOfflineError` is true (default: false) and the server or client are
+   * offline (detected as response status 504).
    */
-  private async fetchLatestManifest(): Promise<Manifest> {
+  private async fetchLatestManifest(ignoreOfflineError?: false): Promise<Manifest>;
+  private async fetchLatestManifest(ignoreOfflineError: true): Promise<Manifest | null>;
+  private async fetchLatestManifest(ignoreOfflineError = false): Promise<Manifest | null> {
     const res =
         await this.safeFetch(this.adapter.newRequest('ngsw.json?ngsw-cache-bust=' + Math.random()));
     if (!res.ok) {
       if (res.status === 404) {
         await this.deleteAllCaches();
         await this.scope.registration.unregister();
+      } else if (res.status === 504 && ignoreOfflineError) {
+        return null;
       }
-      throw new Error('Manifest fetch failed!');
+      throw new Error(`Manifest fetch failed! (status: ${res.status})`);
     }
     this.lastUpdateCheck = this.adapter.time;
     return res.json();
@@ -728,7 +734,15 @@ export class Driver implements Debuggable, UpdateSource {
   async checkForUpdate(): Promise<boolean> {
     let hash: string = '(unknown)';
     try {
-      const manifest = await this.fetchLatestManifest();
+      const manifest = await this.fetchLatestManifest(true);
+
+      if (manifest === null) {
+        // Client or server offline. Unable to check for updates at this time.
+        // Continue to service clients (existing and new).
+        this.debugger.log('Check for update aborted. (Client or server offline.)');
+        return false;
+      }
+
       hash = hashManifest(manifest);
 
       // Check whether this is really an update.
@@ -972,4 +986,4 @@ function errorToString(error: any): string {
   } else {
     return `${error}`;
   }
-}
\ No newline at end of file
+}
diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts
index 2a63385850..f560394805 100644
--- a/packages/service-worker/worker/test/happy_spec.ts
+++ b/packages/service-worker/worker/test/happy_spec.ts
@@ -516,6 +516,17 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
       expect(await scope.caches.keys()).toEqual([]);
     });
 
+    async_it('does not unregister or change state when offline (i.e. manifest 504s)', async() => {
+      expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
+      await driver.initialized;
+      server.online = false;
+
+      expect(await driver.checkForUpdate()).toEqual(false);
+      expect(driver.state).toEqual(DriverReadyState.NORMAL);
+      expect(scope.unregistered).toBeFalsy();
+      expect(await scope.caches.keys()).not.toEqual([]);
+    });
+
     describe('unhashed requests', () => {
       async_beforeEach(async() => {
         expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
diff --git a/packages/service-worker/worker/testing/mock.ts b/packages/service-worker/worker/testing/mock.ts
index 4ab6502464..208e75f9ad 100644
--- a/packages/service-worker/worker/testing/mock.ts
+++ b/packages/service-worker/worker/testing/mock.ts
@@ -96,6 +96,7 @@ export class MockServerState {
   private gate: Promise<void> = Promise.resolve();
   private resolve: Function|null = null;
   private resolveNextRequest: Function;
+  online = true;
   nextRequest: Promise<Request>;
 
   constructor(private resources: Map<string, Response>, private errors: Set<string>) {
@@ -108,6 +109,10 @@ export class MockServerState {
 
     await this.gate;
 
+    if (!this.online) {
+      throw new Error('Offline.');
+    }
+
     if (req.credentials === 'include') {
       return new MockResponse(null, {status: 0, statusText: '', type: 'opaque'});
     }
@@ -171,6 +176,7 @@ export class MockServerState {
     this.nextRequest = new Promise(resolve => { this.resolveNextRequest = resolve; });
     this.gate = Promise.resolve();
     this.resolve = null;
+    this.online = true;
   }
 }