From 9787ac3abc14f7f7d83619a594d9f152f458cac2 Mon Sep 17 00:00:00 2001 From: Valere Fedronic Date: Wed, 25 Jun 2025 19:35:50 +0200 Subject: [PATCH] bugfix: #3344 Reconnecting to the same SFU on membership change (#3361) * bugfix: #3344 Reconnecting to the same SFU on membership change * fixup! commit error * Keep useActiveLivekitFocus from changing focus spuriously * Remove redundant fix for spurious focus changes We've now fixed it at the source by prohibiting state changes in useActiveLivekitFocus itself. --------- Co-authored-by: Robin --- playwright/sfu-reconnect-bug.spec.ts | 104 +++++++++++++++++++++++++++ src/room/useActiveFocus.ts | 29 ++++---- 2 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 playwright/sfu-reconnect-bug.spec.ts diff --git a/playwright/sfu-reconnect-bug.spec.ts b/playwright/sfu-reconnect-bug.spec.ts new file mode 100644 index 00000000..c756570a --- /dev/null +++ b/playwright/sfu-reconnect-bug.spec.ts @@ -0,0 +1,104 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { expect, test } from "@playwright/test"; + +test("When creator left, avoid reconnect to the same SFU", async ({ + browser, +}) => { + // Use reduce motion to disable animations that are making the tests a bit flaky + const creatorContext = await browser.newContext({ reducedMotion: "reduce" }); + const creatorPage = await creatorContext.newPage(); + + await creatorPage.goto("/"); + + // ======== + // ARRANGE: The first user creates a call as guest, join it, then click the invite button to copy the invite link + // ======== + await creatorPage.getByTestId("home_callName").click(); + await creatorPage.getByTestId("home_callName").fill("Welcome"); + await creatorPage.getByTestId("home_displayName").click(); + await creatorPage.getByTestId("home_displayName").fill("Inviter"); + await creatorPage.getByTestId("home_go").click(); + await expect(creatorPage.locator("video")).toBeVisible(); + + // join + await creatorPage.getByTestId("lobby_joinCall").click(); + // Spotlight mode to make checking the test visually clearer + await creatorPage.getByRole("radio", { name: "Spotlight" }).check(); + + // Get the invite link + await creatorPage.getByRole("button", { name: "Invite" }).click(); + await expect( + creatorPage.getByRole("heading", { name: "Invite to this call" }), + ).toBeVisible(); + await expect(creatorPage.getByRole("img", { name: "QR Code" })).toBeVisible(); + await expect(creatorPage.getByTestId("modal_inviteLink")).toBeVisible(); + await expect(creatorPage.getByTestId("modal_inviteLink")).toBeVisible(); + await creatorPage.getByTestId("modal_inviteLink").click(); + + const inviteLink = (await creatorPage.evaluate( + "navigator.clipboard.readText()", + )) as string; + expect(inviteLink).toContain("room/#/"); + + // ======== + // ACT: The other user use the invite link to join the call as a guest + // ======== + const guestB = await browser.newContext({ + reducedMotion: "reduce", + }); + const guestBPage = await guestB.newPage(); + + await guestBPage.goto(inviteLink); + await guestBPage.getByTestId("joincall_displayName").fill("Invitee"); + await expect(guestBPage.getByTestId("joincall_joincall")).toBeVisible(); + await guestBPage.getByTestId("joincall_joincall").click(); + await guestBPage.getByTestId("lobby_joinCall").click(); + await guestBPage.getByRole("radio", { name: "Spotlight" }).check(); + + // ======== + // ACT: add a third user to the call to reproduce the bug + // ======== + const guestC = await browser.newContext({ + reducedMotion: "reduce", + }); + const guestCPage = await guestC.newPage(); + let sfuGetCallCount = 0; + await guestCPage.route("**/livekit/jwt/sfu/get", async (route) => { + sfuGetCallCount++; + await route.continue(); + }); + // Track WebSocket connections + let wsConnectionCount = 0; + await guestCPage.routeWebSocket("**", (ws) => { + // For some reason the interception is not working with the ** + if (ws.url().includes("livekit/sfu/rtc")) { + wsConnectionCount++; + } + ws.connectToServer(); + }); + + await guestCPage.goto(inviteLink); + await guestCPage.getByTestId("joincall_displayName").fill("Invitee"); + await expect(guestCPage.getByTestId("joincall_joincall")).toBeVisible(); + await guestCPage.getByTestId("joincall_joincall").click(); + await guestCPage.getByTestId("lobby_joinCall").click(); + await guestCPage.getByRole("radio", { name: "Spotlight" }).check(); + + await guestCPage.waitForTimeout(1000); + + // ======== + // the creator leaves the call + await creatorPage.getByTestId("incall_leave").click(); + + await guestCPage.waitForTimeout(2000); + // https://github.com/element-hq/element-call/issues/3344 + // The app used to request a new jwt token then to reconnect to the SFU + expect(wsConnectionCount).toBe(1); + expect(sfuGetCallCount).toBe(1); +}); diff --git a/src/room/useActiveFocus.ts b/src/room/useActiveFocus.ts index 41b42341..7a8f4521 100644 --- a/src/room/useActiveFocus.ts +++ b/src/room/useActiveFocus.ts @@ -9,7 +9,7 @@ import { type MatrixRTCSession, MatrixRTCSessionEvent, } from "matrix-js-sdk/lib/matrixrtc"; -import { useCallback, useEffect, useRef } from "react"; +import { useCallback, useRef } from "react"; import { deepCompare } from "matrix-js-sdk/lib/utils"; import { logger } from "matrix-js-sdk/lib/logger"; import { type LivekitFocus, isLivekitFocus } from "matrix-js-sdk/lib/matrixrtc"; @@ -24,27 +24,22 @@ import { useTypedEventEmitterState } from "../useEvents"; export function useActiveLivekitFocus( rtcSession: MatrixRTCSession, ): LivekitFocus | undefined { - const activeFocus = useTypedEventEmitterState( + const prevActiveFocus = useRef(undefined); + return useTypedEventEmitterState( rtcSession, MatrixRTCSessionEvent.MembershipsChanged, useCallback(() => { const f = rtcSession.getActiveFocus(); // Only handle foci with type="livekit" for now. - return !!f && isLivekitFocus(f) ? f : undefined; + if (f && isLivekitFocus(f) && !deepCompare(f, prevActiveFocus.current)) { + const oldestMembership = rtcSession.getOldestMembership(); + logger.info( + `Got new active focus from membership: ${oldestMembership?.sender}/${oldestMembership?.deviceId}. + Updated focus (focus switch) from ${JSON.stringify(prevActiveFocus.current)} to ${JSON.stringify(f)}`, + ); + prevActiveFocus.current = f; + } + return prevActiveFocus.current; }, [rtcSession]), ); - - const prevActiveFocus = useRef(activeFocus); - useEffect(() => { - if (!deepCompare(activeFocus, prevActiveFocus.current)) { - const oldestMembership = rtcSession.getOldestMembership(); - logger.warn( - `Got new active focus from membership: ${oldestMembership?.sender}/${oldestMembership?.deviceId}. - Updated focus (focus switch) from ${JSON.stringify(prevActiveFocus.current)} to ${JSON.stringify(activeFocus)}`, - ); - prevActiveFocus.current = activeFocus; - } - }, [activeFocus, rtcSession]); - - return activeFocus; }