Add transaction retries

Author: schmidt-sebastian

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.
@@ -753,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,56 @@ 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) {
+        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 +785,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/simple_db.test.ts

@@ -515,8 +515,87 @@ describe('SimpleDb', () => {
     );
   });
 
-  // A little perf test for convenient benchmarking
-  // eslint-disable-next-line no-restricted-properties
+  it('retries idempotent transactions', async () => {
+    let attemptCount = 0;
+
+    const result = await db.runTransaction(
+      'readwrite',
+      /* idempotent= */ true,
+      ['users'],
+      txn => {
+        ++attemptCount;
+        if (attemptCount === 1) {
+          const store = txn.store<string[], typeof dummyUser>('users');
+          return store
+            .add(dummyUser)
+            .next(() => {
+              return store.add(dummyUser);
+            })
+            .next(() => 'Uncaught unique key violation');
+        } else {
+          console.log('retry');
+          return PersistencePromise.resolve('success');
+        }
+      }
+    );
+
+    expect(result).to.equal('success');
+    expect(attemptCount).to.equal(2);
+  });
+
+  it('retries transaction only three times', async () => {
+    let attemptCount = 0;
+
+    await expect(
+      db.runTransaction('readwrite', /* idempotent= */ true, ['users'], txn => {
+        ++attemptCount;
+        const store = txn.store<string[], typeof dummyUser>('users');
+        return store
+          .add(dummyUser)
+          .next(() => {
+            return store.add(dummyUser);
+          })
+          .next(() => 'Uncaught unique key violation');
+      })
+    ).to.eventually.be.rejected;
+
+    expect(attemptCount).to.equal(3);
+  });
+
+  it('does not retry explicitly aborted transactions', async () => {
+    let attemptCount = 0;
+
+    await expect(
+      db.runTransaction('readonly', /* idempotent= */ true, ['users'], txn => {
+        ++attemptCount;
+        txn.abort();
+        return PersistencePromise.reject(new Error('Aborted'));
+      })
+    ).to.eventually.be.rejected;
+
+    expect(attemptCount).to.equal(1);
+  });
+
+  it('does not retry non-idempotent transactions', async () => {
+    let attemptCount = 0;
+
+    await expect(
+      db.runTransaction('readonly', /* idempotent= */ false, ['users'], txn => {
+        ++attemptCount;
+        const store = txn.store<string[], typeof dummyUser>('users');
+        return store
+          .add(dummyUser)
+          .next(() => {
+            return store.add(dummyUser);
+          })
+          .next(() => 'Uncaught unique key violation');
+      })
+    ).to.eventually.be.rejectedWith('Uncaught unique key violation');
+
+    expect(attemptCount).to.equal(1);
+  });
+
+  // eslint-disable-next-line no-restricted-properties, A little perf test for convenient benchmarking
   it.skip('Perf', () => {
     return runTransaction(store => {
       const start = new Date().getTime();