From 0852c5d8b29f9d6c094e8604cd99575541aa0572 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Thu, 7 May 2020 20:57:27 -0700
Subject: [PATCH 1/5] IndexedDB recovery for waitForPendingWrites()

---
 packages/firestore/src/core/event_manager.ts  | 69 +++++++++----------
 packages/firestore/src/core/sync_engine.ts    | 63 +++++++++--------
 .../src/local/lru_garbage_collector.ts        | 28 ++++----
 packages/firestore/src/util/async_queue.ts    | 20 ++++++
 4 files changed, 99 insertions(+), 81 deletions(-)

diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts
index 7bf88268ce6..43f304cd184 100644
--- a/packages/firestore/src/core/event_manager.ts
+++ b/packages/firestore/src/core/event_manager.ts
@@ -24,6 +24,7 @@ import { OnlineState } from './types';
 import { ChangeType, DocumentViewChange, ViewSnapshot } from './view_snapshot';
 import { logError } from '../util/log';
 import { Code, FirestoreError } from '../util/error';
+import { executeWithIndexedDbRecovery } from '../util/async_queue';
 
 const LOG_TAG = 'EventManager';
 
@@ -62,47 +63,45 @@ export class EventManager implements SyncEngineListener {
     this.syncEngine.subscribe(this);
   }
 
-  async listen(listener: QueryListener): Promise<void> {
-    const query = listener.query;
-    let firstListen = false;
+  listen(listener: QueryListener): Promise<void> {
+    return executeWithIndexedDbRecovery(
+      async () => {
+        const query = listener.query;
+        let firstListen = false;
 
-    let queryInfo = this.queries.get(query);
-    if (!queryInfo) {
-      firstListen = true;
-      queryInfo = new QueryListenersInfo();
-    }
+        let queryInfo = this.queries.get(query);
+        if (!queryInfo) {
+          firstListen = true;
+          queryInfo = new QueryListenersInfo();
+        }
 
-    if (firstListen) {
-      try {
-        queryInfo.viewSnap = await this.syncEngine.listen(query);
-      } catch (e) {
-        const msg = `Initialization of query '${query}' failed: ${e}`;
-        logError(LOG_TAG, msg);
-        if (e.name === 'IndexedDbTransactionError') {
-          listener.onError(new FirestoreError(Code.UNAVAILABLE, msg));
-        } else {
-          throw e;
+        if (firstListen) {
+          queryInfo.viewSnap = await this.syncEngine.listen(query);
         }
-        return;
-      }
-    }
 
-    this.queries.set(query, queryInfo);
-    queryInfo.listeners.push(listener);
+        this.queries.set(query, queryInfo);
+        queryInfo.listeners.push(listener);
 
-    // Run global snapshot listeners if a consistent snapshot has been emitted.
-    const raisedEvent = listener.applyOnlineStateChange(this.onlineState);
-    debugAssert(
-      !raisedEvent,
-      "applyOnlineStateChange() shouldn't raise an event for brand-new listeners."
-    );
-
-    if (queryInfo.viewSnap) {
-      const raisedEvent = listener.onViewSnapshot(queryInfo.viewSnap);
-      if (raisedEvent) {
-        this.raiseSnapshotsInSyncEvent();
+        // Run global snapshot listeners if a consistent snapshot has been emitted.
+        const raisedEvent = listener.applyOnlineStateChange(this.onlineState);
+        debugAssert(
+          !raisedEvent,
+          "applyOnlineStateChange() shouldn't raise an event for brand-new listeners."
+        );
+
+        if (queryInfo.viewSnap) {
+          const raisedEvent = listener.onViewSnapshot(queryInfo.viewSnap);
+          if (raisedEvent) {
+            this.raiseSnapshotsInSyncEvent();
+          }
+        }
+      },
+      e => {
+        const msg = `Initialization of query '${listener.query}' failed: ${e}`;
+        logError(LOG_TAG, msg);
+        listener.onError(new FirestoreError(Code.UNAVAILABLE, msg));
       }
-    }
+    );
   }
 
   async unlisten(listener: QueryListener): Promise<void> {
diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts
index acfe6945d57..f90c1fbfc69 100644
--- a/packages/firestore/src/core/sync_engine.ts
+++ b/packages/firestore/src/core/sync_engine.ts
@@ -73,7 +73,7 @@ import {
   ViewDocumentChanges
 } from './view';
 import { ViewSnapshot } from './view_snapshot';
-import { AsyncQueue } from '../util/async_queue';
+import { AsyncQueue, executeWithIndexedDbRecovery } from '../util/async_queue';
 import { TransactionRunner } from './transaction_runner';
 
 const LOG_TAG = 'SyncEngine';
@@ -350,30 +350,25 @@ export class SyncEngine implements RemoteSyncer {
    * userCallback is resolved once the write was acked/rejected by the
    * backend (or failed locally for any other reason).
    */
-  async write(batch: Mutation[], userCallback: Deferred<void>): Promise<void> {
+   write(batch: Mutation[], userCallback: Deferred<void>): Promise<void> {
     this.assertSubscribed('write()');
 
-    let result: LocalWriteResult;
-    try {
-      result = await this.localStore.localWrite(batch);
-    } catch (e) {
-      if (e.name === 'IndexedDbTransactionError') {
+    return executeWithIndexedDbRecovery(
+      async () => {
+        const result = await this.localStore.localWrite(batch);
+        this.sharedClientState.addPendingMutation(result.batchId);
+        this.addMutationCallback(result.batchId, userCallback);
+        await this.emitNewSnapsAndNotifyLocalStore(result.changes);
+        await this.remoteStore.fillWritePipeline();
+      },
+      e => {
         // If we can't persist the mutation, we reject the user callback and
         // don't send the mutation. The user can then retry the write.
-        logError(LOG_TAG, 'Dropping write that cannot be persisted: ' + e);
-        userCallback.reject(
-          new FirestoreError(Code.UNAVAILABLE, 'Failed to persist write: ' + e)
-        );
-        return;
-      } else {
-        throw e;
+        const msg = `Failed to persist write: ${e}`;
+        logError(LOG_TAG, msg);
+        userCallback.reject(new FirestoreError(Code.UNAVAILABLE, msg));
       }
-    }
-
-    this.sharedClientState.addPendingMutation(result.batchId);
-    this.addMutationCallback(result.batchId, userCallback);
-    await this.emitNewSnapsAndNotifyLocalStore(result.changes);
-    await this.remoteStore.fillWritePipeline();
+    );
   }
 
   /**
@@ -584,16 +579,24 @@ export class SyncEngine implements RemoteSyncer {
       );
     }
 
-    const highestBatchId = await this.localStore.getHighestUnacknowledgedBatchId();
-    if (highestBatchId === BATCHID_UNKNOWN) {
-      // Trigger the callback right away if there is no pending writes at the moment.
-      callback.resolve();
-      return;
-    }
-
-    const callbacks = this.pendingWritesCallbacks.get(highestBatchId) || [];
-    callbacks.push(callback);
-    this.pendingWritesCallbacks.set(highestBatchId, callbacks);
+    return executeWithIndexedDbRecovery(
+      async () => {
+        const highestBatchId = await this.localStore.getHighestUnacknowledgedBatchId();
+        if (highestBatchId === BATCHID_UNKNOWN) {
+          // Trigger the callback right away if there is no pending writes at the moment.
+          callback.resolve();
+          return;
+        }
+        const callbacks = this.pendingWritesCallbacks.get(highestBatchId) || [];
+        callbacks.push(callback);
+        this.pendingWritesCallbacks.set(highestBatchId, callbacks);
+      },
+      e => {
+        const msg = `Initialization of waitForPendingWrites() operation failed: ${e}`;
+        logError(LOG_TAG, msg);
+        callback.reject(new FirestoreError(Code.UNAVAILABLE, msg));
+      }
+    );
   }
 
   /**
diff --git a/packages/firestore/src/local/lru_garbage_collector.ts b/packages/firestore/src/local/lru_garbage_collector.ts
index 753c7a4af6f..4e865b44d70 100644
--- a/packages/firestore/src/local/lru_garbage_collector.ts
+++ b/packages/firestore/src/local/lru_garbage_collector.ts
@@ -18,7 +18,11 @@
 import { ListenSequence } from '../core/listen_sequence';
 import { ListenSequenceNumber, TargetId } from '../core/types';
 import { debugAssert } from '../util/assert';
-import { AsyncQueue, TimerId } from '../util/async_queue';
+import {
+  AsyncQueue,
+  TimerId,
+  executeWithIndexedDbRecovery
+} from '../util/async_queue';
 import { getLogLevel, logDebug, LogLevel } from '../util/log';
 import { primitiveComparator } from '../util/misc';
 import { CancelablePromise } from '../util/promise';
@@ -267,23 +271,15 @@ export class LruScheduler implements GarbageCollectionScheduler {
     this.gcTask = this.asyncQueue.enqueueAfterDelay(
       TimerId.LruGarbageCollection,
       delay,
-      async () => {
+      () => {
         this.gcTask = null;
         this.hasRun = true;
-        try {
-          await localStore.collectGarbage(this.garbageCollector);
-        } catch (e) {
-          if (e.name === 'IndexedDbTransactionError') {
-            logDebug(
-              LOG_TAG,
-              'Ignoring IndexedDB error during garbage collection: ',
-              e
-            );
-          } else {
-            await ignoreIfPrimaryLeaseLoss(e);
-          }
-        }
-        await this.scheduleGC(localStore);
+        return executeWithIndexedDbRecovery(
+          () => localStore.collectGarbage(this.garbageCollector).then(() => {}),
+          /* recoveryHandler= */ () => {}
+        )
+          .then(() => this.scheduleGC(localStore))
+          .catch(ignoreIfPrimaryLeaseLoss);
       }
     );
   }
diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts
index 8fbbc4f5208..77105ac6490 100644
--- a/packages/firestore/src/util/async_queue.ts
+++ b/packages/firestore/src/util/async_queue.ts
@@ -21,6 +21,7 @@ import { logDebug, logError } from './log';
 import { CancelablePromise, Deferred } from './promise';
 import { ExponentialBackoff } from '../remote/backoff';
 import { PlatformSupport } from '../platform/platform';
+import { IndexedDbTransactionError } from '../local/simple_db';
 
 const LOG_TAG = 'AsyncQueue';
 
@@ -501,3 +502,22 @@ export class AsyncQueue {
     this.delayedOperations.splice(index, 1);
   }
 }
+
+/**
+ * Runs the provided `op`. If `op` fails with an `IndexedDbTransactionError`,
+ * calls `recoveryHandler` and returns a resolved Promise. If `op` is successful
+ * or fails with another type of error, returns op's result.
+ */
+export function executeWithIndexedDbRecovery<T>(
+  op: () => Promise<void>,
+  recoveryHandler: (e: IndexedDbTransactionError) => void
+): Promise<void> {
+  return op().catch(e => {
+    logDebug(LOG_TAG, 'Internal operation failed: ', e);
+    if (e.name === 'IndexedDbTransactionError') {
+      recoveryHandler(e);
+    } else {
+      throw e;
+    }
+  });
+}

From 8f47d7a48a2d2f292ded55d4a130dfcd39a2b2d1 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Thu, 7 May 2020 21:37:38 -0700
Subject: [PATCH 2/5] Lint

---
 packages/firestore/src/core/sync_engine.ts            | 1 -
 packages/firestore/src/local/lru_garbage_collector.ts | 2 --
 2 files changed, 3 deletions(-)

diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts
index f90c1fbfc69..e97466fd1f5 100644
--- a/packages/firestore/src/core/sync_engine.ts
+++ b/packages/firestore/src/core/sync_engine.ts
@@ -19,7 +19,6 @@ import { User } from '../auth/user';
 import {
   ignoreIfPrimaryLeaseLoss,
   LocalStore,
-  LocalWriteResult,
   MultiTabLocalStore
 } from '../local/local_store';
 import { LocalViewChanges } from '../local/local_view_changes';
diff --git a/packages/firestore/src/local/lru_garbage_collector.ts b/packages/firestore/src/local/lru_garbage_collector.ts
index 4e865b44d70..dee7680ade4 100644
--- a/packages/firestore/src/local/lru_garbage_collector.ts
+++ b/packages/firestore/src/local/lru_garbage_collector.ts
@@ -36,8 +36,6 @@ import {
 import { PersistencePromise } from './persistence_promise';
 import { TargetData } from './target_data';
 
-const LOG_TAG = 'LruGarbageCollector';
-
 /**
  * Persistence layers intending to use LRU Garbage collection should have reference delegates that
  * implement this interface. This interface defines the operations that the LRU garbage collector

From d68d8d1d904656d9af093adbfde76fe78e335001 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Wed, 13 May 2020 16:39:17 -0700
Subject: [PATCH 3/5] Review

---
 packages/firestore/src/core/event_manager.ts  | 72 +++++++++----------
 packages/firestore/src/core/sync_engine.ts    | 68 +++++++++---------
 .../src/local/lru_garbage_collector.ts        | 31 ++++----
 packages/firestore/src/local/simple_db.ts     |  7 ++
 packages/firestore/src/remote/remote_store.ts |  3 +-
 packages/firestore/src/util/async_queue.ts    | 31 ++++----
 6 files changed, 108 insertions(+), 104 deletions(-)

diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts
index 43f304cd184..22a210efe3f 100644
--- a/packages/firestore/src/core/event_manager.ts
+++ b/packages/firestore/src/core/event_manager.ts
@@ -22,11 +22,7 @@ import { Query } from './query';
 import { SyncEngine, SyncEngineListener } from './sync_engine';
 import { OnlineState } from './types';
 import { ChangeType, DocumentViewChange, ViewSnapshot } from './view_snapshot';
-import { logError } from '../util/log';
-import { Code, FirestoreError } from '../util/error';
-import { executeWithIndexedDbRecovery } from '../util/async_queue';
-
-const LOG_TAG = 'EventManager';
+import { wrapInUserErrorIfRecoverable } from '../util/async_queue';
 
 /**
  * Holds the listeners and the last received ViewSnapshot for a query being
@@ -63,45 +59,45 @@ export class EventManager implements SyncEngineListener {
     this.syncEngine.subscribe(this);
   }
 
-  listen(listener: QueryListener): Promise<void> {
-    return executeWithIndexedDbRecovery(
-      async () => {
-        const query = listener.query;
-        let firstListen = false;
+  async listen(listener: QueryListener): Promise<void> {
+    const query = listener.query;
+    let firstListen = false;
 
-        let queryInfo = this.queries.get(query);
-        if (!queryInfo) {
-          firstListen = true;
-          queryInfo = new QueryListenersInfo();
-        }
+    let queryInfo = this.queries.get(query);
+    if (!queryInfo) {
+      firstListen = true;
+      queryInfo = new QueryListenersInfo();
+    }
 
-        if (firstListen) {
-          queryInfo.viewSnap = await this.syncEngine.listen(query);
-        }
+    if (firstListen) {
+      try {
+        queryInfo.viewSnap = await this.syncEngine.listen(query);
+      } catch (e) {
+        const firestoreError = wrapInUserErrorIfRecoverable(
+          e,
+          `Initialization of query '${listener.query}' failed`
+        );
+        listener.onError(firestoreError);
+      }
+      return;
+    }
 
-        this.queries.set(query, queryInfo);
-        queryInfo.listeners.push(listener);
+    this.queries.set(query, queryInfo);
+    queryInfo.listeners.push(listener);
 
-        // Run global snapshot listeners if a consistent snapshot has been emitted.
-        const raisedEvent = listener.applyOnlineStateChange(this.onlineState);
-        debugAssert(
-          !raisedEvent,
-          "applyOnlineStateChange() shouldn't raise an event for brand-new listeners."
-        );
+    // Run global snapshot listeners if a consistent snapshot has been emitted.
+    const raisedEvent = listener.applyOnlineStateChange(this.onlineState);
+    debugAssert(
+      !raisedEvent,
+      "applyOnlineStateChange() shouldn't raise an event for brand-new listeners."
+    );
 
-        if (queryInfo.viewSnap) {
-          const raisedEvent = listener.onViewSnapshot(queryInfo.viewSnap);
-          if (raisedEvent) {
-            this.raiseSnapshotsInSyncEvent();
-          }
-        }
-      },
-      e => {
-        const msg = `Initialization of query '${listener.query}' failed: ${e}`;
-        logError(LOG_TAG, msg);
-        listener.onError(new FirestoreError(Code.UNAVAILABLE, msg));
+    if (queryInfo.viewSnap) {
+      const raisedEvent = listener.onViewSnapshot(queryInfo.viewSnap);
+      if (raisedEvent) {
+        this.raiseSnapshotsInSyncEvent();
       }
-    );
+    }
   }
 
   async unlisten(listener: QueryListener): Promise<void> {
diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts
index b831406a2a5..7792e779334 100644
--- a/packages/firestore/src/core/sync_engine.ts
+++ b/packages/firestore/src/core/sync_engine.ts
@@ -38,7 +38,7 @@ import { RemoteStore } from '../remote/remote_store';
 import { RemoteSyncer } from '../remote/remote_syncer';
 import { debugAssert, fail, hardAssert } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
-import { logDebug, logError } from '../util/log';
+import { logDebug } from '../util/log';
 import { primitiveComparator } from '../util/misc';
 import { ObjectMap } from '../util/obj_map';
 import { Deferred } from '../util/promise';
@@ -72,7 +72,7 @@ import {
   ViewDocumentChanges
 } from './view';
 import { ViewSnapshot } from './view_snapshot';
-import { AsyncQueue, executeWithIndexedDbRecovery } from '../util/async_queue';
+import { AsyncQueue, wrapInUserErrorIfRecoverable } from '../util/async_queue';
 import { TransactionRunner } from './transaction_runner';
 
 const LOG_TAG = 'SyncEngine';
@@ -349,25 +349,21 @@ export class SyncEngine implements RemoteSyncer {
    * userCallback is resolved once the write was acked/rejected by the
    * backend (or failed locally for any other reason).
    */
-   write(batch: Mutation[], userCallback: Deferred<void>): Promise<void> {
+  async write(batch: Mutation[], userCallback: Deferred<void>): Promise<void> {
     this.assertSubscribed('write()');
 
-    return executeWithIndexedDbRecovery(
-      async () => {
-        const result = await this.localStore.localWrite(batch);
-        this.sharedClientState.addPendingMutation(result.batchId);
-        this.addMutationCallback(result.batchId, userCallback);
-        await this.emitNewSnapsAndNotifyLocalStore(result.changes);
-        await this.remoteStore.fillWritePipeline();
-      },
-      e => {
-        // If we can't persist the mutation, we reject the user callback and
-        // don't send the mutation. The user can then retry the write.
-        const msg = `Failed to persist write: ${e}`;
-        logError(LOG_TAG, msg);
-        userCallback.reject(new FirestoreError(Code.UNAVAILABLE, msg));
-      }
-    );
+    try {
+      const result = await this.localStore.localWrite(batch);
+      this.sharedClientState.addPendingMutation(result.batchId);
+      this.addMutationCallback(result.batchId, userCallback);
+      await this.emitNewSnapsAndNotifyLocalStore(result.changes);
+      await this.remoteStore.fillWritePipeline();
+    } catch (e) {
+      // If we can't persist the mutation, we reject the user callback and
+      // don't send the mutation. The user can then retry the write.
+      const error = wrapInUserErrorIfRecoverable(e, `Failed to persist write`);
+      userCallback.reject(error);
+    }
   }
 
   /**
@@ -582,24 +578,24 @@ export class SyncEngine implements RemoteSyncer {
       );
     }
 
-    return executeWithIndexedDbRecovery(
-      async () => {
-        const highestBatchId = await this.localStore.getHighestUnacknowledgedBatchId();
-        if (highestBatchId === BATCHID_UNKNOWN) {
-          // Trigger the callback right away if there is no pending writes at the moment.
-          callback.resolve();
-          return;
-        }
-        const callbacks = this.pendingWritesCallbacks.get(highestBatchId) || [];
-        callbacks.push(callback);
-        this.pendingWritesCallbacks.set(highestBatchId, callbacks);
-      },
-      e => {
-        const msg = `Initialization of waitForPendingWrites() operation failed: ${e}`;
-        logError(LOG_TAG, msg);
-        callback.reject(new FirestoreError(Code.UNAVAILABLE, msg));
+    try {
+      const highestBatchId = await this.localStore.getHighestUnacknowledgedBatchId();
+      if (highestBatchId === BATCHID_UNKNOWN) {
+        // Trigger the callback right away if there is no pending writes at the moment.
+        callback.resolve();
+        return;
       }
-    );
+
+      const callbacks = this.pendingWritesCallbacks.get(highestBatchId) || [];
+      callbacks.push(callback);
+      this.pendingWritesCallbacks.set(highestBatchId, callbacks);
+    } catch (e) {
+      const firestoreError = wrapInUserErrorIfRecoverable(
+        e,
+        'Initialization of waitForPendingWrites() operation failed'
+      );
+      callback.reject(firestoreError);
+    }
   }
 
   /**
diff --git a/packages/firestore/src/local/lru_garbage_collector.ts b/packages/firestore/src/local/lru_garbage_collector.ts
index dee7680ade4..e303780fcbc 100644
--- a/packages/firestore/src/local/lru_garbage_collector.ts
+++ b/packages/firestore/src/local/lru_garbage_collector.ts
@@ -18,11 +18,7 @@
 import { ListenSequence } from '../core/listen_sequence';
 import { ListenSequenceNumber, TargetId } from '../core/types';
 import { debugAssert } from '../util/assert';
-import {
-  AsyncQueue,
-  TimerId,
-  executeWithIndexedDbRecovery
-} from '../util/async_queue';
+import { AsyncQueue, TimerId } from '../util/async_queue';
 import { getLogLevel, logDebug, LogLevel } from '../util/log';
 import { primitiveComparator } from '../util/misc';
 import { CancelablePromise } from '../util/promise';
@@ -35,6 +31,9 @@ import {
 } from './persistence';
 import { PersistencePromise } from './persistence_promise';
 import { TargetData } from './target_data';
+import { isIndexedDbTransactionError } from './simple_db';
+
+const LOG_TAG = 'LruGarbageCollector';
 
 /**
  * Persistence layers intending to use LRU Garbage collection should have reference delegates that
@@ -269,15 +268,23 @@ export class LruScheduler implements GarbageCollectionScheduler {
     this.gcTask = this.asyncQueue.enqueueAfterDelay(
       TimerId.LruGarbageCollection,
       delay,
-      () => {
+      async () => {
         this.gcTask = null;
         this.hasRun = true;
-        return executeWithIndexedDbRecovery(
-          () => localStore.collectGarbage(this.garbageCollector).then(() => {}),
-          /* recoveryHandler= */ () => {}
-        )
-          .then(() => this.scheduleGC(localStore))
-          .catch(ignoreIfPrimaryLeaseLoss);
+        try {
+          await localStore.collectGarbage(this.garbageCollector);
+        } catch (e) {
+          if (isIndexedDbTransactionError(e)) {
+            logDebug(
+              LOG_TAG,
+              'Ignoring IndexedDB error during garbage collection: ',
+              e
+            );
+          } else {
+            await ignoreIfPrimaryLeaseLoss(e);
+          }
+        }
+        await this.scheduleGC(localStore);
       }
     );
   }
diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts
index 5c71d6d8370..974d720095a 100644
--- a/packages/firestore/src/local/simple_db.ts
+++ b/packages/firestore/src/local/simple_db.ts
@@ -418,6 +418,13 @@ export class IndexedDbTransactionError extends FirestoreError {
   }
 }
 
+/** Verifies whether `e` is an IndexedDbTransactionError. */
+export function isIndexedDbTransactionError(e: Error): boolean {
+  // Use name equality, as instanceof checks on errors don't work with errors
+  // that wrap other errors.
+  return e.name === 'IndexedDbTransactionError';
+}
+
 /**
  * Wraps an IDBTransaction and exposes a store() method to get a handle to a
  * specific object store.
diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts
index 050617e472b..4921520801a 100644
--- a/packages/firestore/src/remote/remote_store.ts
+++ b/packages/firestore/src/remote/remote_store.ts
@@ -54,6 +54,7 @@ import {
   WatchTargetChangeState
 } from './watch_change';
 import { ByteString } from '../util/byte_string';
+import { isIndexedDbTransactionError } from '../local/simple_db';
 
 const LOG_TAG = 'RemoteStore';
 
@@ -448,7 +449,7 @@ export class RemoteStore implements TargetMetadataProvider {
    * `enqueueRetryable()`.
    */
   private async disableNetworkUntilRecovery(e: FirestoreError): Promise<void> {
-    if (e.name === 'IndexedDbTransactionError') {
+    if (isIndexedDbTransactionError(e)) {
       debugAssert(
         !this.indexedDbFailed,
         'Unexpected network event when IndexedDB was marked failed.'
diff --git a/packages/firestore/src/util/async_queue.ts b/packages/firestore/src/util/async_queue.ts
index 77105ac6490..2c8653172cf 100644
--- a/packages/firestore/src/util/async_queue.ts
+++ b/packages/firestore/src/util/async_queue.ts
@@ -21,7 +21,7 @@ import { logDebug, logError } from './log';
 import { CancelablePromise, Deferred } from './promise';
 import { ExponentialBackoff } from '../remote/backoff';
 import { PlatformSupport } from '../platform/platform';
-import { IndexedDbTransactionError } from '../local/simple_db';
+import { isIndexedDbTransactionError } from '../local/simple_db';
 
 const LOG_TAG = 'AsyncQueue';
 
@@ -339,7 +339,7 @@ export class AsyncQueue {
           deferred.resolve();
           this.backoff.reset();
         } catch (e) {
-          if (e.name === 'IndexedDbTransactionError') {
+          if (isIndexedDbTransactionError(e)) {
             logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
             this.backoff.backoffAndRun(retryingOp);
           } else {
@@ -504,20 +504,17 @@ export class AsyncQueue {
 }
 
 /**
- * Runs the provided `op`. If `op` fails with an `IndexedDbTransactionError`,
- * calls `recoveryHandler` and returns a resolved Promise. If `op` is successful
- * or fails with another type of error, returns op's result.
+ * Returns a FirestoreError that can be surfaced to the user if the provided
+ * error is an IndexedDbTransactionError. Re-throws the error otherwise.
  */
-export function executeWithIndexedDbRecovery<T>(
-  op: () => Promise<void>,
-  recoveryHandler: (e: IndexedDbTransactionError) => void
-): Promise<void> {
-  return op().catch(e => {
-    logDebug(LOG_TAG, 'Internal operation failed: ', e);
-    if (e.name === 'IndexedDbTransactionError') {
-      recoveryHandler(e);
-    } else {
-      throw e;
-    }
-  });
+export function wrapInUserErrorIfRecoverable(
+  e: Error,
+  msg: string
+): FirestoreError {
+  logError(LOG_TAG, `${msg}: ${e}`);
+  if (isIndexedDbTransactionError(e)) {
+    return new FirestoreError(Code.UNAVAILABLE, `${msg}: ${e}`);
+  } else {
+    throw e;
+  }
 }

From 001cf9c699d4148a52a6d9032e42815af5da5472 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Wed, 13 May 2020 17:09:01 -0700
Subject: [PATCH 4/5] Fix test

---
 packages/firestore/src/core/event_manager.ts           | 2 +-
 packages/firestore/test/unit/specs/listen_spec.test.ts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts
index 22a210efe3f..12ccbfd889b 100644
--- a/packages/firestore/src/core/event_manager.ts
+++ b/packages/firestore/src/core/event_manager.ts
@@ -78,8 +78,8 @@ export class EventManager implements SyncEngineListener {
           `Initialization of query '${listener.query}' failed`
         );
         listener.onError(firestoreError);
+        return;
       }
-      return;
     }
 
     this.queries.set(query, queryInfo);
diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts
index 8062e8aae45..5845e4e6f54 100644
--- a/packages/firestore/test/unit/specs/listen_spec.test.ts
+++ b/packages/firestore/test/unit/specs/listen_spec.test.ts
@@ -47,7 +47,7 @@ describeSpec('Listens:', [], () => {
 
   specTest(
     'Documents outside of view are cleared when listen is removed.',
-    ['eager-gc', 'exclusive'],
+    ['eager-gc'],
     '',
     () => {
       const filteredQuery = Query.atPath(path('collection')).addFilter(

From 8bc241f181903b19b8ea82ed0ebe5585932b1b78 Mon Sep 17 00:00:00 2001
From: Sebastian Schmidt <mrschmidt@google.com>
Date: Thu, 14 May 2020 10:52:24 -0700
Subject: [PATCH 5/5] Fix tests after merge

---
 packages/firestore/src/local/indexeddb_persistence.ts | 11 ++++++++---
 packages/firestore/src/local/local_store.ts           |  3 ++-
 .../test/unit/local/indexeddb_persistence.test.ts     |  8 ++++----
 .../firestore/test/unit/specs/spec_test_components.ts |  4 ++--
 .../firestore/test/unit/specs/spec_test_runner.ts     |  3 ++-
 5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts
index 08f1dc63921..f162746c7df 100644
--- a/packages/firestore/src/local/indexeddb_persistence.ts
+++ b/packages/firestore/src/local/indexeddb_persistence.ts
@@ -72,7 +72,12 @@ import {
 import { PersistencePromise } from './persistence_promise';
 import { ClientId } from './shared_client_state';
 import { TargetData } from './target_data';
-import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db';
+import {
+  isIndexedDbTransactionError,
+  SimpleDb,
+  SimpleDbStore,
+  SimpleDbTransaction
+} from './simple_db';
 
 const LOG_TAG = 'IndexedDbPersistence';
 
@@ -287,7 +292,7 @@ export class IndexedDbPersistence implements Persistence {
         // having the persistence lock), so it's the first thing we should do.
         return this.updateClientMetadataAndTryBecomePrimary();
       })
-      .then(e => {
+      .then(() => {
         if (!this.isPrimary && !this.allowTabSynchronization) {
           // Fail `start()` if `synchronizeTabs` is disabled and we cannot
           // obtain the primary lease.
@@ -423,7 +428,7 @@ export class IndexedDbPersistence implements Persistence {
     )
       .catch(e => {
         if (!this.allowTabSynchronization) {
-          if (e.name === 'IndexedDbTransactionError') {
+          if (isIndexedDbTransactionError(e)) {
             logDebug(LOG_TAG, 'Failed to extend owner lease: ', e);
             // Proceed with the existing state. Any subsequent access to
             // IndexedDB will verify the lease.
diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts
index ac81fa209e1..361cbd48b7f 100644
--- a/packages/firestore/src/local/local_store.ts
+++ b/packages/firestore/src/local/local_store.ts
@@ -66,6 +66,7 @@ import { IndexedDbMutationQueue } from './indexeddb_mutation_queue';
 import { IndexedDbRemoteDocumentCache } from './indexeddb_remote_document_cache';
 import { IndexedDbTargetCache } from './indexeddb_target_cache';
 import { extractFieldMask } from '../model/object_value';
+import { isIndexedDbTransactionError } from './simple_db';
 
 const LOG_TAG = 'LocalStore';
 
@@ -727,7 +728,7 @@ export class LocalStore {
         }
       );
     } catch (e) {
-      if (e.name === 'IndexedDbTransactionError') {
+      if (isIndexedDbTransactionError(e)) {
         // If `notifyLocalViewChanges` fails, we did not advance the sequence
         // number for the documents that were included in this transaction.
         // This might trigger them to be deleted earlier than they otherwise
diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts
index 6a7bfcf8df2..37d6a8ae29d 100644
--- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts
+++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts
@@ -1144,7 +1144,7 @@ describe('IndexedDb: allowTabSynchronization', () => {
       'clientA',
       /* multiClient= */ false,
       async db => {
-        db.injectFailures = true;
+        db.injectFailures = { updateClientMetadataAndTryBecomePrimary: true };
         await expect(db.start()).to.eventually.be.rejectedWith(
           'Failed to obtain exclusive access to the persistence layer.'
         );
@@ -1158,7 +1158,7 @@ describe('IndexedDb: allowTabSynchronization', () => {
       'clientA',
       /* multiClient= */ true,
       async db => {
-        db.injectFailures = true;
+        db.injectFailures = { updateClientMetadataAndTryBecomePrimary: true };
         await db.start();
         await db.shutdown();
       }
@@ -1167,10 +1167,10 @@ describe('IndexedDb: allowTabSynchronization', () => {
 
   it('ignores intermittent IndexedDbTransactionError during lease refresh', async () => {
     await withPersistence('clientA', async (db, _, queue) => {
-      db.injectFailures = true;
+      db.injectFailures = { updateClientMetadataAndTryBecomePrimary: true };
       await queue.runDelayedOperationsEarly(TimerId.ClientMetadataRefresh);
       await queue.enqueue(() => {
-        db.injectFailures = false;
+        db.injectFailures = undefined;
         return db.runTransaction('check success', 'readwrite-primary', () =>
           PersistencePromise.resolve()
         );
diff --git a/packages/firestore/test/unit/specs/spec_test_components.ts b/packages/firestore/test/unit/specs/spec_test_components.ts
index 396410e9da5..dc0fecc0164 100644
--- a/packages/firestore/test/unit/specs/spec_test_components.ts
+++ b/packages/firestore/test/unit/specs/spec_test_components.ts
@@ -59,7 +59,7 @@ import { expect } from 'chai';
 export class MockMemoryPersistence extends MemoryPersistence {
   injectFailures?: SpecDatabaseFailures;
 
-  runTransaction<T>(
+  async runTransaction<T>(
     action: string,
     mode: PersistenceTransactionMode,
     transactionOperation: (
@@ -78,7 +78,7 @@ export class MockMemoryPersistence extends MemoryPersistence {
 export class MockIndexedDbPersistence extends IndexedDbPersistence {
   injectFailures?: SpecDatabaseFailures;
 
-  runTransaction<T>(
+  async runTransaction<T>(
     action: string,
     mode: PersistenceTransactionMode,
     transactionOperation: (
diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts
index 805b52a88ff..c1b82633b6e 100644
--- a/packages/firestore/test/unit/specs/spec_test_runner.ts
+++ b/packages/firestore/test/unit/specs/spec_test_runner.ts
@@ -1214,7 +1214,8 @@ export type PersistenceAction =
   | 'Lookup mutation documents'
   | 'Get target data'
   | 'Get new document changes'
-  | 'Synchronize last document change read time';
+  | 'Synchronize last document change read time'
+  | 'updateClientMetadataAndTryBecomePrimary';
 
 /** Specifies failure or success for a list of database actions. */
 export type SpecDatabaseFailures = Partial<