Add timeout to OnlineState tracking.

packages/firestore/CHANGELOG.md

@@ -1,4 +1,10 @@
 # Unreleased
+- [changed] If the SDK's attempt to connect to the Cloud Firestore backend
+  neither succeeds nor fails within 10 seconds, the SDK will consider itself
+  "offline", causing get() calls to resolve with cached results, rather than
+  continuing to wait.
+
+# 0.3.2
 - [fixed] Fixed a regression in Firebase JS release 4.9.0 that could in certain
   cases result in an "OnlineState should not affect limbo documents." assertion
   crash when the client loses its network connection.

packages/firestore/src/core/event_manager.ts

@@ -248,9 +248,9 @@ export class QueryListener {
       return true;
     }
 
-    // NOTE: We consider OnlineState.Unknown as online (it should become Failed
+    // NOTE: We consider OnlineState.Unknown as online (it should become Offline
     // or Online if we wait long enough).
-    const maybeOnline = onlineState !== OnlineState.Failed;
+    const maybeOnline = onlineState !== OnlineState.Offline;
     // Don't raise the event if we're online, aren't synced yet (checked
     // above) and are waiting for a sync.
     if (this.options.waitForSyncWhenOnline && maybeOnline) {
@@ -262,7 +262,7 @@ export class QueryListener {
     }
 
     // Raise data from cache if we have any documents or we are offline
-    return !snap.docs.isEmpty() || onlineState === OnlineState.Failed;
+    return !snap.docs.isEmpty() || onlineState === OnlineState.Offline;
   }
 
   private shouldRaiseEvent(snap: ViewSnapshot): boolean {

packages/firestore/src/core/firestore_client.ts

@@ -288,6 +288,7 @@ export class FirestoreClient {
         this.remoteStore = new RemoteStore(
           this.localStore,
           datastore,
+          this.asyncQueue,
           onlineStateChangedHandler
         );
 

packages/firestore/src/core/types.ts

@@ -51,12 +51,12 @@ export enum OnlineState {
    * reached after a successful connection and there has been at least one
    * successful message received from the backends.
    */
-  Healthy,
+  Online,
 
   /**
    * The client is either trying to establish a connection but failing, or it
    * has been explicitly marked offline via a call to disableNetwork().
    * Higher-level components should operate in offline mode.
    */
-  Failed
+  Offline
 }

packages/firestore/src/core/view.ts

@@ -276,7 +276,7 @@ export class View {
    * ViewChange if the view's syncState changes as a result.
    */
   applyOnlineStateChange(onlineState: OnlineState): ViewChange {
-    if (this.current && onlineState === OnlineState.Failed) {
+    if (this.current && onlineState === OnlineState.Offline) {
       // If we're offline, set `current` to false and then call applyChanges()
       // to refresh our syncState and generate a ViewChange as appropriate. We
       // are guaranteed to get a new TargetChange that sets `current` back to

packages/firestore/src/remote/online_state_tracker.ts

@@ -0,0 +1,172 @@
+/**
+ * Copyright 2018 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { OnlineState } from '../core/types';
+import * as log from '../util/log';
+import { assert } from '../util/assert';
+import { AsyncQueue, TimerId } from '../util/async_queue';
+import { CancelablePromise } from '../util/promise';
+
+const LOG_TAG = 'OnlineStateTracker';
+
+// To deal with transient failures, we allow multiple stream attempts before
+// giving up and transitioning from OnlineState.Unknown to Offline.
+const MAX_WATCH_STREAM_FAILURES = 2;
+
+// To deal with stream attempts that don't succeed or fail in a timely manner,
+// we have a timeout for OnlineState to reach Online or Offline.
+// If the timeout is reached, we transition to Offline rather than waiting
+// indefinitely.
+const ONLINE_STATE_TIMEOUT_MS = 10 * 1000;
+
+/**
+ * A component used by the RemoteStore to track the OnlineState (that is,
+ * whether or not the client as a whole should be considered to be online or
+ * offline), implementing the appropriate heuristics.
+ *
+ * In particular, when the client is trying to connect to the backend, we
+ * allow up to MAX_WATCH_STREAM_FAILURES within ONLINE_STATE_TIMEOUT_MS for
+ * a connection to succeed. If we have too many failures or the timeout elapses,
+ * then we set the OnlineState to Offline, and the client will behave as if
+ * it is offline (get()s will return cached data, etc.).
+ */
+export class OnlineStateTracker {
+  /** The current OnlineState. */
+  private state = OnlineState.Unknown;
+
+  /**
+   * A count of consecutive failures to open the stream. If it reaches the
+   * maximum defined by MAX_WATCH_STREAM_FAILURES, we'll set the OnlineState to
+   * Offline.
+   */
+  private watchStreamFailures = 0;
+
+  /**
+   * A timer that elapses after ONLINE_STATE_TIMEOUT_MS, at which point we
+   * transition from OnlineState.Unknown to OnlineState.Offline without waiting
+   * for the stream to actually fail (MAX_WATCH_STREAM_FAILURES times).
+   */
+  private onlineStateTimer: CancelablePromise<void> | null = null;
+
+  /**
+   * Whether the client should log a warning message if it fails to connect to
+   * the backend (initially true, cleared after a successful stream, or if we've
+   * logged the message already).
+   */
+  private shouldWarnClientIsOffline = true;
+
+  constructor(
+    private asyncQueue: AsyncQueue,
+    private onlineStateHandler: (onlineState: OnlineState) => void
+  ) {}
+
+  /**
+   * Called by RemoteStore when a watch stream is started.
+   *
+   * It sets the OnlineState to Unknown and starts the onlineStateTimer
+   * if necessary.
+   */
+  handleWatchStreamStart(): void {
+    this.setAndBroadcast(OnlineState.Unknown);
+
+    if (this.onlineStateTimer === null) {
+      this.onlineStateTimer = this.asyncQueue.enqueueAfterDelay(
+        TimerId.OnlineStateTimeout,
+        ONLINE_STATE_TIMEOUT_MS,
+        () => {
+          this.onlineStateTimer = null;
+          assert(
+            this.state === OnlineState.Unknown,
+            'Timer should be canceled if we transitioned to a different state.'
+          );
+          log.debug(
+            LOG_TAG,
+            `Watch stream didn't reach online or offline within ` +
+              `${ONLINE_STATE_TIMEOUT_MS}ms. Considering client offline.`
+          );
+          this.logClientOfflineWarningIfNecessary();
+          this.setAndBroadcast(OnlineState.Offline);
+
+          // NOTE: handleWatchStreamFailure() will continue to increment
+          // watchStreamFailures even though we are already marked Offline,
+          // but this is non-harmful.
+
+          return Promise.resolve();
+        }
+      );
+    }
+  }
+
+  /**
+   * Updates our OnlineState as appropriate after the watch stream reports a
+   * failure. The first failure moves us to the 'Unknown' state. We then may
+   * allow multiple failures (based on MAX_WATCH_STREAM_FAILURES) before we
+   * actually transition to the 'Offline' state.
+   */
+  handleWatchStreamFailure(): void {
+    if (this.state === OnlineState.Online) {
+      this.setAndBroadcast(OnlineState.Unknown);
+    } else {
+      this.watchStreamFailures++;
+      if (this.watchStreamFailures >= MAX_WATCH_STREAM_FAILURES) {
+        this.clearOnlineStateTimer();
+        this.logClientOfflineWarningIfNecessary();
+        this.setAndBroadcast(OnlineState.Offline);
+      }
+    }
+  }
+
+  /**
+   * Explicitly sets the OnlineState to the specified state.
+   *
+   * Note that this resets our timers / failure counters, etc. used by our
+   * Offline heuristics, so must not be used in place of
+   * handleWatchStreamStart() and handleWatchStreamFailure().
+   */
+  set(newState: OnlineState): void {
+    this.clearOnlineStateTimer();
+    this.watchStreamFailures = 0;
+
+    if (newState === OnlineState.Online) {
+      // We've connected to watch at least once. Don't warn the developer
+      // about being offline going forward.
+      this.shouldWarnClientIsOffline = false;
+    }
+
+    this.setAndBroadcast(newState);
+  }
+
+  private setAndBroadcast(newState: OnlineState): void {
+    if (newState !== this.state) {
+      this.state = newState;
+      this.onlineStateHandler(newState);
+    }
+  }
+
+  private logClientOfflineWarningIfNecessary(): void {
+    if (this.shouldWarnClientIsOffline) {
+      log.error('Could not reach Firestore backend.');
+      this.shouldWarnClientIsOffline = false;
+    }
+  }
+
+  private clearOnlineStateTimer(): void {
+    if (this.onlineStateTimer !== null) {
+      this.onlineStateTimer.cancel();
+      this.onlineStateTimer = null;
+    }
+  }
+}

packages/firestore/src/remote/persistent_stream.ts

@@ -265,17 +265,6 @@ export abstract class PersistentStream<
         IDLE_TIMEOUT_MS,
         () => this.handleIdleCloseTimer()
       );
-
-      this.inactivityTimerPromise.catch((err: FirestoreError) => {
-        // When the AsyncQueue gets drained during testing, pending Promises
-        // (including these idle checks) will get rejected. We special-case
-        // these cancelled idle checks to make sure that these specific Promise
-        // rejections are not considered unhandled.
-        assert(
-          err.code === Code.CANCELLED,
-          `Received unexpected error in idle timeout closure. Expected CANCELLED, but was: ${err}`
-        );
-      });
     }
   }
 
@@ -529,7 +518,7 @@ export class PersistentListenStream extends PersistentStream<
   ) {
     super(
       queue,
-      TimerId.ListenStreamConnection,
+      TimerId.ListenStreamConnectionBackoff,
       TimerId.ListenStreamIdle,
       connection,
       credentials
@@ -637,7 +626,7 @@ export class PersistentWriteStream extends PersistentStream<
   ) {
     super(
       queue,
-      TimerId.WriteStreamConnection,
+      TimerId.WriteStreamConnectionBackoff,
       TimerId.WriteStreamIdle,
       connection,
       credentials