Merge 67070f1 into 1325b58

Author: schmidt-sebastian

packages/firestore/src/core/firestore_client.ts

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

packages/firestore/src/core/sync_engine.ts

@@ -860,15 +860,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/test/unit/specs/recovery_spec.test.ts

@@ -484,4 +484,41 @@ 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': true })
+        .changeUser('user2')
+        .recoverDatabase()
+        .runTimer(TimerId.AsyncQueueRetry)
+        .expectEvents(query, { removed: [doc1], fromCache: true })
+        .failDatabaseTransactions({ 'Handle user change': true })
+        .changeUser('user1')
+        .recoverDatabase()
+        .runTimer(TimerId.AsyncQueueRetry)
+        .expectEvents(query, {
+          added: [doc1],
+          fromCache: true,
+          hasPendingWrites: true
+        });
+    }
+  );
 });

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

@@ -710,11 +710,19 @@ abstract class TestRunner {
     return Promise.resolve();
   }
 
-  private doChangeUser(user: string | null): Promise<void> {
+  private async doChangeUser(user: string | null): Promise<void> {
     this.user = new User(user);
-    return this.queue.enqueue(() =>
-      this.syncEngine.handleCredentialChange(this.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;
   }
 
   private async doFailDatabase(