diff --git a/.changeset/cold-impalas-drop.md b/.changeset/cold-impalas-drop.md new file mode 100644 index 00000000000..37fd7c2e244 --- /dev/null +++ b/.changeset/cold-impalas-drop.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Fixes a bug that caused the client to not raise snapshots from cache if a user change happened while the network connection was disabled. diff --git a/packages/firestore/exp/src/api/components.ts b/packages/firestore/exp/src/api/components.ts index 040b4dcc681..11409f63806 100644 --- a/packages/firestore/exp/src/api/components.ts +++ b/packages/firestore/exp/src/api/components.ts @@ -98,6 +98,7 @@ export async function setOnlineComponentProvider( // The CredentialChangeListener of the online component provider takes // precedence over the offline component provider. firestore._setCredentialChangeListener(user => + // TODO(firestoreexp): This should be enqueueRetryable. firestore._queue.enqueueAndForget(() => onlineComponentProvider.remoteStore.handleCredentialChange(user) ) diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 934c97484c4..893fbedef8b 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -791,21 +791,24 @@ export class RemoteStore implements TargetMetadataProvider { async handleCredentialChange(user: User): Promise { this.asyncQueue.verifyOperationInProgress(); + debugAssert( + !!this.remoteSyncer.handleCredentialChange, + 'handleCredentialChange() not set' + ); + + logDebug(LOG_TAG, 'RemoteStore received new credentials'); + const canUseNetwork = this.canUseNetwork(); // Tear down and re-create our network streams. This will ensure we get a // fresh auth token for the new user and re-fill the write pipeline with // new mutations from the LocalStore (since mutations are per-user). - logDebug(LOG_TAG, 'RemoteStore received new credentials'); this.offlineCauses.add(OfflineCause.CredentialChange); - await this.disableNetworkInternal(); - this.onlineStateTracker.set(OnlineState.Unknown); - debugAssert( - !!this.remoteSyncer.handleCredentialChange, - 'handleCredentialChange() not set' - ); + if (canUseNetwork) { + // Don't set the network status to Unknown if we are offline. + this.onlineStateTracker.set(OnlineState.Unknown); + } await this.remoteSyncer.handleCredentialChange(user); - this.offlineCauses.delete(OfflineCause.CredentialChange); await this.enableNetworkInternal(); } diff --git a/packages/firestore/test/unit/specs/offline_spec.test.ts b/packages/firestore/test/unit/specs/offline_spec.test.ts index 31af51f23ec..b59a0ad9da1 100644 --- a/packages/firestore/test/unit/specs/offline_spec.test.ts +++ b/packages/firestore/test/unit/specs/offline_spec.test.ts @@ -239,4 +239,21 @@ describeSpec('Offline:', [], () => { ); } ); + + specTest('Client stays offline during credential change', [], () => { + // Reproduces a bug that caused the client to switch to OnlineState + // `Unknown` during a credential change. + + const query1 = query('collection'); + return ( + spec() + .disableNetwork() + .changeUser('user1') + .userListens(query1) + // Client is still offline and we raise a `fromCache` event immediately + .expectEvents(query1, { + fromCache: true + }) + ); + }); });