From b8daa1c08f51940fc8c7899c5e601bf324bc2aef Mon Sep 17 00:00:00 2001 From: Robin Date: Fri, 21 Mar 2025 14:59:27 -0400 Subject: [PATCH] Show errors that occur in GroupCallView using the error boundary We were previously using the useGroupCallErrorBoundary hook to surface errors that happened during joining, but because that part is outside the GroupCallErrorBoundary it just ended up sending them to the app-level error boundary where they got displayed with a more generic message. --- src/room/GroupCallView.test.tsx | 100 +++++++++++++++++++++----------- src/room/GroupCallView.tsx | 26 ++++++--- src/utils/test.ts | 5 +- 3 files changed, 88 insertions(+), 43 deletions(-) diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index 0a57d081..f0023b22 100644 --- a/src/room/GroupCallView.test.tsx +++ b/src/room/GroupCallView.test.tsx @@ -5,7 +5,14 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { beforeEach, expect, type MockedFunction, test, vitest } from "vitest"; +import { + beforeEach, + expect, + type MockedFunction, + onTestFinished, + test, + vi, +} from "vitest"; import { render, waitFor, screen } from "@testing-library/react"; import { type MatrixClient } from "matrix-js-sdk/src/client"; import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; @@ -15,6 +22,7 @@ import { BrowserRouter } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import { type RelationsContainer } from "matrix-js-sdk/src/models/relations-container"; import { useState } from "react"; +import { TooltipProvider } from "@vector-im/compound-web"; import { type MuteStates } from "./MuteStates"; import { prefetchSounds } from "../soundUtils"; @@ -28,20 +36,33 @@ import { MockRTCSession, } from "../utils/test"; import { GroupCallView } from "./GroupCallView"; -import { leaveRTCSession } from "../rtcSessionHelpers"; import { type WidgetHelpers } from "../widget"; import { LazyEventEmitter } from "../LazyEventEmitter"; +import { MatrixRTCFocusMissingError } from "../utils/errors"; -vitest.mock("../soundUtils"); -vitest.mock("../useAudioContext"); -vitest.mock("./InCallView"); +vi.mock("../soundUtils"); +vi.mock("../useAudioContext"); +vi.mock("./InCallView"); +vi.mock("react-use-measure", () => ({ + default: (): [() => void, object] => [(): void => {}, {}], +})); -vitest.mock("../rtcSessionHelpers", async (importOriginal) => { +const enterRTCSession = vi.hoisted(() => vi.fn(async () => Promise.resolve())); +const leaveRTCSession = vi.hoisted(() => + vi.fn( + async ( + rtcSession: unknown, + cause: unknown, + promiseBeforeHangup = Promise.resolve(), + ) => await promiseBeforeHangup, + ), +); + +vi.mock("../rtcSessionHelpers", async (importOriginal) => { // TODO: perhaps there is a more elegant way to manage the type import here? // eslint-disable-next-line @typescript-eslint/consistent-type-imports const orig = await importOriginal(); - vitest.spyOn(orig, "leaveRTCSession"); - return orig; + return { ...orig, enterRTCSession, leaveRTCSession }; }); let playSound: MockedFunction< @@ -55,11 +76,11 @@ const roomMembers = new Map([carol].map((p) => [p.userId, p])); const roomId = "!foo:bar"; beforeEach(() => { - vitest.clearAllMocks(); + vi.clearAllMocks(); (prefetchSounds as MockedFunction).mockResolvedValue({ sound: new ArrayBuffer(0), }); - playSound = vitest.fn(); + playSound = vi.fn(); (useAudioContext as MockedFunction).mockReturnValue({ playSound, }); @@ -75,7 +96,10 @@ beforeEach(() => { ); }); -function createGroupCallView(widget: WidgetHelpers | null): { +function createGroupCallView( + widget: WidgetHelpers | null, + joined = true, +): { rtcSession: MockRTCSession; getByText: ReturnType["getByText"]; } { @@ -88,7 +112,7 @@ function createGroupCallView(widget: WidgetHelpers | null): { const room = mockMatrixRoom({ relations: { getChildEventsForEvent: () => - vitest.mocked({ + vi.mocked({ getRelations: () => [], }), } as unknown as RelationsContainer, @@ -106,24 +130,27 @@ function createGroupCallView(widget: WidgetHelpers | null): { localRtcMember, [], ).withMemberships(of([])); + rtcSession.joined = joined; const muteState = { audio: { enabled: false }, video: { enabled: false }, } as MuteStates; const { getByText } = render( - + + + , ); return { @@ -132,7 +159,7 @@ function createGroupCallView(widget: WidgetHelpers | null): { }; } -test("will play a leave sound asynchronously in SPA mode", async () => { +test("GroupCallView plays a leave sound asynchronously in SPA mode", async () => { const user = userEvent.setup(); const { getByText, rtcSession } = createGroupCallView(null); const leaveButton = getByText("Leave"); @@ -143,13 +170,13 @@ test("will play a leave sound asynchronously in SPA mode", async () => { "user", expect.any(Promise), ); - expect(rtcSession.leaveRoomSession).toHaveBeenCalledOnce(); + expect(leaveRTCSession).toHaveBeenCalledOnce(); // Ensure that the playSound promise resolves within this test to avoid // impacting the results of other tests await waitFor(() => expect(leaveRTCSession).toHaveResolved()); }); -test("will play a leave sound synchronously in widget mode", async () => { +test("GroupCallView plays a leave sound synchronously in widget mode", async () => { const user = userEvent.setup(); const widget = { api: { @@ -158,7 +185,7 @@ test("will play a leave sound synchronously in widget mode", async () => { lazyActions: new LazyEventEmitter(), }; let resolvePlaySound: () => void; - playSound = vitest + playSound = vi .fn() .mockReturnValue( new Promise((resolve) => (resolvePlaySound = resolve)), @@ -183,7 +210,7 @@ test("will play a leave sound synchronously in widget mode", async () => { "user", expect.any(Promise), ); - expect(rtcSession.leaveRoomSession).toHaveBeenCalledOnce(); + expect(leaveRTCSession).toHaveBeenCalledOnce(); }); test("GroupCallView leaves the session when an error occurs", async () => { @@ -205,8 +232,15 @@ test("GroupCallView leaves the session when an error occurs", async () => { "error", expect.any(Promise), ); - expect(rtcSession.leaveRoomSession).toHaveBeenCalledOnce(); - // Ensure that the playSound promise resolves within this test to avoid - // impacting the results of other tests - await waitFor(() => expect(leaveRTCSession).toHaveResolved()); +}); + +test("GroupCallView shows errors that occur during joining", async () => { + const user = userEvent.setup(); + enterRTCSession.mockRejectedValue(new MatrixRTCFocusMissingError("")); + onTestFinished(() => { + enterRTCSession.mockReset(); + }); + createGroupCallView(null, false); + await user.click(screen.getByRole("button", { name: "Join call" })); + screen.getByText("Call is not supported"); }); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 1b3d0f20..5e307273 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -67,7 +67,6 @@ import { useSetting, } from "../settings/settings"; import { useTypedEventEmitter } from "../useEvents"; -import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; declare global { interface Window { @@ -100,6 +99,11 @@ export const GroupCallView: FC = ({ muteStates, widget, }) => { + // Used to thread through any errors that occur outside the error boundary + const [externalError, setExternalError] = useState( + null, + ); + const memberships = useMatrixRTCSessionMemberships(rtcSession); const leaveSoundContext = useLatest( useAudioContext({ @@ -121,13 +125,11 @@ export const GroupCallView: FC = ({ }; }, [rtcSession]); - const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary(); - useTypedEventEmitter( rtcSession, MatrixRTCSessionEvent.MembershipManagerError, (error) => { - showGroupCallErrorBoundary( + setExternalError( new RTCSessionError( ErrorCode.MEMBERSHIP_MANAGER_UNRECOVERABLE, error.message ?? error, @@ -190,17 +192,17 @@ export const GroupCallView: FC = ({ ); } catch (e) { if (e instanceof ElementCallError) { - showGroupCallErrorBoundary(e); + setExternalError(e); } else { logger.error(`Unknown Error while entering RTC session`, e); const error = new UnknownCallError( e instanceof Error ? e : new Error("Unknown error", { cause: e }), ); - showGroupCallErrorBoundary(error); + setExternalError(error); } } }, - [showGroupCallErrorBoundary], + [setExternalError], ); useEffect(() => { @@ -422,7 +424,15 @@ export const GroupCallView: FC = ({ ); let body: ReactNode; - if (isJoined) { + if (externalError) { + // If an error was recorded within this component but outside + // GroupCallErrorBoundary, create a component that rethrows the error from + // within the error boundary, so it can be handled uniformly + const ErrorComponent = (): ReactNode => { + throw externalError; + }; + body = ; + } else if (isJoined) { body = ( <> {shareModal} diff --git a/src/utils/test.ts b/src/utils/test.ts index 72c49d7a..9845d51d 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -286,8 +286,9 @@ export class MockRTCSession extends TypedEventEmitter< super(); } - public isJoined(): true { - return true; + public joined = true; + public isJoined(): boolean { + return this.joined; } public withMemberships(