Revert: IndexedDB recovery for handleUserChange

Reverts #3087
Fixes #3179

Author: schmidt-sebastian

packages/firestore/src/core/firestore_client.ts

@@ -179,7 +179,7 @@ export class FirestoreClient {
           persistenceResult
         ).then(initializationDone.resolve, initializationDone.reject);
       } else {
-        this.asyncQueue.enqueueRetryable(() => {
+        this.asyncQueue.enqueueAndForget(() => {
           return this.handleCredentialChange(user);
         });
       }

packages/firestore/src/core/sync_engine.ts

@@ -862,15 +862,15 @@ export class SyncEngine implements RemoteSyncer {
 
   async handleCredentialChange(user: User): Promise<void> {
     const userChanged = !this.currentUser.isEqual(user);
+    this.currentUser = user;
 
     if (userChanged) {
-      const result = await this.localStore.handleUserChange(user);
-      this.currentUser = user;
-
       // Fails tasks waiting for pending writes requested by previous user.
       this.rejectOutstandingPendingWritesCallbacks(
         "'waitForPendingWrites' promise is rejected due to a user change."
       );
+
+      const result = await this.localStore.handleUserChange(user);
       // TODO(b/114226417): Consider calling this only in the primary tab.
       this.sharedClientState.handleUserChange(
         user,

packages/firestore/src/util/async_queue.ts

@@ -323,32 +323,29 @@ export class AsyncQueue {
    * operations were retried successfully.
    */
   enqueueRetryable(op: () => Promise<void>): void {
-    this.verifyNotFailed();
+    const resultAfterRetries = new Deferred<void>();
+
+    const retryAttempt = async (): Promise<void> => {
+      try {
+        await op();
+        resultAfterRetries.resolve();
+        this.backoff.reset();
+      } catch (e) {
+        if (isIndexedDbTransactionError(e)) {
+          logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
+          this.backoff.backoffAndRun(retryAttempt);
+        } else {
+          resultAfterRetries.resolve();
+          throw e; // Failure will be handled by AsyncQueue
+        }
+      }
+    };
 
-    if (this._isShuttingDown) {
-      return;
-    }
+    this.retryableTail = this.retryableTail.then(
+      () => resultAfterRetries.promise
+    );
 
-    this.retryableTail = this.retryableTail.then(() => {
-      const deferred = new Deferred<void>();
-      const retryingOp = async (): Promise<void> => {
-        try {
-          await op();
-          deferred.resolve();
-          this.backoff.reset();
-        } catch (e) {
-          if (isIndexedDbTransactionError(e)) {
-            logDebug(LOG_TAG, 'Operation failed with retryable error: ' + e);
-            this.backoff.backoffAndRun(retryingOp);
-          } else {
-            deferred.resolve();
-            throw e; // Failure will be handled by AsyncQueue
-          }
-        }
-      };
-      this.enqueueAndForget(retryingOp);
-      return deferred.promise;
-    });
+    this.enqueueAndForget(retryAttempt);
   }
 
   private enqueueInternal<T extends unknown>(op: () => Promise<T>): Promise<T> {

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

@@ -709,41 +709,4 @@ describeSpec('Persistence Recovery', ['no-ios', 'no-android'], () => {
         removed: [doc1]
       });
   });
-
-  specTest(
-    'User change handles transaction failures (with recovery)',
-    ['durable-persistence'],
-    () => {
-      const query = Query.atPath(path('collection'));
-      const doc1 = doc(
-        'collection/key1',
-        0,
-        { foo: 'a' },
-        { hasLocalMutations: true }
-      );
-      return spec()
-        .changeUser('user1')
-        .userSets('collection/key1', { foo: 'a' })
-        .userListens(query)
-        .expectEvents(query, {
-          added: [doc1],
-          fromCache: true,
-          hasPendingWrites: true
-        })
-        .failDatabaseTransactions('Handle user change')
-        .changeUser('user2')
-        .recoverDatabase()
-        .runTimer(TimerId.AsyncQueueRetry)
-        .expectEvents(query, { removed: [doc1], fromCache: true })
-        .failDatabaseTransactions('Handle user change')
-        .changeUser('user1')
-        .recoverDatabase()
-        .runTimer(TimerId.AsyncQueueRetry)
-        .expectEvents(query, {
-          added: [doc1],
-          fromCache: true,
-          hasPendingWrites: true
-        });
-    }
-  );
 });

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

@@ -714,19 +714,11 @@ abstract class TestRunner {
     return Promise.resolve();
   }
 
-  private async doChangeUser(user: string | null): Promise<void> {
+  private doChangeUser(user: string | null): Promise<void> {
     this.user = new User(user);
-    const deferred = new Deferred<void>();
-    await this.queue.enqueueRetryable(async () => {
-      try {
-        await this.syncEngine.handleCredentialChange(this.user);
-      } finally {
-        // Resolve the deferred Promise even if the operation failed. This allows
-        // the spec tests to manually retry the failed user change.
-        deferred.resolve();
-      }
-    });
-    return deferred.promise;
+    return this.queue.enqueue(async () =>
+      this.syncEngine.handleCredentialChange(this.user)
+    );
   }
 
   private async doFailDatabase(

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

@@ -286,7 +286,7 @@ describe('AsyncQueue', () => {
     expect(completedSteps).to.deep.equal([1, 1]);
   });
 
-  it('Retries retryable operations in order', async () => {
+  it.only('Retries retryable operations in order', async () => {
     const queue = new AsyncQueue();
     const completedSteps: number[] = [];
     const doStep = (n: number): void => {
@@ -296,21 +296,52 @@ describe('AsyncQueue', () => {
     const blockingPromise = new Deferred<void>();
 
     queue.enqueueRetryable(async () => {
-      doStep(1);
-      if (completedSteps.length > 1) {
+      if (completedSteps.length == 0) {
+        doStep(1);
+        throw new IndexedDbTransactionError(
+          new Error('Simulated retryable error')
+        );
       } else {
+        doStep(3);
+      }
+    });
+    queue.enqueueRetryable(async () => {
+      if (completedSteps.length == 0) {
+        doStep(2);
         throw new IndexedDbTransactionError(
           new Error('Simulated retryable error')
         );
+      } else {
+        doStep(4);
+        blockingPromise.resolve();
       }
     });
+
+    await blockingPromise.promise;
+    expect(completedSteps).to.deep.equal([1, 2, 3, 4]);
+  });
+
+  it('Retryable and non-retryable operations maintain order', async () => {
+    const queue = new AsyncQueue();
+    const completedSteps: number[] = [];
+    const doStep = (n: number): void => {
+      completedSteps.push(n);
+    };
+
     queue.enqueueRetryable(async () => {
+      doStep(1);
+    });
+    queue.enqueueAndForget(async () => {
       doStep(2);
-      blockingPromise.resolve();
+    });
+    queue.enqueueRetryable(async () => {
+      doStep(3);
+    });
+    await queue.enqueue(async () => {
+      doStep(4);
     });
 
-    await blockingPromise.promise;
-    expect(completedSteps).to.deep.equal([1, 1, 2]);
+    expect(completedSteps).to.deep.equal([1, 2, 3, 4]);
   });
 
   it('Can drain (non-delayed) operations', async () => {