From 8bf81defd4fab71aa67c7cb838bba0638a299cf9 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 24 Feb 2025 18:05:27 +0700 Subject: [PATCH 1/2] Leave session when error occurs and show error screens in widget mode This is a refactor of how we handle and present errors that happen during a call. Rather than assuming that any errors will be manually passed to onLeave, this wraps the call in a React error boundary that takes care of leaving the session and presenting connection errors or crash screens as needed. Importantly, this now allows the 'connection lost' screen to be visible even in widget mode, since we aren't entangling errors with the 'call ended' component (which is deliberately hidden from widgets). --- src/room/CallEndedView.tsx | 83 ++++++----------------- src/room/GroupCallView.test.tsx | 28 +++++++- src/room/GroupCallView.tsx | 112 ++++++++++++++++++++++---------- src/room/InCallView.tsx | 15 ++--- 4 files changed, 132 insertions(+), 106 deletions(-) diff --git a/src/room/CallEndedView.tsx b/src/room/CallEndedView.tsx index 7bd36844b..88f84a24b 100644 --- a/src/room/CallEndedView.tsx +++ b/src/room/CallEndedView.tsx @@ -5,17 +5,10 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { - type FC, - type FormEventHandler, - type ReactNode, - useCallback, - useState, -} from "react"; +import { type FC, type FormEventHandler, useCallback, useState } from "react"; import { type MatrixClient } from "matrix-js-sdk/src/client"; import { Trans, useTranslation } from "react-i18next"; import { Button, Heading, Text } from "@vector-im/compound-web"; -import { OfflineIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; import { useNavigate } from "react-router-dom"; import { logger } from "matrix-js-sdk/src/logger"; @@ -28,15 +21,12 @@ import { FieldRow, InputField } from "../input/Input"; import { StarRatingInput } from "../input/StarRatingInput"; import { Link } from "../button/Link"; import { LinkButton } from "../button"; -import { ErrorView } from "../ErrorView"; interface Props { client: MatrixClient; isPasswordlessUser: boolean; confineToRoom: boolean; endedCallId: string; - leaveError?: Error; - reconnect: () => void; } export const CallEndedView: FC = ({ @@ -44,8 +34,6 @@ export const CallEndedView: FC = ({ isPasswordlessUser, confineToRoom, endedCallId, - leaveError, - reconnect, }) => { const { t } = useTranslation(); const navigate = useNavigate(); @@ -143,61 +131,32 @@ export const CallEndedView: FC = ({ ); - const renderBody = (): ReactNode => { - if (leaveError) { - return ( - <> -
- -

{t("error.connection_lost_description")}

- - -
- - ); - } else { - return ( - <> -
- - {surveySubmitted - ? t("call_ended_view.headline", { - displayName, - }) - : t("call_ended_view.headline", { - displayName, - }) + - "\n" + - t("call_ended_view.survey_prompt")} - - {(!surveySubmitted || confineToRoom) && - PosthogAnalytics.instance.isEnabled() - ? qualitySurveyDialog - : createAccountDialog} -
- {!confineToRoom && ( - - {t("call_ended_view.not_now_button")} - - )} - - ); - } - }; - return ( <>
{!confineToRoom && }
-
{renderBody()}
+
+
+ + {surveySubmitted + ? t("call_ended_view.headline", { displayName }) + : t("call_ended_view.headline", { displayName }) + + "\n" + + t("call_ended_view.survey_prompt")} + + {(!surveySubmitted || confineToRoom) && + PosthogAnalytics.instance.isEnabled() + ? qualitySurveyDialog + : createAccountDialog} +
+ {!confineToRoom && ( + + {t("call_ended_view.not_now_button")} + + )} +
); }; diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index 7d53194d1..00d6fa004 100644 --- a/src/room/GroupCallView.test.tsx +++ b/src/room/GroupCallView.test.tsx @@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details. */ import { beforeEach, expect, type MockedFunction, test, vitest } from "vitest"; -import { render, waitFor } from "@testing-library/react"; +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"; import { of } from "rxjs"; @@ -14,6 +14,7 @@ import { JoinRule, type RoomState } from "matrix-js-sdk/src/matrix"; 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 { type MuteStates } from "./MuteStates"; import { prefetchSounds } from "../soundUtils"; @@ -184,3 +185,28 @@ test("will play a leave sound synchronously in widget mode", async () => { ); expect(rtcSession.leaveRoomSession).toHaveBeenCalledOnce(); }); + +test("GroupCallView leaves the session when an error occurs", async () => { + (ActiveCall as MockedFunction).mockImplementation(() => { + const [error, setError] = useState(null); + if (error !== null) throw error; + return ( +
+ +
+ ); + }); + const user = userEvent.setup(); + const { rtcSession } = createGroupCallView(null); + await user.click(screen.getByRole("button", { name: "Panic!" })); + screen.getByText("error.generic"); + expect(leaveRTCSession).toHaveBeenCalledWith( + rtcSession, + "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()); +}); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 163ddfd70..71c1cc482 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -5,7 +5,15 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { type FC, useCallback, useEffect, useMemo, useState } from "react"; +import { + type FC, + type ReactElement, + type ReactNode, + useCallback, + useEffect, + useMemo, + useState, +} from "react"; import { type MatrixClient } from "matrix-js-sdk/src/client"; import { Room, @@ -14,9 +22,14 @@ import { import { logger } from "matrix-js-sdk/src/logger"; import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; import { JoinRule } from "matrix-js-sdk/src/matrix"; -import { WebBrowserIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; +import { + OfflineIcon, + WebBrowserIcon, +} from "@vector-im/compound-design-tokens/assets/web/icons"; import { useTranslation } from "react-i18next"; import { useNavigate } from "react-router-dom"; +import { ErrorBoundary } from "@sentry/react"; +import { Button } from "@vector-im/compound-web"; import type { IWidgetApiRequest } from "matrix-widget-api"; import { @@ -24,14 +37,14 @@ import { type JoinCallData, type WidgetHelpers, } from "../widget"; -import { FullScreenView } from "../FullScreenView"; +import { ErrorPage, FullScreenView } from "../FullScreenView"; import { LobbyView } from "./LobbyView"; import { type MatrixInfo } from "./VideoPreview"; import { CallEndedView } from "./CallEndedView"; import { PosthogAnalytics } from "../analytics/PosthogAnalytics"; import { useProfile } from "../profile/useProfile"; import { findDeviceByName } from "../utils/media"; -import { ActiveCall } from "./InCallView"; +import { ActiveCall, ConnectionLostError } from "./InCallView"; import { MUTE_PARTICIPANT_COUNT, type MuteStates } from "./MuteStates"; import { useMediaDevices } from "../livekit/MediaDevicesContext"; import { useMatrixRTCSessionMemberships } from "../useMatrixRTCSessionMemberships"; @@ -55,6 +68,11 @@ declare global { } } +interface GroupCallErrorPageProps { + error: Error | unknown; + resetError: () => void; +} + interface Props { client: MatrixClient; isPasswordlessUser: boolean; @@ -229,16 +247,14 @@ export const GroupCallView: FC = ({ ]); const [left, setLeft] = useState(false); - const [leaveError, setLeaveError] = useState(undefined); const navigate = useNavigate(); const onLeave = useCallback( - (leaveError?: Error): void => { + (cause: "user" | "error" = "user"): void => { const audioPromise = leaveSoundContext.current?.playSound("left"); // In embedded/widget mode the iFrame will be killed right after the call ended prohibiting the posthog event from getting sent, // therefore we want the event to be sent instantly without getting queued/batched. const sendInstantly = !!widget; - setLeaveError(leaveError); setLeft(true); // we need to wait until the callEnded event is tracked on posthog. // Otherwise the iFrame gets killed before the callEnded event got tracked. @@ -254,7 +270,7 @@ export const GroupCallView: FC = ({ leaveRTCSession( rtcSession, - leaveError === undefined ? "user" : "error", + cause, // Wait for the sound in widget mode (it's not long) Promise.all([audioPromise, posthogRequest]), ) @@ -303,14 +319,6 @@ export const GroupCallView: FC = ({ } }, [widget, isJoined, rtcSession]); - const onReconnect = useCallback(() => { - setLeft(false); - setLeaveError(undefined); - enterRTCSession(rtcSession, perParticipantE2EE).catch((e) => { - logger.error("Error re-entering RTC session on reconnect", e); - }); - }, [rtcSession, perParticipantE2EE]); - const joinRule = useJoinRule(rtcSession.room); const [shareModalOpen, setInviteModalOpen] = useState(false); @@ -327,6 +335,43 @@ export const GroupCallView: FC = ({ const { t } = useTranslation(); + const errorPage = useMemo(() => { + function GroupCallErrorPage({ + error, + resetError, + }: GroupCallErrorPageProps): ReactElement { + useEffect(() => { + if (rtcSession.isJoined()) onLeave("error"); + }, [error]); + + const onReconnect = useCallback(() => { + setLeft(false); + resetError(); + enterRTCSession(rtcSession, perParticipantE2EE).catch((e) => { + logger.error("Error re-entering RTC session on reconnect", e); + }); + }, [resetError]); + + return error instanceof ConnectionLostError ? ( + + +

{t("error.connection_lost_description")}

+ + + + ) : ( + + ); + } + return GroupCallErrorPage; + }, [onLeave, rtcSession, perParticipantE2EE, t]); + if (!isE2EESupportedBrowser() && e2eeSystem.kind !== E2eeType.NONE) { // If we have a encryption system but the browser does not support it. return ( @@ -361,8 +406,9 @@ export const GroupCallView: FC = ({ ); + let body: ReactNode; if (isJoined) { - return ( + body = ( <> {shareModal} = ({ // submitting anything. if ( isPasswordlessUser || - (PosthogAnalytics.instance.isEnabled() && widget === null) || - leaveError + (PosthogAnalytics.instance.isEnabled() && widget === null) ) { - return ( - <> - - ; - + body = ( + ); } else { // If the user is a regular user, we'll have sent them back to the homepage, // so just sit here & do nothing: otherwise we would (briefly) mount the // LobbyView again which would open capture devices again. - return null; + body = null; } } else if (left && widget !== null) { // Left in widget mode: if (!returnToLobby) { - return null; + body = null; } } else if (preload || skipLobby) { - return null; + body = null; + } else { + body = lobbyView; } - return lobbyView; + return {body}; }; diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index bcf0a6941..ad4252609 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -102,6 +102,8 @@ const canScreenshare = "getDisplayMedia" in (navigator.mediaDevices ?? {}); const maxTapDurationMs = 400; +export class ConnectionLostError extends Error {} + export interface ActiveCallProps extends Omit { e2eeSystem: EncryptionSystem; @@ -173,7 +175,7 @@ export interface InCallViewProps { livekitRoom: Room; muteStates: MuteStates; participantCount: number; - onLeave: (error?: Error) => void; + onLeave: () => void; hideHeader: boolean; otelGroupCallMembership?: OTelGroupCallMembership; connState: ECConnectionState; @@ -198,13 +200,10 @@ export const InCallView: FC = ({ useWakeLock(); - useEffect(() => { - if (connState === ConnectionState.Disconnected) { - // annoyingly we don't get the disconnection reason this way, - // only by listening for the emitted event - onLeave(new Error("Disconnected from call server")); - } - }, [connState, onLeave]); + // annoyingly we don't get the disconnection reason this way, + // only by listening for the emitted event + if (connState === ConnectionState.Disconnected) + throw new ConnectionLostError(); const containerRef1 = useRef(null); const [containerRef2, bounds] = useMeasure(); From 564349c96f078dd1b4f28ae5e9f829952afb9366 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 26 Feb 2025 10:18:56 +0000 Subject: [PATCH 2/2] Update src/room/InCallView.tsx --- src/room/InCallView.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index ad4252609..c0ee0711b 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -175,6 +175,7 @@ export interface InCallViewProps { livekitRoom: Room; muteStates: MuteStates; participantCount: number; + /** Function to call when the user explicitly ends the call */ onLeave: () => void; hideHeader: boolean; otelGroupCallMembership?: OTelGroupCallMembership;