Add transaction retries (#2250)

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 IndexedDbTransactionMode =
+  | 'readonly'
+  | 'readwrite'
+  | 'readwrite-primary'
+  | 'readonly-idempotent'
+  | 'readwrite-idempotent'
+  | 'readwrite-primary-idempotent';
+
 /**
  * An IndexedDB-backed instance of Persistence. Data is stored persistently
  * across sessions.
@@ -742,17 +751,28 @@ export class IndexedDbPersistence implements Persistence {
 
   runTransaction<T>(
     action: string,
-    mode: 'readonly' | 'readwrite' | 'readwrite-primary',
+    mode: IndexedDbTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
     log.debug(LOG_TAG, 'Starting transaction:', action);
 
+    // TODO(schmidt-sebastian): Simplify once all transactions are idempotent.
+    const idempotent = mode.endsWith('idempotent');
+    const readonly = mode.startsWith('readonly');
+    const simpleDbMode = readonly
+      ? idempotent
+        ? 'readonly-idempotent'
+        : 'readonly'
+      : idempotent
+      ? 'readwrite-idempotent'
+      : 'readwrite';
+
     // Do all transactions as readwrite against all object stores, since we
     // are the only reader/writer.
     return this.simpleDb.runTransaction(
-      mode === 'readonly' ? 'readonly' : 'readwrite',
+      simpleDbMode,
       ALL_STORES,
       simpleDbTxn => {
         if (mode === 'readwrite-primary') {

packages/firestore/src/local/simple_db.ts

@@ -25,6 +25,19 @@ 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;
+
+// The different modes supported by `SimpleDb.runTransaction()`
+type SimpleDbTransactionMode =
+  | 'readonly'
+  | 'readwrite'
+  | 'readonly-idempotent'
+  | 'readwrite-idempotent';
+
 export interface SimpleDbSchemaConverter {
   createOrUpgrade(
     db: IDBDatabase,
@@ -242,32 +255,64 @@ export class SimpleDb {
     };
   }
 
-  runTransaction<T>(
-    mode: 'readonly' | 'readwrite',
+  async runTransaction<T>(
+    mode: SimpleDbTransactionMode,
     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);
+    const readonly = mode.startsWith('readonly');
+    const idempotent = mode.endsWith('idempotent');
+    let attemptNumber = 0;
+
+    while (true) {
+      ++attemptNumber;
+
+      const transaction = SimpleDbTransaction.open(
+        this.db,
+        readonly ? 'readonly' : 'readwrite',
+        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(schmidt-sebastian): We could probably be smarter about this and
+        // 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 +800,13 @@ function checkForAndReportiOSError(error: DOMException): Error {
   }
   return error;
 }
+
+/** Checks whether an error is a DOMException (e.g. as thrown by IndexedDb). */
+function isDomException(error: Error): boolean {
+  // DOMException is not a global type in Node with persistence, and hence we
+  // check the constructor name if the type in unknown.
+  return (
+    (typeof DOMException !== 'undefined' && error instanceof DOMException) ||
+    error.constructor.name === 'DOMException'
+  );
+}

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

@@ -500,6 +500,84 @@ describe('SimpleDb', () => {
     });
   });
 
+  it('retries transactions marked as idempotent', async () => {
+    let attemptCount = 0;
+
+    const result = await db.runTransaction(
+      'readwrite-idempotent',
+      ['users'],
+      txn => {
+        ++attemptCount;
+        if (attemptCount === 1) {
+          const store = txn.store<string[], typeof dummyUser>('users');
+          return store
+            .add(dummyUser)
+            .next(() => {
+              return store.add(dummyUser); // Fails with a unique key violation
+            })
+            .next(() => 'Aborted');
+        } else {
+          return PersistencePromise.resolve('success');
+        }
+      }
+    );
+
+    expect(result).to.equal('success');
+    expect(attemptCount).to.equal(2);
+  });
+
+  it('retries transactions only three times', async () => {
+    let attemptCount = 0;
+
+    await expect(
+      db.runTransaction('readwrite-idempotent', ['users'], txn => {
+        ++attemptCount;
+        const store = txn.store<string[], typeof dummyUser>('users');
+        return store
+          .add(dummyUser)
+          .next(() => {
+            return store.add(dummyUser); // Fails with a unique key violation
+          })
+          .next(() => 'Aborted');
+      })
+    ).to.eventually.be.rejected;
+
+    expect(attemptCount).to.equal(3);
+  });
+
+  it('does not retry explicitly aborted transactions', async () => {
+    let attemptCount = 0;
+
+    await expect(
+      db.runTransaction('readwrite-idempotent', ['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('readwrite', ['users'], txn => {
+        ++attemptCount;
+        const store = txn.store<string[], typeof dummyUser>('users');
+        return store
+          .add(dummyUser)
+          .next(() => {
+            return store.add(dummyUser); // Fails with a unique key violation
+          })
+          .next(() => 'Aborted');
+      })
+    ).to.eventually.be.rejected;
+
+    expect(attemptCount).to.equal(1);
+  });
+
   // A little perf test for convenient benchmarking
   // eslint-disable-next-line no-restricted-properties
   it.skip('Perf', () => {