diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index 1b120546..93403a96 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -97,6 +97,13 @@ export interface ConfigOptions { enable_video?: boolean; }; + /** + * Grace period in milliseconds to wait before reporting the sync loop as disconnected. + * This allows brief sync interruptions without triggering a reconnection message. + * Default is 10000ms (10 seconds). Set to 0 to disable the grace period. + */ + sync_disconnect_grace_period_ms?: number; + /** * These are low level options that are used to configure the MatrixRTC session. * Take care when changing these options. @@ -155,6 +162,7 @@ export interface ResolvedConfigOptions extends ConfigOptions { server_name: string; }; }; + sync_disconnect_grace_period_ms: number; ssla: string; } @@ -168,5 +176,6 @@ export const DEFAULT_CONFIG: ResolvedConfigOptions = { features: { feature_use_device_session_member_events: true, }, + sync_disconnect_grace_period_ms: 10000, ssla: "https://static.element.io/legal/element-software-and-services-license-agreement-uk-1.pdf", }; diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts index 87ca35d0..3de6a7d5 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts @@ -13,6 +13,7 @@ import { MembershipManagerEvent, Status } from "matrix-js-sdk/lib/matrixrtc"; import { ObservableScope } from "../../ObservableScope"; import { createHomeserverConnected$ } from "./HomeserverConnected"; +import { testScope, withTestScheduler } from "../../../utils/test"; /** * Minimal stub of a Matrix client sufficient for our tests: @@ -96,19 +97,20 @@ 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", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); expect(hsConnected.combined$.value).toBe(false); }); it("remains false while membership status is not Connected even if sync is Syncing", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); expect(hsConnected.combined$.value).toBe(false); // membership still disconnected }); it("is false when membership status transitions to Connected but ProbablyLeft is true", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); // Make sync loop OK client.setSyncState(SyncState.Syncing); // Indicate probable leave before connection @@ -118,7 +120,7 @@ describe("createHomeserverConnected$", () => { }); it("becomes true only when all three conditions are satisfied", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + 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 @@ -128,7 +130,7 @@ describe("createHomeserverConnected$", () => { }); it("drops back to false when sync loop leaves Syncing", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); // Reach connected state client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); @@ -140,7 +142,7 @@ describe("createHomeserverConnected$", () => { }); it("drops back to false when membership status becomes disconnected", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); expect(hsConnected.combined$.value).toBe(true); @@ -150,7 +152,7 @@ describe("createHomeserverConnected$", () => { }); it("drops to false when ProbablyLeft is emitted after being true", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); expect(hsConnected.combined$.value).toBe(true); @@ -160,7 +162,7 @@ describe("createHomeserverConnected$", () => { }); it("recovers to true if ProbablyLeft becomes false again while other conditions remain true", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); expect(hsConnected.combined$.value).toBe(true); @@ -174,7 +176,7 @@ describe("createHomeserverConnected$", () => { }); it("composite sequence reflects each individual failure reason", () => { - const hsConnected = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session, 0); // Initially false (sync error + disconnected + not probably left) expect(hsConnected.combined$.value).toBe(false); @@ -200,3 +202,62 @@ describe("createHomeserverConnected$", () => { expect(hsConnected.combined$.value).toBe(false); }); }); + +describe("createHomeserverConnected$ - Grace Period", () => { + const GRACE_PERIOD = 5; + + function marbleTest( + syncStateMarbles: string, + expectedConnectedMarbles: string, + ): void { + withTestScheduler(({ behavior, schedule, expectObservable }) => { + const syncState$ = behavior(syncStateMarbles, { + s: SyncState.Syncing, + e: SyncState.Error, + }); + const client = new MockMatrixClient(syncState$.value); + schedule(syncStateMarbles, { + s: () => client.setSyncState(SyncState.Syncing), + e: () => client.setSyncState(SyncState.Error), + }); + const session = new MockMatrixRTCSession({ + membershipStatus: Status.Connected, + probablyLeft: false, + }); + const hsConnected = createHomeserverConnected$( + testScope(), + client, + session, + GRACE_PERIOD, + ); + expectObservable(hsConnected.combined$).toBe(expectedConnectedMarbles, { + y: true, + n: false, + }); + }); + } + + it("respects gracePeriodMs: stays true during grace period and flips false after", () => { + // - Initial state: Everything is connected + // - Sync error occurs -> should remain connected due to grace period + // - After grace period, not connected + marbleTest("se", "y-----n"); + // If the sync error takes longer to occur, it should take equally long for + // the connection state to change + marbleTest("s--e", "y-------n"); + }); + + it("recovers immediately if sync returns during grace period", () => { + // - Initial state: Connected + // - Sync error occurs + // - Sync recovers BEFORE the grace period expires + // - Connection state remains constant + marbleTest("se--s", "y"); + }); + + it("flips to true IMMEDIATELY even if a grace period was pending", () => { + // - Initial error: connection eventually flips to false + // - Back to Syncing -> Must be connected immediately (synchronously) + marbleTest("e-----s", "y----ny"); + }); +}); diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.ts b/src/state/CallViewModel/localMember/HomeserverConnected.ts index c8bcd021..65cc24c6 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.ts @@ -12,9 +12,20 @@ import { type MatrixRTCSession, } from "matrix-js-sdk/lib/matrixrtc"; import { ClientEvent, type MatrixClient, SyncState } from "matrix-js-sdk"; -import { fromEvent, startWith, map, tap, type Observable } from "rxjs"; +import { + fromEvent, + startWith, + map, + tap, + type Observable, + distinctUntilChanged, + switchMap, + of, + delay, +} from "rxjs"; 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"; @@ -35,28 +46,46 @@ export interface HomeserverConnected { * for the purposes of a MatrixRTC session. * * Becomes FALSE if ANY sub-condition is fulfilled: - * 1. Sync loop is not in SyncState.Syncing + * 1. Sync loop is not in SyncState.Syncing (after grace period) * 2. membershipStatus !== Status.Connected * 3. probablyLeft === true + * + * @param scope - The observable scope for lifecycle management. + * @param client - The Matrix client to monitor sync state. + * @param matrixRTCSession - The RTC session to monitor membership. + * @param gracePeriodMs - Grace period in milliseconds to wait before reporting sync disconnect. + * If not provided, uses the config value (default 10000ms). */ export function createHomeserverConnected$( scope: ObservableScope, client: NodeStyleEventEmitter & Pick, matrixRTCSession: NodeStyleEventEmitter & Pick, + gracePeriodMs?: number, ): HomeserverConnected { + // Get grace period from parameter or config (default 10000ms) + const graceMs = gracePeriodMs ?? Config.get().sync_disconnect_grace_period_ms; + const syncing$ = ( fromEvent(client, ClientEvent.Sync) as Observable<[SyncState]> ).pipe( startWith([client.getSyncState()]), map(([state]) => state === SyncState.Syncing), + distinctUntilChanged(), + switchMap((isSyncing) => { + if (isSyncing || graceMs <= 0) { + return of(isSyncing); + } + return of(false).pipe(delay(graceMs), startWith(true)); + }), + distinctUntilChanged(), ); const rtsSession$ = scope.behavior( fromEvent(matrixRTCSession, MembershipManagerEvent.StatusChanged).pipe( map(() => matrixRTCSession.membershipStatus ?? Status.Unknown), ), - Status.Unknown, + matrixRTCSession.membershipStatus ?? Status.Unknown, ); const membershipConnected$ = rtsSession$.pipe(