Only retry DOMException/DOMError

Author: schmidt-sebastian

packages/firestore/src/util/async_queue.ts

@@ -318,9 +318,10 @@ export class AsyncQueue {
   /**
    * Enqueue a retryable operation.
    *
-   * A retryable operation is rescheduled with backoff if it fails with any
-   * exception. All retryable operations are executed in order and only run
-   * if all prior operations were retried successfully.
+   * A retryable operation is rescheduled with backoff if it fails with a
+   * DOMException or a DOMError (the error types used by IndexedDB). All
+   * retryable operations are executed in order and only run if all prior
+   * operations were retried successfully.
    */
   enqueueRetryable(op: () => Promise<void>): void {
     this.verifyNotFailed();
@@ -337,8 +338,20 @@ export class AsyncQueue {
           deferred.resolve();
           this.backoff.reset();
         } catch (e) {
-          logDebug(LOG_TAG, 'Retryable operation failed: ' + e.message);
-          this.backoff.backoffAndRun(retryingOp);
+          // We only retry error that match the ones thrown by IndexedDB (see
+          // https://developer.mozilla.org/en-US/docs/Web/API/IDBTransaction/error)
+          // We don't use `instanceof` as these types are not available in all
+          // environments.
+          if (e.name === 'DOMException' || e.name === 'DOMError') {
+            logDebug(
+              LOG_TAG,
+              'Operation failed with retryable error: ' + e.message
+            );
+            this.backoff.backoffAndRun(retryingOp);
+          } else {
+            deferred.resolve(); 
+            throw e; // Failure will be handled by AsyncQueue
+          }
         }
       };
       this.enqueueAndForget(retryingOp);

packages/firestore/test/unit/specs/recovery_spec.test.ts

@@ -42,7 +42,7 @@ describeSpec(
             // Client 1 has received the WebStorage notification that the write
             // has been acknowledged, but failed to process the change. Hence,
             // we did not get a user callback. We schedule the first retry and
-            // make sure that it also does not get processed until 
+            // make sure that it also does not get processed until
             // `recoverDatabase` is called.
             .runTimer(TimerId.AsyncQueueRetry)
             .recoverDatabase()

packages/firestore/test/unit/specs/spec_test_components.ts

@@ -42,6 +42,7 @@ import {
   MemoryPersistence
 } from '../../../src/local/memory_persistence';
 import { LruParams } from '../../../src/local/lru_garbage_collector';
+import { throwFakeDomException } from '../../util/helpers';
 
 /**
  * A test-only persistence implementation that delegates all calls to the
@@ -105,15 +106,15 @@ export class MockPersistence implements Persistence {
     return this.delegate.getIndexManager();
   }
 
-  runTransaction<T>(
+  async runTransaction<T>(
     action: string,
     mode: PersistenceTransactionMode,
     transactionOperation: (
       transaction: PersistenceTransaction
     ) => PersistencePromise<T>
   ): Promise<T> {
     if (this.injectFailures) {
-      return Promise.reject(new Error('Injected Failure'));
+      throwFakeDomException('Simulated retryable error');
     } else {
       return this.delegate.runTransaction(action, mode, transactionOperation);
     }

packages/firestore/test/unit/util/async_queue.test.ts

@@ -15,11 +15,17 @@
  * limitations under the License.
  */
 
-import { expect } from 'chai';
+import * as chaiAsPromised from 'chai-as-promised';
+
+import { expect, use } from 'chai';
 import { AsyncQueue, TimerId } from '../../../src/util/async_queue';
 import { Code } from '../../../src/util/error';
 import { getLogLevel, setLogLevel, LogLevel } from '../../../src/util/log';
 import { Deferred, Rejecter, Resolver } from '../../../src/util/promise';
+import { fail } from '../../../src/util/assert';
+import { throwFakeDomException } from '../../util/helpers';
+
+use(chaiAsPromised);
 
 describe('AsyncQueue', () => {
   // We reuse these TimerIds for generic testing.
@@ -212,13 +218,29 @@ describe('AsyncQueue', () => {
     queue.enqueueRetryable(async () => {
       doStep(1);
       if (completedSteps.length === 1) {
-        throw new Error('Simulated retryable error');
+        throwFakeDomException('Simulated retryable error');
       }
     });
     await queue.runDelayedOperationsEarly(TimerId.AsyncQueueRetry);
     expect(completedSteps).to.deep.equal([1, 1]);
   });
 
+  it("Doesn't retry internal exceptions", async () => {
+    const queue = new AsyncQueue();
+    // We use a Deferred Promise as retryable operations are scheduled only
+    // when Promise chains are resolved, which can happen after the
+    // `queue.enqueue` call below.
+    const deferred = new Deferred<void>();
+    queue.enqueueRetryable(async () => {
+      deferred.resolve();
+      throw fail('Simulated test failure');
+    });
+    await deferred.promise;
+    await expect(
+      queue.enqueue(() => Promise.resolve())
+    ).to.eventually.be.rejectedWith('Simulated test failure');
+  });
+
   it('Schedules first retryable attempt with no delay', async () => {
     const queue = new AsyncQueue();
     const completedSteps: number[] = [];
@@ -244,7 +266,7 @@ describe('AsyncQueue', () => {
     queue.enqueueRetryable(async () => {
       doStep(1);
       if (completedSteps.length === 1) {
-        throw new Error('Simulated retryable error');
+        throwFakeDomException('Simulated retryable error');
       }
     });
 
@@ -270,7 +292,7 @@ describe('AsyncQueue', () => {
       doStep(1);
       if (completedSteps.length > 1) {
       } else {
-        throw new Error('Simulated retryable error');
+        throwFakeDomException('Simulated retryable error');
       }
     });
     queue.enqueueRetryable(async () => {

packages/firestore/test/util/helpers.ts

@@ -815,6 +815,16 @@ export function expectEqualitySets<T>(
   }
 }
 
+/**
+ * Throws an exception that passes `error.name === DOMException` checks even in
+ * environments without DOMExceptions (such as Node).
+ */
+export function throwFakeDomException(message: string): never {
+  const error = new Error(message);
+  error.name = 'DOMException';
+  throw error;
+}
+
 export function expectFirestoreError(err: Error): void {
   expect(err.name).to.equal('FirebaseError');
 }