From d85cf5f929411922b6a2aeefb70ce93a78bc98bc Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Sep 2025 14:21:38 +0200 Subject: [PATCH 1/4] Fix the reconnect button After a membership manager error, clicking the 'reconnect' button did nothing. This is because we were forgetting to clear the external error state, causing it to transition directly back to the same error state. --- src/room/GroupCallErrorBoundary.tsx | 10 ++++++---- src/room/GroupCallView.test.tsx | 17 +++++++++++++++-- src/room/GroupCallView.tsx | 5 +++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/room/GroupCallErrorBoundary.tsx b/src/room/GroupCallErrorBoundary.tsx index 3d55d005..72958683 100644 --- a/src/room/GroupCallErrorBoundary.tsx +++ b/src/room/GroupCallErrorBoundary.tsx @@ -36,7 +36,9 @@ import { type WidgetHelpers } from "../widget.ts"; export type CallErrorRecoveryAction = "reconnect"; // | "retry" ; -export type RecoveryActionHandler = (action: CallErrorRecoveryAction) => void; +export type RecoveryActionHandler = ( + action: CallErrorRecoveryAction, +) => Promise; interface ErrorPageProps { error: ElementCallError; @@ -71,7 +73,7 @@ const ErrorPage: FC = ({ if (error instanceof ConnectionLostError) { actions.push({ label: t("call_ended_view.reconnect_button"), - onClick: () => recoveryActionHandler("reconnect"), + onClick: () => void recoveryActionHandler("reconnect"), }); } @@ -131,9 +133,9 @@ export const GroupCallErrorBoundary = ({ widget={widget ?? null} error={callError} resetError={resetError} - recoveryActionHandler={(action: CallErrorRecoveryAction) => { + recoveryActionHandler={async (action: CallErrorRecoveryAction) => { + await recoveryActionHandler(action); resetError(); - recoveryActionHandler(action); }} /> ); diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index 12dfdf61..a1aa9452 100644 --- a/src/room/GroupCallView.test.tsx +++ b/src/room/GroupCallView.test.tsx @@ -15,11 +15,14 @@ import { } from "vitest"; import { render, waitFor, screen } from "@testing-library/react"; import { type MatrixClient, JoinRule, type RoomState } from "matrix-js-sdk"; -import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; +import { + MatrixRTCSessionEvent, + type MatrixRTCSession, +} from "matrix-js-sdk/lib/matrixrtc"; import { BrowserRouter } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import { type RelationsContainer } from "matrix-js-sdk/lib/models/relations-container"; -import { useState } from "react"; +import { act, useState } from "react"; import { TooltipProvider } from "@vector-im/compound-web"; import { type MuteStates } from "./MuteStates"; @@ -258,3 +261,13 @@ test("GroupCallView shows errors that occur during joining", async () => { await user.click(screen.getByRole("button", { name: "Join call" })); screen.getByText("Call is not supported"); }); + +test("user can reconnect after a membership manager error", async () => { + const user = userEvent.setup(); + const { rtcSession } = createGroupCallView(null, true); + await act(() => + rtcSession.emit(MatrixRTCSessionEvent.MembershipManagerError, undefined), + ); + await user.click(screen.getByRole("button", { name: "Reconnect" })); + await waitFor(() => screen.getByRole("button", { name: "Leave" })); +}); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 63fc942f..dd6db229 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -502,10 +502,11 @@ export const GroupCallView: FC = ({ return ( { + recoveryActionHandler={async (action) => { + setExternalError(null); if (action == "reconnect") { setLeft(false); - enterRTCSessionOrError(rtcSession).catch((e) => { + await enterRTCSessionOrError(rtcSession).catch((e) => { logger.error("Error re-entering RTC session", e); }); } From db04cbfbfcaef5dfbf1035adc2c954a1901f6c82 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Sep 2025 14:58:46 +0200 Subject: [PATCH 2/4] Fix type error --- src/room/GroupCallErrorBoundary.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/room/GroupCallErrorBoundary.test.tsx b/src/room/GroupCallErrorBoundary.test.tsx index f2a10bc2..51912956 100644 --- a/src/room/GroupCallErrorBoundary.test.tsx +++ b/src/room/GroupCallErrorBoundary.test.tsx @@ -132,9 +132,10 @@ test("ConnectionLostError: Action handling should reset error state", async () = const WrapComponent = (): ReactNode => { const [failState, setFailState] = useState(true); const reconnectCallback = useCallback( - (action: CallErrorRecoveryAction) => { + async (action: CallErrorRecoveryAction) => { reconnectCallbackSpy(action); setFailState(false); + return Promise.resolve(); }, [setFailState], ); From 34a8977dd15689fc4919901f334a8622f2e4cce3 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Sep 2025 15:24:46 +0200 Subject: [PATCH 3/4] Increase timeout to hopefully avoid test flakes --- src/room/GroupCallView.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index a1aa9452..247f4461 100644 --- a/src/room/GroupCallView.test.tsx +++ b/src/room/GroupCallView.test.tsx @@ -269,5 +269,8 @@ test("user can reconnect after a membership manager error", async () => { rtcSession.emit(MatrixRTCSessionEvent.MembershipManagerError, undefined), ); await user.click(screen.getByRole("button", { name: "Reconnect" })); - await waitFor(() => screen.getByRole("button", { name: "Leave" })); + // In-call controls should be visible again + await waitFor(() => screen.getByRole("button", { name: "Leave" }), { + timeout: 3000, + }); }); From 32cb8541f4c688303b567c154c5123e1fbf8c86e Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Sep 2025 16:42:09 +0200 Subject: [PATCH 4/4] Actually fix the test flake --- src/room/GroupCallView.test.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index 247f4461..ff2b89d5 100644 --- a/src/room/GroupCallView.test.tsx +++ b/src/room/GroupCallView.test.tsx @@ -13,7 +13,7 @@ import { test, vi, } from "vitest"; -import { render, waitFor, screen } from "@testing-library/react"; +import { render, waitFor, screen, act } from "@testing-library/react"; import { type MatrixClient, JoinRule, type RoomState } from "matrix-js-sdk"; import { MatrixRTCSessionEvent, @@ -22,7 +22,7 @@ import { import { BrowserRouter } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import { type RelationsContainer } from "matrix-js-sdk/lib/models/relations-container"; -import { act, useState } from "react"; +import { useState } from "react"; import { TooltipProvider } from "@vector-im/compound-web"; import { type MuteStates } from "./MuteStates"; @@ -268,9 +268,12 @@ test("user can reconnect after a membership manager error", async () => { await act(() => rtcSession.emit(MatrixRTCSessionEvent.MembershipManagerError, undefined), ); - await user.click(screen.getByRole("button", { name: "Reconnect" })); + // XXX: Wrapping the following click in act() shouldn't be necessary (the + // async state update should be processed automatically by the waitFor call), + // and yet here we are. + await act(async () => + user.click(screen.getByRole("button", { name: "Reconnect" })), + ); // In-call controls should be visible again - await waitFor(() => screen.getByRole("button", { name: "Leave" }), { - timeout: 3000, - }); + await waitFor(() => screen.getByRole("button", { name: "Leave" })); });