Skip to content

Commit

Permalink
fix(client-presence): update connectivity checks (#22580)
Browse files Browse the repository at this point in the history
Checking runtime.clientId only determines if ever connected.
runtime.connected reveals current state.
  • Loading branch information
jason-ha committed Sep 20, 2024
1 parent 2f7a9d0 commit e84ac46
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
11 changes: 7 additions & 4 deletions packages/framework/presence/src/presenceDatastoreManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ function isPresenceMessage(
*/
export type IEphemeralRuntime = Pick<
(IContainerRuntime & IRuntimeInternal) | IFluidDataStoreRuntime,
"clientId" | "getQuorum" | "off" | "on" | "submitSignal"
"clientId" | "connected" | "getQuorum" | "off" | "on" | "submitSignal"
>;

/**
Expand Down Expand Up @@ -130,7 +130,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
// Note: In some manual testing, this does not appear to be enough to
// always trigger an initial connect.
const clientId = runtime.clientId;
if (clientId !== undefined) {
if (clientId !== undefined && runtime.connected) {
this.onConnect(clientId);
}
}
Expand Down Expand Up @@ -178,7 +178,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
forceBroadcast: boolean,
): void => {
// Check for connectivity before sending updates.
if (this.runtime.clientId === undefined) {
if (!this.runtime.connected) {
return;
}

Expand Down Expand Up @@ -258,6 +258,7 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
(this.averageLatency + message.content.avgLatency + message.content.sendTimestamp);

if (message.type === joinMessageType) {
assert(this.runtime.connected, "Received presence join signal while not connected");
const updateProviders = message.content.updateProviders;
this.refreshBroadcastRequested = true;
// We must be connected to receive this message, so clientId should be defined.
Expand All @@ -283,7 +284,9 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
// given an chance before us with named providers given more time.
const waitTime = 20 + 20 * (3 * updateProviders.length + indexOfSelf);
setTimeout(() => {
if (this.refreshBroadcastRequested) {
// Make sure a broadcast is still needed and we are currently connected.
// If not connected, nothing we can do.
if (this.refreshBroadcastRequested && this.runtime.connected) {
// TODO: Add telemetry for this attempt to satisfy join
this.broadcastAllKnownState();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/presence/src/presenceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class PresenceManager implements IPresenceManager {
]);

public constructor(runtime: IEphemeralRuntime) {
// If already connected, populate self and attendees.
// If already connected (now or in the past), populate self and attendees.
const originalClientId = runtime.clientId;
if (originalClientId !== undefined) {
this.selfAttendee.currentConnectionId = () => originalClientId;
Expand Down

0 comments on commit e84ac46

Please sign in to comment.