From 0ae7dc9a553a6fc9c97083b80bd71c7511e7f6a8 Mon Sep 17 00:00:00 2001 From: fkwp Date: Mon, 11 May 2026 19:27:52 +0200 Subject: [PATCH] Merge disconnectReason$ into combined$; rename HomeserverDisconnectReason items --- src/analytics/PosthogEvents.test.ts | 28 ++-- src/analytics/PosthogEvents.ts | 35 ++-- .../localMember/HomeserverConnected.test.ts | 136 ++++++--------- .../localMember/HomeserverConnected.ts | 42 ++--- .../localMember/LocalMember.test.ts | 157 ++++++++++++++---- .../CallViewModel/localMember/LocalMember.ts | 55 +++--- 6 files changed, 259 insertions(+), 194 deletions(-) diff --git a/src/analytics/PosthogEvents.test.ts b/src/analytics/PosthogEvents.test.ts index 4299f279..83ef4d7c 100644 --- a/src/analytics/PosthogEvents.test.ts +++ b/src/analytics/PosthogEvents.test.ts @@ -94,9 +94,9 @@ describe("CallEnded", () => { roomEventEncryptionKeysReceived: 5, roomEventEncryptionKeysReceivedAverageAge: 100, callReconnectingCount: 0, - callReconnectingCountSyncing: 0, - callReconnectingCountMembershipConnected: 0, - callReconnectingCountCertainlyConnected: 0, + callReconnectingCountSync: 0, + callReconnectingCountMembership: 0, + callReconnectingCountProbablyLeft: 0, callReconnectingCountLivekit: 0, }, { send_instantly: true }, @@ -174,18 +174,18 @@ describe("CallEnded", () => { const mockSession = createMockRtcSession(); tracker.cacheStartCall(new Date()); - tracker.cacheReconnecting("syncing"); - tracker.cacheReconnecting("syncing"); + tracker.cacheReconnecting("sync"); + tracker.cacheReconnecting("sync"); tracker.cacheReconnecting("livekit"); - tracker.cacheReconnecting("membershipConnected"); + tracker.cacheReconnecting("membership"); tracker.track("test-call-id", 1, false, mockSession); expect(PosthogAnalytics.instance.trackEvent).toHaveBeenCalledWith( expect.objectContaining({ callReconnectingCount: 4, - callReconnectingCountSyncing: 2, - callReconnectingCountMembershipConnected: 1, - callReconnectingCountCertainlyConnected: 0, + callReconnectingCountSync: 2, + callReconnectingCountMembership: 1, + callReconnectingCountProbablyLeft: 0, callReconnectingCountLivekit: 1, }), expect.anything(), @@ -211,20 +211,20 @@ describe("CallReconnecting", () => { it("tracks event with correct shape", () => { const tracker = new CallReconnectingTracker(); - tracker.track("!room:example.org", "syncing", 3.5); + tracker.track("!room:example.org", "sync", 3.5); expect(PosthogAnalytics.instance.trackEvent).toHaveBeenCalledWith({ eventName: "CallReconnecting", callId: "!room:example.org", - reason: "syncing", + reason: "sync", reconnectDuration: 3.5, }); }); it.each([ - "syncing", - "membershipConnected", - "certainlyConnected", + "sync", + "membership", + "probablyLeft", "livekit", ] as CallReconnectingReason[])("tracks reason %s correctly", (reason) => { const tracker = new CallReconnectingTracker(); diff --git a/src/analytics/PosthogEvents.ts b/src/analytics/PosthogEvents.ts index e8fcaad3..f49a7553 100644 --- a/src/analytics/PosthogEvents.ts +++ b/src/analytics/PosthogEvents.ts @@ -25,9 +25,9 @@ interface CallEnded extends IPosthogEvent { roomEventEncryptionKeysReceived: number; roomEventEncryptionKeysReceivedAverageAge: number; callReconnectingCount: number; - callReconnectingCountSyncing: number; - callReconnectingCountMembershipConnected: number; - callReconnectingCountCertainlyConnected: number; + callReconnectingCountSync: number; + callReconnectingCountMembership: number; + callReconnectingCountProbablyLeft: number; callReconnectingCountLivekit: number; } @@ -42,9 +42,9 @@ export class CallEndedTracker { maxParticipantsCount: 0, reconnectingCount: 0, reconnectingCountByReason: { - syncing: 0, - membershipConnected: 0, - certainlyConnected: 0, + sync: 0, + membership: 0, + probablyLeft: 0, livekit: 0, }, }; @@ -55,9 +55,9 @@ export class CallEndedTracker { maxParticipantsCount: 0, reconnectingCount: 0, reconnectingCountByReason: { - syncing: 0, - membershipConnected: 0, - certainlyConnected: 0, + sync: 0, + membership: 0, + probablyLeft: 0, livekit: 0, }, }; @@ -100,12 +100,11 @@ export class CallEndedTracker { rtcSession.statistics.counters.roomEventEncryptionKeysReceived : 0, callReconnectingCount: this.cache.reconnectingCount, - callReconnectingCountSyncing: - this.cache.reconnectingCountByReason.syncing, - callReconnectingCountMembershipConnected: - this.cache.reconnectingCountByReason.membershipConnected, - callReconnectingCountCertainlyConnected: - this.cache.reconnectingCountByReason.certainlyConnected, + callReconnectingCountSync: this.cache.reconnectingCountByReason.sync, + callReconnectingCountMembership: + this.cache.reconnectingCountByReason.membership, + callReconnectingCountProbablyLeft: + this.cache.reconnectingCountByReason.probablyLeft, callReconnectingCountLivekit: this.cache.reconnectingCountByReason.livekit, }, @@ -292,9 +291,9 @@ export class CallConnectDurationTracker { } export type CallReconnectingReason = - | "syncing" - | "membershipConnected" - | "certainlyConnected" + | "sync" + | "membership" + | "probablyLeft" | "livekit"; interface CallReconnecting extends IPosthogEvent { diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts index 16bac502..ecd5818a 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts @@ -98,112 +98,112 @@ describe("createHomeserverConnected$", () => { // LLM generated test cases. They are a bit overkill but I improved the mocking so it is // easy enough to read them so I think they can stay. // Note: gracePeriodMs is set to 0 to avoid debouncing delays in tests - it("is false when sync state is not Syncing", () => { + it("reports syncing reason when sync state is not Syncing", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("sync"); }); - it("remains false while membership status is not Connected even if sync is Syncing", () => { + it("reports membership reason when sync is Syncing but membership is not Connected", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); - expect(hsConnected.combined$.value).toBe(false); // membership still disconnected + expect(hsConnected.combined$.value).toBe("membership"); }); - it("is false when membership status transitions to Connected but ProbablyLeft is true", () => { + it("reports probablyLeft reason when membership transitions to Connected but ProbablyLeft is true", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); // Make sync loop OK client.setSyncState(SyncState.Syncing); // Indicate probable leave before connection session.setProbablyLeft(true); session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("probablyLeft"); }); - it("becomes true only when all three conditions are satisfied", () => { + it("becomes null (connected) only when all three conditions are satisfied", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); // 1. Sync loop connected client.setSyncState(SyncState.Syncing); - expect(hsConnected.combined$.value).toBe(false); // not yet membership connected + expect(hsConnected.combined$.value).toBe("membership"); // not yet membership connected // 2. Membership connected session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); // probablyLeft is false + expect(hsConnected.combined$.value).toBeNull(); // probablyLeft is false }); - it("drops back to false when sync loop leaves Syncing", () => { + it("returns syncing reason when sync loop leaves Syncing", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); // Reach connected state client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); - // Sync loop error => should flip false + // Sync loop error => should report syncing reason client.setSyncState(SyncState.Error); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("sync"); }); - it("drops back to false when membership status becomes disconnected", () => { + it("returns membershipConnected reason when membership status becomes disconnected", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); session.setMembershipStatus(Status.Disconnected); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("membership"); }); - it("drops to false when ProbablyLeft is emitted after being true", () => { + it("returns certainlyConnected reason when ProbablyLeft is emitted", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); session.setProbablyLeft(true); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("probablyLeft"); }); - it("recovers to true if ProbablyLeft becomes false again while other conditions remain true", () => { + it("recovers to null (connected) if ProbablyLeft becomes false again while other conditions remain true", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); session.setProbablyLeft(true); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("probablyLeft"); // Simulate clearing the flag (in realistic scenario membership manager would update) session.setProbablyLeft(false); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); }); it("composite sequence reflects each individual failure reason", () => { const hsConnected = createHomeserverConnected$(scope, client, session, 0); - // Initially false (sync error + disconnected + not probably left) - expect(hsConnected.combined$.value).toBe(false); + // Initially: sync error + membership disconnected → syncing wins (highest priority) + expect(hsConnected.combined$.value).toBe("sync"); - // Fix sync only + // Fix sync only → membershipConnected is now the blocker client.setSyncState(SyncState.Syncing); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("membership"); - // Fix membership + // Fix membership → all conditions satisfied session.setMembershipStatus(Status.Connected); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); - // Introduce probablyLeft -> false + // Introduce probablyLeft → certainlyConnected session.setProbablyLeft(true); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("probablyLeft"); - // Restore notProbablyLeft -> true again + // Restore notProbablyLeft → connected again session.setProbablyLeft(false); - expect(hsConnected.combined$.value).toBe(true); + expect(hsConnected.combined$.value).toBeNull(); - // Drop sync -> false + // Drop sync → syncing reason client.setSyncState(SyncState.Error); - expect(hsConnected.combined$.value).toBe(false); + expect(hsConnected.combined$.value).toBe("sync"); }); }); -describe("createHomeserverConnected$ - disconnectReason$", () => { +describe("createHomeserverConnected$ - combined$ reason values", () => { let scope: ObservableScope; let client: MockMatrixClient; let session: MockMatrixRTCSession; @@ -223,86 +223,56 @@ describe("createHomeserverConnected$ - disconnectReason$", () => { }); it("is null when all three conditions are satisfied", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(disconnectReason$.value).toBe(null); + expect(combined$.value).toBeNull(); }); it("reports syncing when sync loop is not Syncing", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); // client starts with SyncState.Error, membership also disconnected - expect(disconnectReason$.value).toBe("syncing"); + expect(combined$.value).toBe("sync"); }); it("reports membershipConnected when sync is fine but membership is not Connected", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); // session still Status.Disconnected - expect(disconnectReason$.value).toBe("membershipConnected"); + expect(combined$.value).toBe("membership"); }); it("reports certainlyConnected when probablyLeft is true", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); session.setProbablyLeft(true); - expect(disconnectReason$.value).toBe("certainlyConnected"); + expect(combined$.value).toBe("probablyLeft"); }); it("prioritises syncing over membershipConnected when both fail", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); // Both sync (Error) and membership (Disconnected) are failing - expect(disconnectReason$.value).toBe("syncing"); + expect(combined$.value).toBe("sync"); }); it("updates reason as conditions change", () => { - const { disconnectReason$ } = createHomeserverConnected$( - scope, - client, - session, - 0, - ); + const { combined$ } = createHomeserverConnected$(scope, client, session, 0); // Initially: syncing fails - expect(disconnectReason$.value).toBe("syncing"); + expect(combined$.value).toBe("sync"); // Fix sync → membershipConnected is now the blocker client.setSyncState(SyncState.Syncing); - expect(disconnectReason$.value).toBe("membershipConnected"); + expect(combined$.value).toBe("membership"); // Fix membership → probablyLeft makes certainlyConnected fail session.setProbablyLeft(true); session.setMembershipStatus(Status.Connected); - expect(disconnectReason$.value).toBe("certainlyConnected"); + expect(combined$.value).toBe("probablyLeft"); // Clear probablyLeft → all conditions satisfied session.setProbablyLeft(false); - expect(disconnectReason$.value).toBe(null); + expect(combined$.value).toBeNull(); }); }); @@ -334,8 +304,8 @@ describe("createHomeserverConnected$ - Grace Period", () => { GRACE_PERIOD, ); expectObservable(hsConnected.combined$).toBe(expectedConnectedMarbles, { - y: true, - n: false, + y: null, + n: "sync", }); }); } diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.ts b/src/state/CallViewModel/localMember/HomeserverConnected.ts index 479a261d..f15dc77e 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.ts @@ -29,7 +29,6 @@ import { logger as rootLogger } from "matrix-js-sdk/lib/logger"; import { Config } from "../../../config/Config"; import { type ObservableScope } from "../../ObservableScope"; import { type Behavior } from "../../Behavior"; -import { and$ } from "../../../utils/observable"; import { type NodeStyleEventEmitter } from "../../../utils/test"; /** @@ -37,25 +36,26 @@ import { type NodeStyleEventEmitter } from "../../../utils/test"; */ const logger = rootLogger.getChild("[HomeserverConnected]"); -export type HomeserverDisconnectReason = - | "syncing" - | "membershipConnected" - | "certainlyConnected"; +export type HomeserverDisconnectReason = "sync" | "membership" | "probablyLeft"; export interface HomeserverConnected { - combined$: Behavior; + /** + * Emits `null` when the homeserver connection is healthy, or the reason for + * disconnection when one of the three sub-conditions fails. + */ + combined$: Behavior; rtsSession$: Behavior; - disconnectReason$: Behavior; } /** * Behavior representing whether we consider ourselves connected to the Matrix homeserver * for the purposes of a MatrixRTC session. * - * Becomes FALSE if ANY sub-condition is fulfilled: - * 1. Sync loop is not in SyncState.Syncing (after grace period) - * 2. membershipStatus !== Status.Connected - * 3. probablyLeft === true + * `combined$` emits `null` when all conditions are satisfied, or the first failing + * reason (priority: syncing > membershipConnected > certainlyConnected): + * 1. Sync loop is not in SyncState.Syncing (after grace period) → "sync" + * 2. membershipStatus !== Status.Connected → "membership" + * 3. probablyLeft === true → "probablyLeft" * * @param scope - The observable scope for lifecycle management. * @param client - The Matrix client to monitor sync state. @@ -116,24 +116,18 @@ export function createHomeserverConnected$( ); const combined$ = scope.behavior( - and$(syncing$, membershipConnected$, certainlyConnected$).pipe( - tap((connected) => { - logger.info(`Homeserver connected update: ${connected}`); - }), - ), - ); - - const disconnectReason$ = scope.behavior( combineLatest([syncing$, membershipConnected$, certainlyConnected$]).pipe( map(([syncing, membership, certainly]) => { - if (!syncing) return "syncing" as const; - if (!membership) return "membershipConnected" as const; - if (!certainly) return "certainlyConnected" as const; + if (!syncing) return "sync" as const; + if (!membership) return "membership" as const; + if (!certainly) return "probablyLeft" as const; return null; }), + tap((reason) => { + logger.info(`Homeserver connected update: ${reason ?? "connected"}`); + }), ), - null, ); - return { combined$, rtsSession$, disconnectReason$ }; + return { combined$, rtsSession$ }; } diff --git a/src/state/CallViewModel/localMember/LocalMember.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts index c4b780ac..87a1f296 100644 --- a/src/state/CallViewModel/localMember/LocalMember.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -225,9 +225,8 @@ describe("LocalMembership", () => { createPublisherFactory: vi.fn(), joinMatrixRTC: async (): Promise => {}, homeserverConnected: { - combined$: constant(true), + combined$: constant(null), rtsSession$: constant(RTCMemberStatus.Connected), - disconnectReason$: constant(null), }, callId: "!test-room-id:example.org", }; @@ -693,16 +692,17 @@ describe("LocalMembership", () => { PosthogAnalytics.resetInstance(); }); - it("fires CallReconnecting with homeserver reason and duration when reconnected", async () => { + it("does not fire CallReconnecting for the initial non-connected state at startup", async () => { const scope = new ObservableScope(); const trackSpy = vi.spyOn( PosthogAnalytics.instance.eventCallReconnecting, "track", ); - const hsConnected$ = new BehaviorSubject(true); - const disconnectReason$ = - new BehaviorSubject(null); + // Simulate startup where membership isn't established yet + const hsReason$ = new BehaviorSubject( + "membership", + ); const connectionManagerData = new ConnectionManagerData(); connectionManagerData.add(connectionTransportAConnected, []); @@ -711,9 +711,8 @@ describe("LocalMembership", () => { scope, ...defaultCreateLocalMemberValues, homeserverConnected: { - combined$: hsConnected$, + combined$: hsReason$, rtsSession$: constant(RTCMemberStatus.Connected), - disconnectReason$, }, connectionManager: { connectionManagerData$: constant(new Epoch(connectionManagerData)), @@ -726,15 +725,52 @@ describe("LocalMembership", () => { await flushPromises(); - // Disconnect with syncing reason, then reconnect - disconnectReason$.next("syncing"); - hsConnected$.next(false); - disconnectReason$.next(null); - hsConnected$.next(true); + // Membership is established — call is now connected + hsReason$.next(null); + + expect(trackSpy).not.toHaveBeenCalled(); + + scope.end(); + }); + + it("fires CallReconnecting with homeserver reason and duration when reconnected", async () => { + const scope = new ObservableScope(); + const trackSpy = vi.spyOn( + PosthogAnalytics.instance.eventCallReconnecting, + "track", + ); + + const hsReason$ = new BehaviorSubject( + null, + ); + + const connectionManagerData = new ConnectionManagerData(); + connectionManagerData.add(connectionTransportAConnected, []); + + createLocalMembership$({ + scope, + ...defaultCreateLocalMemberValues, + homeserverConnected: { + combined$: hsReason$, + rtsSession$: constant(RTCMemberStatus.Connected), + }, + connectionManager: { + connectionManagerData$: constant(new Epoch(connectionManagerData)), + }, + localTransport$: new BehaviorSubject({ + advertised$: new BehaviorSubject(aTransport), + active$: new BehaviorSubject(aTransportWithSFUConfig), + }), + }); + + await flushPromises(); + + hsReason$.next("sync"); + hsReason$.next(null); expect(trackSpy).toHaveBeenCalledWith( defaultCreateLocalMemberValues.callId, - "syncing", + "sync", expect.any(Number), ); @@ -763,10 +799,10 @@ describe("LocalMembership", () => { scope, ...defaultCreateLocalMemberValues, homeserverConnected: { - combined$: new BehaviorSubject(true), + combined$: new BehaviorSubject( + null, + ), rtsSession$: constant(RTCMemberStatus.Connected), - disconnectReason$: - new BehaviorSubject(null), }, connectionManager: { connectionManagerData$: constant(new Epoch(connectionManagerData)), @@ -779,7 +815,6 @@ describe("LocalMembership", () => { await flushPromises(); - // Livekit drops then recovers connectionState$.next(ConnectionState.LivekitDisconnected); connectionState$.next(ConnectionState.LivekitConnected); @@ -799,9 +834,9 @@ describe("LocalMembership", () => { "track", ); - const hsConnected$ = new BehaviorSubject(true); - const disconnectReason$ = - new BehaviorSubject(null); + const hsReason$ = new BehaviorSubject( + null, + ); const connectionManagerData = new ConnectionManagerData(); connectionManagerData.add(connectionTransportAConnected, []); @@ -810,9 +845,8 @@ describe("LocalMembership", () => { scope, ...defaultCreateLocalMemberValues, homeserverConnected: { - combined$: hsConnected$, + combined$: hsReason$, rtsSession$: constant(RTCMemberStatus.Connected), - disconnectReason$, }, connectionManager: { connectionManagerData$: constant(new Epoch(connectionManagerData)), @@ -825,28 +859,81 @@ describe("LocalMembership", () => { await flushPromises(); - // First full reconnect cycle - disconnectReason$.next("membershipConnected"); - hsConnected$.next(false); - disconnectReason$.next(null); - hsConnected$.next(true); - // Second full reconnect cycle - disconnectReason$.next("certainlyConnected"); - hsConnected$.next(false); - disconnectReason$.next(null); - hsConnected$.next(true); + hsReason$.next("membership"); + hsReason$.next(null); + + hsReason$.next("probablyLeft"); + hsReason$.next(null); expect(trackSpy).toHaveBeenCalledTimes(2); expect(trackSpy).toHaveBeenNthCalledWith( 1, defaultCreateLocalMemberValues.callId, - "membershipConnected", + "membership", expect.any(Number), ); expect(trackSpy).toHaveBeenNthCalledWith( 2, defaultCreateLocalMemberValues.callId, - "certainlyConnected", + "probablyLeft", + expect.any(Number), + ); + + scope.end(); + }); + + it("uses the first-disconnect reason when a brief intermediate reconnect occurs (multi-phase livekit reconnect)", async () => { + const scope = new ObservableScope(); + const trackSpy = vi.spyOn( + PosthogAnalytics.instance.eventCallReconnecting, + "track", + ); + + const hsReason$ = new BehaviorSubject( + null, + ); + + const connectionManagerData = new ConnectionManagerData(); + connectionManagerData.add(connectionTransportAConnected, []); + + createLocalMembership$({ + scope, + ...defaultCreateLocalMemberValues, + homeserverConnected: { + combined$: hsReason$, + rtsSession$: constant(RTCMemberStatus.Connected), + }, + connectionManager: { + connectionManagerData$: constant(new Epoch(connectionManagerData)), + }, + localTransport$: new BehaviorSubject({ + advertised$: new BehaviorSubject(aTransport), + active$: new BehaviorSubject(aTransportWithSFUConfig), + }), + }); + + await flushPromises(); + + // Disconnect → brief reconnect → disconnect again → stable reconnect + // This simulates a multi-phase LiveKit reconnect. Two CallReconnecting + // events are emitted (one per null transition) but both carry the + // reason from the first disconnect. + hsReason$.next("sync"); + hsReason$.next(null); // brief reconnect + hsReason$.next("sync"); // re-disconnect (reconnectStart already set, keeps original reason) + hsReason$.next(null); // stable reconnect + + expect(trackSpy).toHaveBeenCalledTimes(2); + expect(trackSpy).toHaveBeenNthCalledWith( + 1, + defaultCreateLocalMemberValues.callId, + "sync", + expect.any(Number), + ); + expect(trackSpy).toHaveBeenNthCalledWith( + 2, + defaultCreateLocalMemberValues.callId, + "sync", expect.any(Number), ); diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 1767fa16..af0de570 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -53,7 +53,6 @@ import { import { ElementWidgetActions, widget } from "../../../widget.ts"; import { getUrlParams } from "../../../UrlParams.ts"; import { PosthogAnalytics } from "../../../analytics/PosthogAnalytics.ts"; -import { type CallReconnectingReason } from "../../../analytics/PosthogEvents.ts"; import { MatrixRTCMode } from "../../../settings/settings.ts"; import { Config } from "../../../config/Config.ts"; import { @@ -62,7 +61,6 @@ import { type FailedToStartError, } from "../remoteMembers/Connection.ts"; import { type HomeserverConnected } from "./HomeserverConnected.ts"; -import { and$ } from "../../../utils/observable.ts"; import { type LocalTransport } from "./LocalTransport.ts"; import { areLivekitTransportsEqual } from "../remoteMembers/MatrixLivekitMembers.ts"; @@ -498,20 +496,35 @@ export const createLocalMembership$ = ({ ); /** - * Whether we are "fully" connected to the call. Accounts for both the - * connection to the MatrixRTC session and the LiveKit publish connection. + * The disconnect reason for the combined Matrix + LiveKit connection, or null + * when fully connected. Homeserver reasons take priority over livekit. + * Both connectivity state and reason come from the same combineLatest emission, + * avoiding any race between the two. */ - const matrixAndLivekitConnected$ = scope.behavior( - and$( + const connectionDisconnectReason$ = scope.behavior( + combineLatest([ homeserverConnected.combined$, localConnectionState$.pipe( map((state) => state === ConnectionState.LivekitConnected), ), - ).pipe( + ]).pipe( + map(([homeserverReason, livekitConnected]) => { + if (homeserverReason !== null) return homeserverReason; + if (!livekitConnected) return "livekit" as const; + return null; + }), tap((v) => logger.debug("livekit+matrix: Connected state changed", v)), ), ); + /** + * Whether we are "fully" connected to the call. Accounts for both the + * connection to the MatrixRTC session and the LiveKit publish connection. + */ + const matrixAndLivekitConnected$ = scope.behavior( + connectionDisconnectReason$.pipe(map((reason) => reason === null)), + ); + /** * Whether we should tell the user that we're reconnecting to the call. */ @@ -523,23 +536,25 @@ export const createLocalMembership$ = ({ false, ); - let reconnectStart: { time: number; reason: CallReconnectingReason } | null = - null; - reconnecting$ - .pipe(distinctUntilChanged(), scope.bind()) - .subscribe((reconnecting) => { - if (reconnecting) { - const homeserverReason = homeserverConnected.disconnectReason$.value; - reconnectStart = { - time: Date.now(), - reason: homeserverReason !== null ? homeserverReason : "livekit", - }; + let reconnectStart: { + time: number; + reason: NonNullable<(typeof connectionDisconnectReason$)["value"]>; + } | null = null; + connectionDisconnectReason$ + .pipe(distinctUntilChanged(), pairwise(), scope.bind()) + .subscribe(([prev, reason]) => { + if (reason !== null) { + // Only begin tracking when transitioning FROM connected (null → non-null). + // This prevents the initial startup phase — where we may be non-null before + // the first real connection — from being counted as a reconnect. + if (prev === null) { + reconnectStart ??= { time: Date.now(), reason }; + } } else if (reconnectStart !== null) { - const duration = (Date.now() - reconnectStart.time) / 1000; PosthogAnalytics.instance.eventCallReconnecting.track( callId, reconnectStart.reason, - duration, + (Date.now() - reconnectStart.time) / 1000, ); PosthogAnalytics.instance.eventCallEnded.cacheReconnecting( reconnectStart.reason,