From 8bf81defd4fab71aa67c7cb838bba0638a299cf9 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 24 Feb 2025 18:05:27 +0700 Subject: [PATCH 1/4] 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 61c7bc989a9e67d086c3ed8d577060f665127e98 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 24 Feb 2025 19:09:00 +0700 Subject: [PATCH 2/4] Show an error screen when the SFU is at capacity --- locales/en/app.json | 2 + src/RichError.tsx | 21 ++++++- src/livekit/useECConnectionState.test.tsx | 72 +++++++++++++++++++++++ src/livekit/useECConnectionState.ts | 31 +++++++--- 4 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 src/livekit/useECConnectionState.test.tsx diff --git a/locales/en/app.json b/locales/en/app.json index d27b9a6c0..98482a7bd 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -82,6 +82,8 @@ "e2ee_unsupported_description": "Your web browser does not support encrypted calls. Supported browsers include Chrome, Safari, and Firefox 117+.", "generic": "Something went wrong", "generic_description": "Submitting debug logs will help us track down the problem.", + "insufficient_capacity": "Insufficient capacity", + "insufficient_capacity_description": "The server has reached its maximum capacity and you cannot join calls at this time. Try again later, or contact your server admin if the problem persists.", "open_elsewhere": "Opened in another tab", "open_elsewhere_description": "{{brand}} has been opened in another tab. If that doesn't sound right, try reloading the page." }, diff --git a/src/RichError.tsx b/src/RichError.tsx index 5ce31e044..d16ef6409 100644 --- a/src/RichError.tsx +++ b/src/RichError.tsx @@ -7,7 +7,10 @@ Please see LICENSE in the repository root for full details. import { type FC, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; -import { PopOutIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; +import { + HostIcon, + PopOutIcon, +} from "@vector-im/compound-design-tokens/assets/web/icons"; import { ErrorView } from "./ErrorView"; @@ -46,3 +49,19 @@ export class OpenElsewhereError extends RichError { super("App opened in another tab", ); } } + +const InsufficientCapacity: FC = () => { + const { t } = useTranslation(); + + return ( + +

{t("error.insufficient_capacity_description")}

+ + ); +}; + +export class InsufficientCapacityError extends RichError { + public constructor() { + super("Insufficient server capacity", ); + } +} diff --git a/src/livekit/useECConnectionState.test.tsx b/src/livekit/useECConnectionState.test.tsx new file mode 100644 index 000000000..7194c252b --- /dev/null +++ b/src/livekit/useECConnectionState.test.tsx @@ -0,0 +1,72 @@ +/* +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 { type FC, useCallback, useState } from "react"; +import { test } from "vitest"; +import { + ConnectionError, + ConnectionErrorReason, + type Room, +} from "livekit-client"; +import userEvent from "@testing-library/user-event"; +import { render, screen } from "@testing-library/react"; +import { ErrorBoundary } from "@sentry/react"; +import { MemoryRouter } from "react-router-dom"; + +import { ErrorPage } from "../FullScreenView"; +import { useECConnectionState } from "./useECConnectionState"; +import { type SFUConfig } from "./openIDSFU"; + +test.each<[string, ConnectionError]>([ + [ + "LiveKit", + new ConnectionError("", ConnectionErrorReason.InternalError, 503), + ], + [ + "LiveKit Cloud", + new ConnectionError("", ConnectionErrorReason.NotAllowed, 429), + ], +])( + "useECConnectionState throws error when %s hits track limit", + async (_server, error) => { + const mockRoom = { + on: () => {}, + off: () => {}, + once: () => {}, + connect: () => { + throw error; + }, + localParticipant: { + getTrackPublication: () => {}, + createTracks: () => [], + }, + } as unknown as Room; + + const TestComponent: FC = () => { + const [sfuConfig, setSfuConfig] = useState( + undefined, + ); + const connect = useCallback( + () => setSfuConfig({ url: "URL", jwt: "JWT token" }), + [], + ); + useECConnectionState({}, false, mockRoom, sfuConfig); + return ; + }; + + const user = userEvent.setup(); + render( + + + + + , + ); + await user.click(screen.getByRole("button", { name: "Connect" })); + screen.getByText("error.insufficient_capacity"); + }, +); diff --git a/src/livekit/useECConnectionState.ts b/src/livekit/useECConnectionState.ts index 56139037d..911c6448a 100644 --- a/src/livekit/useECConnectionState.ts +++ b/src/livekit/useECConnectionState.ts @@ -7,6 +7,7 @@ Please see LICENSE in the repository root for full details. import { type AudioCaptureOptions, + ConnectionError, ConnectionState, type LocalTrack, type Room, @@ -19,6 +20,7 @@ import * as Sentry from "@sentry/react"; import { type SFUConfig, sfuConfigEquals } from "./openIDSFU"; import { PosthogAnalytics } from "../analytics/PosthogAnalytics"; +import { InsufficientCapacityError, RichError } from "../RichError"; declare global { interface Window { @@ -106,7 +108,8 @@ async function doConnect( await connectAndPublish(livekitRoom, sfuConfig, preCreatedAudioTrack, []); } catch (e) { preCreatedAudioTrack?.stop(); - logger.warn("Stopped precreated audio tracks.", e); + logger.debug("Stopped precreated audio tracks."); + throw e; } } @@ -129,12 +132,20 @@ async function connectAndPublish( tracker.cacheConnectStart(); livekitRoom.once(RoomEvent.SignalConnected, tracker.cacheWsConnect); - await livekitRoom!.connect(sfuConfig!.url, sfuConfig!.jwt, { - // Due to stability issues on Firefox we are testing the effect of different - // timeouts, and allow these values to be set through the console - peerConnectionTimeout: window.peerConnectionTimeout ?? 45000, - websocketTimeout: window.websocketTimeout ?? 45000, - }); + try { + await livekitRoom!.connect(sfuConfig!.url, sfuConfig!.jwt, { + // Due to stability issues on Firefox we are testing the effect of different + // timeouts, and allow these values to be set through the console + peerConnectionTimeout: window.peerConnectionTimeout ?? 45000, + websocketTimeout: window.websocketTimeout ?? 45000, + }); + } catch (e) { + // LiveKit uses 503 to indicate that the server has hit its track limits + // or equivalently, 429 in LiveKit Cloud + if (e instanceof ConnectionError && (e.status === 503 || e.status === 429)) + throw new InsufficientCapacityError(); + throw e; + } // remove listener in case the connect promise rejects before `SignalConnected` is emitted. livekitRoom.off(RoomEvent.SignalConnected, tracker.cacheWsConnect); @@ -175,6 +186,8 @@ export function useECConnectionState( const [isSwitchingFocus, setSwitchingFocus] = useState(false); const [isInDoConnect, setIsInDoConnect] = useState(false); + const [error, setError] = useState(null); + if (error !== null) throw error; const onConnStateChanged = useCallback((state: ConnectionState) => { if (state == ConnectionState.Connected) setSwitchingFocus(false); @@ -256,7 +269,9 @@ export function useECConnectionState( initialAudioOptions, ) .catch((e) => { - logger.error("Failed to connect to SFU", e); + if (e instanceof RichError) + setError(e); // Bubble up any error screens to React + else logger.error("Failed to connect to SFU", e); }) .finally(() => setIsInDoConnect(false)); } From 0e4d97526b780cd2348044896d41e0712497907c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 26 Feb 2025 11:34:36 +0000 Subject: [PATCH 3/4] Update src/livekit/useECConnectionState.ts --- src/livekit/useECConnectionState.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/livekit/useECConnectionState.ts b/src/livekit/useECConnectionState.ts index 911c6448a..8cd5f87ea 100644 --- a/src/livekit/useECConnectionState.ts +++ b/src/livekit/useECConnectionState.ts @@ -142,6 +142,8 @@ async function connectAndPublish( } catch (e) { // LiveKit uses 503 to indicate that the server has hit its track limits // or equivalently, 429 in LiveKit Cloud + // For reference, the 503 response is generated at: https://github.com/livekit/livekit/blob/fcb05e97c5a31812ecf0ca6f7efa57c485cea9fb/pkg/service/rtcservice.go#L171 + if (e instanceof ConnectionError && (e.status === 503 || e.status === 429)) throw new InsufficientCapacityError(); throw e; From c37b4d70164632e72795a02ece50c879b2759bce Mon Sep 17 00:00:00 2001 From: fkwp Date: Wed, 26 Feb 2025 12:46:18 +0100 Subject: [PATCH 4/4] Update locales/en/app.json Co-authored-by: Hugh Nimmo-Smith --- locales/en/app.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locales/en/app.json b/locales/en/app.json index 98482a7bd..7da0f5939 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -83,7 +83,7 @@ "generic": "Something went wrong", "generic_description": "Submitting debug logs will help us track down the problem.", "insufficient_capacity": "Insufficient capacity", - "insufficient_capacity_description": "The server has reached its maximum capacity and you cannot join calls at this time. Try again later, or contact your server admin if the problem persists.", + "insufficient_capacity_description": "The server has reached its maximum capacity and you cannot join the call at this time. Try again later, or contact your server admin if the problem persists.", "open_elsewhere": "Opened in another tab", "open_elsewhere_description": "{{brand}} has been opened in another tab. If that doesn't sound right, try reloading the page." },