diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 1a8724efe0a..9db8d8dd428 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/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); }); } diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 0311830b604..0204f403e50 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -860,15 +860,15 @@ export class SyncEngine implements RemoteSyncer { async handleCredentialChange(user: User): Promise { 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, diff --git a/packages/firestore/test/unit/specs/recovery_spec.test.ts b/packages/firestore/test/unit/specs/recovery_spec.test.ts index 9e6af374edc..8e0d9b37d0f 100644 --- a/packages/firestore/test/unit/specs/recovery_spec.test.ts +++ b/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 + }); + } + ); }); diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index c1b82633b6e..153cfddef6e 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/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 { + private async doChangeUser(user: string | null): Promise { this.user = new User(user); - return this.queue.enqueue(() => - this.syncEngine.handleCredentialChange(this.user) - ); + const deferred = new Deferred(); + 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(