Feedback

Author: schmidt-sebastian

packages/firestore/src/util/async_queue.ts

@@ -207,7 +207,7 @@ export class AsyncQueue {
   // The last promise in the queue.
   private tail: Promise<unknown> = Promise.resolve();
 
-  // A list of retryable operations Retryable operation are run in order and
+  // A list of retryable operations. Retryable operations are run in order and
   // retried with backoff.
   private retryableOps: Array<() => Promise<void>> = [];
 
@@ -332,21 +332,29 @@ export class AsyncQueue {
    * reschedules with backoff.
    */
   private async retryNextOp(): Promise<void> {
-    const op = this.retryableOps.shift();
-
-    if (op) {
-      try {
-        await op();
-        this.backoff.reset();
-      } catch (e) {
-        if (isIndexedDbTransactionError(e)) {
-          logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
-          this.retryableOps.unshift(op);
-        } else {
-          throw e; // Failure will be handled by AsyncQueue
-        }
+    if (this.retryableOps.length === 0) {
+      return;
+    }
+
+    try {
+      await this.retryableOps[0]();
+      this.retryableOps.shift();
+      this.backoff.reset();
+    } catch (e) {
+      if (isIndexedDbTransactionError(e)) {
+        logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
+      } else {
+        throw e; // Failure will be handled by AsyncQueue
       }
+    }
 
+    if (this.retryableOps.length > 0) {
+      // Note: This might schedule a `retryNextOp()` even though the items in
+      // the queue may have already been enqueued. Since `backoffAndRun()`
+      // cancels an existing backoff and schedules a new backoff on every call,
+      // there is only ever a single additional operation in the queue. This
+      // operation can then run retryable operations that failed during their
+      // initial attempt.
       this.backoff.backoffAndRun(() => this.retryNextOp());
     }
   }

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

@@ -312,7 +312,7 @@ describe('AsyncQueue', () => {
     expect(completedSteps).to.deep.equal([1, 1, 2]);
   });
 
-  it('Doesn not delay retryable operations that succeed', async () => {
+  it('Does not delay retryable operations that succeed', async () => {
     const queue = new AsyncQueue();
     const completedSteps: number[] = [];
     const doStep = (n: number): void => {