From 630841d9aa123a63251d1d6270646f9634f35203 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 24 Jun 2025 15:22:48 +0200 Subject: [PATCH] bugfix: #3344 Reconnecting to the same SFU on membership change --- playwright/sfu-reconnect-bug.spec.ts | 104 +++++++++++++++++++++++++++ src/livekit/openIDSFU.ts | 25 ++++++- src/livekit/useECConnectionState.ts | 3 +- 3 files changed, 129 insertions(+), 3 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/livekit/openIDSFU.ts b/src/livekit/openIDSFU.ts index 2ebd6045..434f7c3c 100644 --- a/src/livekit/openIDSFU.ts +++ b/src/livekit/openIDSFU.ts @@ -8,7 +8,7 @@ Please see LICENSE in the repository root for full details. import { type IOpenIDToken, type MatrixClient } from "matrix-js-sdk"; import { logger } from "matrix-js-sdk/lib/logger"; import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { type LivekitFocus } from "matrix-js-sdk/lib/matrixrtc"; import { useActiveLivekitFocus } from "../room/useActiveFocus"; @@ -41,12 +41,18 @@ export function useOpenIDSFU( const [sfuConfig, setSFUConfig] = useState(undefined); const activeFocus = useActiveLivekitFocus(rtcSession); + + // TODO: this is an ugly workaround to avoid re-requesting a new jwt token when there is no change in the active focus. + // We have a problem with the hooks here. The root problem should be fixed. + const currentFocus = useRef(Object.assign({}, activeFocus)); + const { showErrorBoundary } = useErrorBoundary(); useEffect(() => { - if (activeFocus) { + if (activeFocus && !liveKitFocusEquals(activeFocus, currentFocus.current)) { getSFUConfigWithOpenID(client, activeFocus).then( (sfuConfig) => { + currentFocus.current = Object.assign({}, activeFocus); setSFUConfig(sfuConfig); }, (e) => { @@ -55,6 +61,7 @@ export function useOpenIDSFU( }, ); } else { + currentFocus.current = {} as LivekitFocus; setSFUConfig(undefined); } }, [client, activeFocus, showErrorBoundary]); @@ -62,6 +69,20 @@ export function useOpenIDSFU( return sfuConfig; } +export function liveKitFocusEquals( + a?: LivekitFocus, + b?: LivekitFocus, +): boolean { + if (a === undefined && b === undefined) return true; + if (a === undefined || b === undefined) return false; + + return ( + a.type === b.type && + a.livekit_service_url === b.livekit_service_url && + a.livekit_alias === b.livekit_alias + ); +} + export async function getSFUConfigWithOpenID( client: OpenIDClientParts, activeFocus: LivekitFocus, diff --git a/src/livekit/useECConnectionState.ts b/src/livekit/useECConnectionState.ts index 3c7b91f8..0c728c42 100644 --- a/src/livekit/useECConnectionState.ts +++ b/src/livekit/useECConnectionState.ts @@ -304,9 +304,10 @@ export function useECConnectionState( sfuConfigValid(sfuConfig) && sfuConfigValid(currentSFUConfig.current) && !sfuConfigEquals(currentSFUConfig.current, sfuConfig) + // (currentSFUConfig.current.url != sfuConfig?.url) ) { logger.info( - `SFU config changed! URL was ${currentSFUConfig.current?.url} now ${sfuConfig?.url}`, + `SFU config changed! URL was ${currentSFUConfig.current?.url}/${currentSFUConfig.current?.jwt} now ${sfuConfig?.url}/${sfuConfig?.jwt}`, ); doFocusSwitch().catch((e) => {