Skip to content

Commit 249d40c

Browse files
Don't switch network status to UNKNOWN during user change (#3700)
1 parent 0dfd516 commit 249d40c

File tree

4 files changed

+34
-8
lines changed

4 files changed

+34
-8
lines changed

.changeset/cold-impalas-drop.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@firebase/firestore": patch
3+
---
4+
5+
Fixes a bug that caused the client to not raise snapshots from cache if a user change happened while the network connection was disabled.

packages/firestore/exp/src/api/components.ts

+1
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export async function setOnlineComponentProvider(
9898
// The CredentialChangeListener of the online component provider takes
9999
// precedence over the offline component provider.
100100
firestore._setCredentialChangeListener(user =>
101+
// TODO(firestoreexp): This should be enqueueRetryable.
101102
firestore._queue.enqueueAndForget(() =>
102103
onlineComponentProvider.remoteStore.handleCredentialChange(user)
103104
)

packages/firestore/src/remote/remote_store.ts

+11-8
Original file line numberDiff line numberDiff line change
@@ -791,21 +791,24 @@ export class RemoteStore implements TargetMetadataProvider {
791791

792792
async handleCredentialChange(user: User): Promise<void> {
793793
this.asyncQueue.verifyOperationInProgress();
794+
debugAssert(
795+
!!this.remoteSyncer.handleCredentialChange,
796+
'handleCredentialChange() not set'
797+
);
798+
799+
logDebug(LOG_TAG, 'RemoteStore received new credentials');
800+
const canUseNetwork = this.canUseNetwork();
794801

795802
// Tear down and re-create our network streams. This will ensure we get a
796803
// fresh auth token for the new user and re-fill the write pipeline with
797804
// new mutations from the LocalStore (since mutations are per-user).
798-
logDebug(LOG_TAG, 'RemoteStore received new credentials');
799805
this.offlineCauses.add(OfflineCause.CredentialChange);
800-
801806
await this.disableNetworkInternal();
802-
this.onlineStateTracker.set(OnlineState.Unknown);
803-
debugAssert(
804-
!!this.remoteSyncer.handleCredentialChange,
805-
'handleCredentialChange() not set'
806-
);
807+
if (canUseNetwork) {
808+
// Don't set the network status to Unknown if we are offline.
809+
this.onlineStateTracker.set(OnlineState.Unknown);
810+
}
807811
await this.remoteSyncer.handleCredentialChange(user);
808-
809812
this.offlineCauses.delete(OfflineCause.CredentialChange);
810813
await this.enableNetworkInternal();
811814
}

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

+17
Original file line numberDiff line numberDiff line change
@@ -239,4 +239,21 @@ describeSpec('Offline:', [], () => {
239239
);
240240
}
241241
);
242+
243+
specTest('Client stays offline during credential change', [], () => {
244+
// Reproduces a bug that caused the client to switch to OnlineState
245+
// `Unknown` during a credential change.
246+
247+
const query1 = query('collection');
248+
return (
249+
spec()
250+
.disableNetwork()
251+
.changeUser('user1')
252+
.userListens(query1)
253+
// Client is still offline and we raise a `fromCache` event immediately
254+
.expectEvents(query1, {
255+
fromCache: true
256+
})
257+
);
258+
});
242259
});

0 commit comments

Comments
 (0)