Add transaction retries

packages/firestore/src/local/indexeddb_persistence.ts

@@ -121,6 +121,15 @@ export class IndexedDbTransaction extends PersistenceTransaction {
   }
 }
 
+// The different modes supported by `IndexedDbPersistence.runTransaction()`
+type TransactionMode =
+  | 'readonly'
+  | 'readwrite'
+  | 'readwrite-primary'
+  | 'readonly-idempotent'
+  | 'readwrite-idempotent'
+  | 'readwrite-primary-idempotent';
+
 /**
  * An IndexedDB-backed instance of Persistence. Data is stored persistently
  * across sessions.
@@ -321,6 +330,7 @@ export class IndexedDbPersistence implements Persistence {
       .then(() => {
         return this.simpleDb.runTransaction(
           'readonly',
+          /* idempotent= */ false,
           [DbTargetGlobal.store],
           txn => {
             return getHighestListenSequenceNumber(txn).next(
@@ -344,8 +354,11 @@ export class IndexedDbPersistence implements Persistence {
   }
 
   private startRemoteDocumentCache(): Promise<void> {
-    return this.simpleDb.runTransaction('readonly', ALL_STORES, txn =>
-      this.remoteDocumentCache.start(txn)
+    return this.simpleDb.runTransaction(
+      'readonly',
+      /* idempotent= */ false,
+      ALL_STORES,
+      txn => this.remoteDocumentCache.start(txn)
     );
   }
 
@@ -391,47 +404,52 @@ export class IndexedDbPersistence implements Persistence {
    * primary lease.
    */
   private updateClientMetadataAndTryBecomePrimary(): Promise<void> {
-    return this.simpleDb.runTransaction('readwrite', ALL_STORES, txn => {
-      const metadataStore = clientMetadataStore(txn);
-      return metadataStore
-        .put(
-          new DbClientMetadata(
-            this.clientId,
-            Date.now(),
-            this.networkEnabled,
-            this.inForeground
+    return this.simpleDb.runTransaction(
+      'readwrite',
+      /* idempotent= */ false,
+      ALL_STORES,
+      txn => {
+        const metadataStore = clientMetadataStore(txn);
+        return metadataStore
+          .put(
+            new DbClientMetadata(
+              this.clientId,
+              Date.now(),
+              this.networkEnabled,
+              this.inForeground
+            )
           )
-        )
-        .next(() => {
-          if (this.isPrimary) {
-            return this.verifyPrimaryLease(txn).next(success => {
-              if (!success) {
-                this.isPrimary = false;
-                this.queue.enqueueAndForget(() =>
-                  this.primaryStateListener(false)
-                );
-              }
-            });
-          }
-        })
-        .next(() => this.canActAsPrimary(txn))
-        .next(canActAsPrimary => {
-          const wasPrimary = this.isPrimary;
-          this.isPrimary = canActAsPrimary;
-
-          if (wasPrimary !== this.isPrimary) {
-            this.queue.enqueueAndForget(() =>
-              this.primaryStateListener(this.isPrimary)
-            );
-          }
+          .next(() => {
+            if (this.isPrimary) {
+              return this.verifyPrimaryLease(txn).next(success => {
+                if (!success) {
+                  this.isPrimary = false;
+                  this.queue.enqueueAndForget(() =>
+                    this.primaryStateListener(false)
+                  );
+                }
+              });
+            }
+          })
+          .next(() => this.canActAsPrimary(txn))
+          .next(canActAsPrimary => {
+            const wasPrimary = this.isPrimary;
+            this.isPrimary = canActAsPrimary;
+
+            if (wasPrimary !== this.isPrimary) {
+              this.queue.enqueueAndForget(() =>
+                this.primaryStateListener(this.isPrimary)
+              );
+            }
 
-          if (wasPrimary && !this.isPrimary) {
-            return this.releasePrimaryLeaseIfHeld(txn);
-          } else if (this.isPrimary) {
-            return this.acquireOrExtendPrimaryLease(txn);
-          }
-        });
-    });
+            if (wasPrimary && !this.isPrimary) {
+              return this.releasePrimaryLeaseIfHeld(txn);
+            } else if (this.isPrimary) {
+              return this.acquireOrExtendPrimaryLease(txn);
+            }
+          });
+      }
+    );
   }
 
   private verifyPrimaryLease(
@@ -646,6 +664,7 @@ export class IndexedDbPersistence implements Persistence {
     this.detachWindowUnloadHook();
     await this.simpleDb.runTransaction(
       'readwrite',
+      /* idempotent= */ false,
       [DbPrimaryClient.store, DbClientMetadata.store],
       txn => {
         return this.releasePrimaryLeaseIfHeld(txn).next(() =>
@@ -678,6 +697,7 @@ export class IndexedDbPersistence implements Persistence {
   getActiveClients(): Promise<ClientId[]> {
     return this.simpleDb.runTransaction(
       'readonly',
+      /* idempotent= */ false,
       [DbClientMetadata.store],
       txn => {
         return clientMetadataStore(txn)
@@ -742,17 +762,26 @@ export class IndexedDbPersistence implements Persistence {
 
   runTransaction<T>(
     action: string,
-    mode: 'readonly' | 'readwrite' | 'readwrite-primary',
+    mode: TransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
     log.debug(LOG_TAG, 'Starting transaction:', action);
 
+    const idempotent =
+      [
+        'readonly-idempotent',
+        'readwrite-idempotent',
+        'readwrite-primary-idempotent'
+      ].indexOf(mode) !== -1;
+    const readonly = ['readonly', 'readonly-idempotent'].indexOf(mode) !== -1;
+
     // Do all transactions as readwrite against all object stores, since we
     // are the only reader/writer.
     return this.simpleDb.runTransaction(
-      mode === 'readonly' ? 'readonly' : 'readwrite',
+      readonly ? 'readonly' : 'readwrite',
+      idempotent,
       ALL_STORES,
       simpleDbTxn => {
         if (mode === 'readwrite-primary') {

packages/firestore/src/local/simple_db.ts

@@ -25,6 +25,12 @@ import { PersistencePromise } from './persistence_promise';
 
 const LOG_TAG = 'SimpleDb';
 
+/**
+ * The maximum number of retry attempts for an IndexedDb transaction that fails
+ * with a DOMException.
+ */
+const TRANSACTION_RETRY_COUNT = 3;
+
 export interface SimpleDbSchemaConverter {
   createOrUpgrade(
     db: IDBDatabase,
@@ -242,32 +248,58 @@ export class SimpleDb {
     };
   }
 
-  runTransaction<T>(
+  async runTransaction<T>(
     mode: 'readonly' | 'readwrite',
+    idempotent: boolean,
     objectStores: string[],
     transactionFn: (transaction: SimpleDbTransaction) => PersistencePromise<T>
   ): Promise<T> {
-    const transaction = SimpleDbTransaction.open(this.db, mode, objectStores);
-    const transactionFnResult = transactionFn(transaction)
-      .catch(error => {
-        // Abort the transaction if there was an error.
-        transaction.abort(error);
-        // We cannot actually recover, and calling `abort()` will cause the transaction's
-        // completion promise to be rejected. This in turn means that we won't use
-        // `transactionFnResult` below. We return a rejection here so that we don't add the
-        // possibility of returning `void` to the type of `transactionFnResult`.
-        return PersistencePromise.reject<T>(error);
-      })
-      .toPromise();
-
-    // As noted above, errors are propagated by aborting the transaction. So
-    // we swallow any error here to avoid the browser logging it as unhandled.
-    transactionFnResult.catch(() => {});
-
-    // Wait for the transaction to complete (i.e. IndexedDb's onsuccess event to
-    // fire), but still return the original transactionFnResult back to the
-    // caller.
-    return transaction.completionPromise.then(() => transactionFnResult);
+    let attemptNumber = 0;
+
+    while (true) {
+      ++attemptNumber;
+
+      const transaction = SimpleDbTransaction.open(this.db, mode, objectStores);
+      try {
+        const transactionFnResult = transactionFn(transaction)
+          .catch(error => {
+            // Abort the transaction if there was an error.
+            transaction.abort(error);
+            // We cannot actually recover, and calling `abort()` will cause the transaction's
+            // completion promise to be rejected. This in turn means that we won't use
+            // `transactionFnResult` below. We return a rejection here so that we don't add the
+            // possibility of returning `void` to the type of `transactionFnResult`.
+            return PersistencePromise.reject<T>(error);
+          })
+          .toPromise();
+
+        // As noted above, errors are propagated by aborting the transaction. So
+        // we swallow any error here to avoid the browser logging it as unhandled.
+        transactionFnResult.catch(() => {});
+
+        // Wait for the transaction to complete (i.e. IndexedDb's onsuccess event to
+        // fire), but still return the original transactionFnResult back to the
+        // caller.
+        await transaction.completionPromise;
+        return transactionFnResult;
+      } catch (e) {
+        // TODO: We could probably be smarter about this an not retry exceptions
+        // that are likely unrecoverable (such as quota exceeded errors).
+        const retryable =
+          idempotent &&
+          isDomException(e) &&
+          attemptNumber < TRANSACTION_RETRY_COUNT;
+        debug(
+          'Transaction failed with error: %s. Retrying: %s.',
+          e.message,
+          retryable
+        );
+
+        if (!retryable) {
+          return Promise.reject(e);
+        }
+      }
+    }
   }
 
   close(): void {
@@ -755,3 +787,15 @@ function checkForAndReportiOSError(error: DOMException): Error {
   }
   return error;
 }
+
+/**
+ * Checks whether an error is a DOMException (e.g. as thrown by IndexedDb).
+ *
+ * Supports both browsers and Node with persistence.
+ */
+function isDomException(e: Error): boolean {
+  return (
+    (typeof DOMException !== 'undefined' && e instanceof DOMException) ||
+    e.constructor.name === 'DOMException'
+  );
+}

packages/firestore/test/unit/local/encoded_resource_path.test.ts

@@ -216,7 +216,12 @@ function runTransaction<T>(
     transaction: SimpleDbTransaction
   ) => PersistencePromise<T>
 ): Promise<T> {
-  return db.runTransaction<T>('readwrite', ['test'], txn => {
-    return fn(txn.store<string, boolean>('test'), txn);
-  });
+  return db.runTransaction<T>(
+    'readwrite',
+    /* idempotent= */ false,
+    ['test'],
+    txn => {
+      return fn(txn.store<string, boolean>('test'), txn);
+    }
+  );
 }