From 28c7c9dd380c20ee59c6950cd61900672bdfc311 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 5 Mar 2025 20:50:19 +0100 Subject: [PATCH 01/11] refactor: Centralize group call errors in custom GroupCallErrorBoundary --- src/RichError.tsx | 70 +- src/livekit/useECConnectionState.test.tsx | 7 +- src/livekit/useECConnectionState.ts | 14 +- src/room/GroupCallErrorBoundary.test.tsx | 170 +++ src/room/GroupCallErrorBoundary.tsx | 137 +++ src/room/GroupCallView.tsx | 99 +- .../GroupCallErrorBoundary.test.tsx.snap | 1051 +++++++++++++++++ src/utils/errors.ts | 55 +- 8 files changed, 1450 insertions(+), 153 deletions(-) create mode 100644 src/room/GroupCallErrorBoundary.test.tsx create mode 100644 src/room/GroupCallErrorBoundary.tsx create mode 100644 src/room/__snapshots__/GroupCallErrorBoundary.test.tsx.snap diff --git a/src/RichError.tsx b/src/RichError.tsx index 1525f153..abacf0b3 100644 --- a/src/RichError.tsx +++ b/src/RichError.tsx @@ -5,16 +5,11 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { Trans, useTranslation } from "react-i18next"; -import { - ErrorIcon, - HostIcon, - PopOutIcon, -} from "@vector-im/compound-design-tokens/assets/web/icons"; +import { useTranslation } from "react-i18next"; +import { PopOutIcon } from "@vector-im/compound-design-tokens/assets/web/icons"; -import type { ComponentType, FC, ReactNode, SVGAttributes } from "react"; +import type { FC, ReactNode } from "react"; import { ErrorView } from "./ErrorView"; -import { type ElementCallError, ErrorCategory } from "./utils/errors.ts"; /** * An error consisting of a terse message to be logged to the console and a @@ -51,62 +46,3 @@ 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", ); - } -} - -type ECErrorProps = { - error: ElementCallError; -}; - -const GenericECError: FC<{ error: ElementCallError }> = ({ - error, -}: ECErrorProps) => { - const { t } = useTranslation(); - - let title: string; - let icon: ComponentType>; - switch (error.category) { - case ErrorCategory.CONFIGURATION_ISSUE: - title = t("error.call_is_not_supported"); - icon = HostIcon; - break; - default: - title = t("error.generic"); - icon = ErrorIcon; - } - return ( - -

- {error.localisedMessage ?? ( - , ]} - values={{ errorCode: error.code }} - /> - )} -

- - ); -}; - -export class ElementCallRichError extends RichError { - public ecError: ElementCallError; - public constructor(ecError: ElementCallError) { - super(ecError.message, ); - this.ecError = ecError; - } -} diff --git a/src/livekit/useECConnectionState.test.tsx b/src/livekit/useECConnectionState.test.tsx index 1314ce81..6ee63c3b 100644 --- a/src/livekit/useECConnectionState.test.tsx +++ b/src/livekit/useECConnectionState.test.tsx @@ -14,12 +14,11 @@ import { } 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"; +import { GroupCallErrorBoundary } from "../room/GroupCallErrorBoundary.tsx"; test.each<[string, ConnectionError]>([ [ @@ -61,9 +60,9 @@ test.each<[string, ConnectionError]>([ const user = userEvent.setup(); render( - + - + , ); await user.click(screen.getByRole("button", { name: "Connect" })); diff --git a/src/livekit/useECConnectionState.ts b/src/livekit/useECConnectionState.ts index 8cd5f87e..e575abef 100644 --- a/src/livekit/useECConnectionState.ts +++ b/src/livekit/useECConnectionState.ts @@ -20,7 +20,11 @@ import * as Sentry from "@sentry/react"; import { type SFUConfig, sfuConfigEquals } from "./openIDSFU"; import { PosthogAnalytics } from "../analytics/PosthogAnalytics"; -import { InsufficientCapacityError, RichError } from "../RichError"; +import { + ElementCallError, + InsufficientCapacityError, + UnknownCallError, +} from "../utils/errors.ts"; declare global { interface Window { @@ -188,7 +192,7 @@ export function useECConnectionState( const [isSwitchingFocus, setSwitchingFocus] = useState(false); const [isInDoConnect, setIsInDoConnect] = useState(false); - const [error, setError] = useState(null); + const [error, setError] = useState(null); if (error !== null) throw error; const onConnStateChanged = useCallback((state: ConnectionState) => { @@ -271,9 +275,11 @@ export function useECConnectionState( initialAudioOptions, ) .catch((e) => { - if (e instanceof RichError) + if (e instanceof ElementCallError) { setError(e); // Bubble up any error screens to React - else logger.error("Failed to connect to SFU", e); + } else if (e instanceof Error) { + setError(new UnknownCallError(e)); + } else logger.error("Failed to connect to SFU", e); }) .finally(() => setIsInDoConnect(false)); } diff --git a/src/room/GroupCallErrorBoundary.test.tsx b/src/room/GroupCallErrorBoundary.test.tsx new file mode 100644 index 00000000..94c96794 --- /dev/null +++ b/src/room/GroupCallErrorBoundary.test.tsx @@ -0,0 +1,170 @@ +/* +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 { describe, expect, test, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { type ReactElement, type ReactNode } from "react"; +import { BrowserRouter } from "react-router-dom"; +import userEvent from "@testing-library/user-event"; + +import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; +import { + ConnectionLostError, + E2EENotSupportedError, + type ElementCallError, + InsufficientCapacityError, + MatrixRTCFocusMissingError, + UnknownCallError, +} from "../utils/errors.ts"; +import { mockConfig } from "../utils/test.ts"; + +test.each([ + { + error: new MatrixRTCFocusMissingError("example.com"), + expectedTitle: "Call is not supported", + }, + { + error: new ConnectionLostError(), + expectedTitle: "Connection lost", + expectedDescription: "You were disconnected from the call.", + }, + { + error: new E2EENotSupportedError(), + expectedTitle: "Incompatible browser", + expectedDescription: + "Your web browser does not support encrypted calls. Supported browsers include Chrome, Safari, and Firefox 117+.", + }, + { + error: new InsufficientCapacityError(), + expectedTitle: "Insufficient capacity", + expectedDescription: + "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.", + }, +])( + "should report correct error for $expectedTitle", + async ({ error, expectedTitle, expectedDescription }) => { + const TestComponent = (): ReactNode => { + throw error; + }; + + const onErrorMock = vi.fn(); + const { asFragment } = render( + + + + + , + ); + + await screen.findByText(expectedTitle); + if (expectedDescription) { + expect(screen.queryByText(expectedDescription)).toBeInTheDocument(); + } + expect(onErrorMock).toHaveBeenCalledWith(error); + + expect(asFragment()).toMatchSnapshot(); + }, +); + +test("should render the error page with link back to home", async () => { + const error = new MatrixRTCFocusMissingError("example.com"); + const TestComponent = (): ReactNode => { + throw error; + }; + + const onErrorMock = vi.fn(); + const { asFragment } = render( + + + + + , + ); + + await screen.findByText("Call is not supported"); + expect(screen.getByText(/Domain: example.com/i)).toBeInTheDocument(); + expect( + screen.getByText(/Error Code: MISSING_MATRIX_RTC_FOCUS/i), + ).toBeInTheDocument(); + + await screen.findByRole("button", { name: "Return to home screen" }); + + expect(onErrorMock).toHaveBeenCalledOnce(); + expect(onErrorMock).toHaveBeenCalledWith(error); + + expect(asFragment()).toMatchSnapshot(); +}); + +test("should have a reconnect button for ConnectionLostError", async () => { + const user = userEvent.setup(); + + const reconnectCallback = vi.fn(); + + const TestComponent = (): ReactNode => { + throw new ConnectionLostError(); + }; + + const { asFragment } = render( + + + + + , + ); + + await screen.findByText("Connection lost"); + await screen.findByRole("button", { name: "Reconnect" }); + await screen.findByRole("button", { name: "Return to home screen" }); + + expect(asFragment()).toMatchSnapshot(); + + await user.click(screen.getByRole("button", { name: "Reconnect" })); + + expect(reconnectCallback).toHaveBeenCalledOnce(); + expect(reconnectCallback).toHaveBeenCalledWith("reconnect"); +}); + +describe("Rageshake button", () => { + function setupTest(testError: ElementCallError): void { + mockConfig({ + rageshake: { + submit_url: "https://rageshake.example.com.localhost", + }, + }); + + const TestComponent = (): ReactElement => { + throw testError; + }; + + render( + + + + + , + ); + } + + test("should show send rageshake button for unknown errors", () => { + setupTest(new UnknownCallError(new Error("FOO"))); + + expect( + screen.queryByRole("button", { name: "Send debug logs" }), + ).toBeInTheDocument(); + }); + + test("should not show send rageshake button for call errors", () => { + setupTest(new E2EENotSupportedError()); + + expect( + screen.queryByRole("button", { name: "Send debug logs" }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/room/GroupCallErrorBoundary.tsx b/src/room/GroupCallErrorBoundary.tsx new file mode 100644 index 00000000..758016b2 --- /dev/null +++ b/src/room/GroupCallErrorBoundary.tsx @@ -0,0 +1,137 @@ +/* +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 { ErrorBoundary, type FallbackRender } from "@sentry/react"; +import { + type ComponentType, + type FC, + type ReactElement, + type ReactNode, + type SVGAttributes, + useCallback, +} from "react"; +import { Trans, useTranslation } from "react-i18next"; +import { + ErrorIcon, + HostIcon, + OfflineIcon, + WebBrowserIcon, +} from "@vector-im/compound-design-tokens/assets/web/icons"; + +import { + ConnectionLostError, + ElementCallError, + ErrorCategory, + ErrorCode, + UnknownCallError, +} from "../utils/errors.ts"; +import { FullScreenView } from "../FullScreenView.tsx"; +import { ErrorView } from "../ErrorView.tsx"; + +export type CallErrorRecoveryAction = "reconnect"; // | "retry" ; + +export type RecoveryActionHandler = (action: CallErrorRecoveryAction) => void; + +interface ErrorPageProps { + error: ElementCallError; + recoveryActionHandler?: RecoveryActionHandler; + resetError: () => void; +} + +const ErrorPage: FC = ({ + error, + recoveryActionHandler, +}: ErrorPageProps): ReactElement => { + const { t } = useTranslation(); + + // let title: string; + let icon: ComponentType>; + switch (error.category) { + case ErrorCategory.CONFIGURATION_ISSUE: + icon = HostIcon; + break; + case ErrorCategory.NETWORK_CONNECTIVITY: + icon = OfflineIcon; + break; + case ErrorCategory.CLIENT_CONFIGURATION: + icon = WebBrowserIcon; + break; + default: + icon = ErrorIcon; + } + + const actions: { label: string; onClick: () => void }[] = []; + if (error instanceof ConnectionLostError) { + actions.push({ + label: t("call_ended_view.reconnect_button"), + onClick: () => recoveryActionHandler?.("reconnect"), + }); + } + + return ( + + +

+ {error.localisedMessage ?? ( + , ]} + values={{ errorCode: error.code }} + /> + )} +

+ {actions && + actions.map((action, index) => ( + + ))} + + + ); +}; + +interface BoundaryProps { + children: ReactNode | (() => ReactNode); + recoveryActionHandler?: RecoveryActionHandler; + onError?: (error: unknown) => void; +} + +export const GroupCallErrorBoundary = ({ + recoveryActionHandler, + onError, + children, +}: BoundaryProps): ReactElement => { + const fallbackRenderer: FallbackRender = useCallback( + ({ error, resetError }): ReactElement => { + const callError = + error instanceof ElementCallError + ? error + : new UnknownCallError(error instanceof Error ? error : new Error()); + return ( + + ); + }, + [recoveryActionHandler], + ); + + return ( + onError?.(error)} + children={children} + /> + ); +}; diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 95d1d12c..226fb9f5 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -7,7 +7,6 @@ Please see LICENSE in the repository root for full details. import { type FC, - type ReactElement, type ReactNode, useCallback, useEffect, @@ -22,14 +21,7 @@ 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 { - 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 { @@ -37,7 +29,6 @@ import { type JoinCallData, type WidgetHelpers, } from "../widget"; -import { ErrorPage, FullScreenView } from "../FullScreenView"; import { LobbyView } from "./LobbyView"; import { type MatrixInfo } from "./VideoPreview"; import { CallEndedView } from "./CallEndedView"; @@ -60,14 +51,12 @@ import { useAudioContext } from "../useAudioContext"; import { callEventAudioSounds } from "./CallEventAudioRenderer"; import { useLatest } from "../useLatest"; import { usePageTitle } from "../usePageTitle"; -import { ErrorView } from "../ErrorView"; import { - ConnectionLostError, + E2EENotSupportedError, ElementCallError, - ErrorCategory, - ErrorCode, + UnknownCallError, } from "../utils/errors.ts"; -import { ElementCallRichError } from "../RichError.tsx"; +import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; declare global { interface Window { @@ -75,11 +64,6 @@ declare global { } } -interface GroupCallErrorPageProps { - error: Error | unknown; - resetError: () => void; -} - interface Props { client: MatrixClient; isPasswordlessUser: boolean; @@ -184,10 +168,8 @@ export const GroupCallView: FC = ({ setEnterRTCError(e); } else { logger.error(`Unknown Error while entering RTC session`, e); - const error = new ElementCallError( - e instanceof Error ? e.message : "Unknown error", - ErrorCode.UNKNOWN_ERROR, - ErrorCategory.UNKNOWN, + const error = new UnknownCallError( + e instanceof Error ? e : new Error("Unknown error", { cause: e }), ); setEnterRTCError(error); } @@ -365,54 +347,9 @@ export const GroupCallView: FC = ({ ); const onShareClick = joinRule === JoinRule.Public ? onShareClickFn : null; - 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(); - enterRTCSessionOrError(rtcSession, perParticipantE2EE).catch((e) => { - logger.error("Error re-entering RTC session", 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 ( - - -

{t("error.e2ee_unsupported_description")}

- - - ); + throw new E2EENotSupportedError(); } const shareModal = ( @@ -443,9 +380,9 @@ export const GroupCallView: FC = ({ let body: ReactNode; if (enterRTCError) { // If an ElementCallError was recorded, then create a component that will fail to render and throw - // an ElementCallRichError error. This will then be handled by the ErrorBoundary component. + // the error. This will then be handled by the ErrorBoundary component. const ErrorComponent = (): ReactNode => { - throw new ElementCallRichError(enterRTCError); + throw enterRTCError; }; body = ; } else if (isJoined) { @@ -504,5 +441,23 @@ export const GroupCallView: FC = ({ body = lobbyView; } - return {body}; + return ( + { + if (action == "reconnect") { + setLeft(false); + enterRTCSessionOrError(rtcSession, perParticipantE2EE).catch((e) => { + logger.error("Error re-entering RTC session", e); + }); + } + }} + onError={ + (/**error*/) => { + if (rtcSession.isJoined()) onLeave("error"); + } + } + > + {body} + + ); }; diff --git a/src/room/__snapshots__/GroupCallErrorBoundary.test.tsx.snap b/src/room/__snapshots__/GroupCallErrorBoundary.test.tsx.snap new file mode 100644 index 00000000..f5814114 --- /dev/null +++ b/src/room/__snapshots__/GroupCallErrorBoundary.test.tsx.snap @@ -0,0 +1,1051 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`should have a reconnect button for ConnectionLostError 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + +
+

+ Connection lost +

+

+ You were disconnected from the call. +

+ + +
+
+
+
+ +`; + +exports[`should render the error page 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + + +
+

+ Call is not supported +

+

+ The server is not configured to work with Element Call. Please contact your server admin (Domain: example.com, Error Code: MISSING_MATRIX_RTC_FOCUS). +

+ +
+
+
+
+ +`; + +exports[`should render the error page with link back to home 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + + +
+

+ Call is not supported +

+

+ The server is not configured to work with Element Call. Please contact your server admin (Domain: example.com, Error Code: MISSING_MATRIX_RTC_FOCUS). +

+ +
+
+
+
+ +`; + +exports[`should report correct error for 'Call is not supported' 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + + +
+

+ Call is not supported +

+

+ The server is not configured to work with Element Call. Please contact your server admin (Domain: example.com, Error Code: MISSING_MATRIX_RTC_FOCUS). +

+ +
+
+
+
+ +`; + +exports[`should report correct error for 'Connection lost' 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + +
+

+ Connection lost +

+

+ You were disconnected from the call. +

+ + +
+
+
+
+ +`; + +exports[`should report correct error for 'Incompatible browser' 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + +
+

+ Incompatible browser +

+

+ Your web browser does not support encrypted calls. Supported browsers include Chrome, Safari, and Firefox 117+. +

+ +
+
+
+
+ +`; + +exports[`should report correct error for 'Insufficient capacity' 1`] = ` + +
+
+
+ +
+
+
+
+
+
+
+ + + +
+

+ Insufficient capacity +

+

+ 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. +

+ +
+
+
+
+ +`; diff --git a/src/utils/errors.ts b/src/utils/errors.ts index c87bdee7..91943fb4 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -13,6 +13,9 @@ export enum ErrorCode { */ MISSING_MATRIX_RTC_FOCUS = "MISSING_MATRIX_RTC_FOCUS", CONNECTION_LOST_ERROR = "CONNECTION_LOST_ERROR", + /** LiveKit indicates that the server has hit its track limits */ + INSUFFICIENT_CAPACITY_ERROR = "INSUFFICIENT_CAPACITY_ERROR", + E2EE_NOT_SUPPORTED = "E2EE_NOT_SUPPORTED", UNKNOWN_ERROR = "UNKNOWN_ERROR", } @@ -20,6 +23,7 @@ export enum ErrorCategory { /** Calling is not supported, server misconfigured (JWT service missing, no MSC support ...)*/ CONFIGURATION_ISSUE = "CONFIGURATION_ISSUE", NETWORK_CONNECTIVITY = "NETWORK_CONNECTIVITY", + CLIENT_CONFIGURATION = "CLIENT_CONFIGURATION", UNKNOWN = "UNKNOWN", // SYSTEM_FAILURE / FEDERATION_FAILURE .. } @@ -31,14 +35,17 @@ export class ElementCallError extends Error { public code: ErrorCode; public category: ErrorCategory; public localisedMessage?: string; + public localisedTitle: string; - public constructor( - name: string, + protected constructor( + localisedTitle: string, code: ErrorCode, category: ErrorCategory, - localisedMessage?: string, + localisedMessage: string, + cause?: Error, ) { - super(name); + super(localisedTitle, { cause }); + this.localisedTitle = localisedTitle; this.localisedMessage = localisedMessage; this.category = category; this.code = code; @@ -50,7 +57,7 @@ export class MatrixRTCFocusMissingError extends ElementCallError { public constructor(domain: string) { super( - "MatrixRTCFocusMissingError", + t("error.call_is_not_supported"), ErrorCode.MISSING_MATRIX_RTC_FOCUS, ErrorCategory.CONFIGURATION_ISSUE, t("error.matrix_rtc_focus_missing", { @@ -66,9 +73,45 @@ export class MatrixRTCFocusMissingError extends ElementCallError { export class ConnectionLostError extends ElementCallError { public constructor() { super( - "Connection lost", + t("error.connection_lost"), ErrorCode.CONNECTION_LOST_ERROR, ErrorCategory.NETWORK_CONNECTIVITY, + t("error.connection_lost_description"), + ); + } +} + +export class E2EENotSupportedError extends ElementCallError { + public constructor() { + super( + t("error.e2ee_unsupported"), + ErrorCode.E2EE_NOT_SUPPORTED, + ErrorCategory.CLIENT_CONFIGURATION, + t("error.e2ee_unsupported_description"), + ); + } +} + +export class UnknownCallError extends ElementCallError { + public constructor(error: Error) { + super( + t("error.generic"), + ErrorCode.UNKNOWN_ERROR, + ErrorCategory.UNKNOWN, + error.message, + // Properly set it as a cause for a better reporting on sentry + error, + ); + } +} + +export class InsufficientCapacityError extends ElementCallError { + public constructor() { + super( + t("error.insufficient_capacity"), + ErrorCode.INSUFFICIENT_CAPACITY_ERROR, + ErrorCategory.UNKNOWN, + t("error.insufficient_capacity_description"), ); } } From 13a19ed7511732d6a22f97cc8c6217839af14923 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 7 Mar 2025 15:18:32 +0100 Subject: [PATCH 02/11] fix: Error recover/retry buttons should reset error state --- src/room/GroupCallErrorBoundary.test.tsx | 44 +++++++++++++++++++++++- src/room/GroupCallErrorBoundary.tsx | 5 ++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/room/GroupCallErrorBoundary.test.tsx b/src/room/GroupCallErrorBoundary.test.tsx index 94c96794..893389a7 100644 --- a/src/room/GroupCallErrorBoundary.test.tsx +++ b/src/room/GroupCallErrorBoundary.test.tsx @@ -7,7 +7,13 @@ Please see LICENSE in the repository root for full details. import { describe, expect, test, vi } from "vitest"; import { render, screen } from "@testing-library/react"; -import { type ReactElement, type ReactNode } from "react"; +import { + type FC, + type ReactElement, + type ReactNode, + useCallback, + useState, +} from "react"; import { BrowserRouter } from "react-router-dom"; import userEvent from "@testing-library/user-event"; @@ -131,6 +137,42 @@ test("should have a reconnect button for ConnectionLostError", async () => { expect(reconnectCallback).toHaveBeenCalledWith("reconnect"); }); +test("Action handling should reset error state", async () => { + const user = userEvent.setup(); + + const TestComponent: FC<{ fail: boolean }> = ({ fail }): ReactNode => { + if (fail) { + throw new ConnectionLostError(); + } + return
HELLO
; + }; + + const WrapComponent = (): ReactNode => { + const [failState, setFailState] = useState(true); + const reconnectCallback = useCallback(() => { + setFailState(false); + }, [setFailState]); + + return ( + + + + + + ); + }; + + render(); + + // Should fail first + await screen.findByText("Connection lost"); + + await user.click(screen.getByRole("button", { name: "Reconnect" })); + + // reconnect should have reset the error, thus rendering should be ok + await screen.findByText("HELLO"); +}); + describe("Rageshake button", () => { function setupTest(testError: ElementCallError): void { mockConfig({ diff --git a/src/room/GroupCallErrorBoundary.tsx b/src/room/GroupCallErrorBoundary.tsx index 758016b2..67b95733 100644 --- a/src/room/GroupCallErrorBoundary.tsx +++ b/src/room/GroupCallErrorBoundary.tsx @@ -120,7 +120,10 @@ export const GroupCallErrorBoundary = ({ { + resetError(); + recoveryActionHandler?.(action); + }} /> ); }, From c22412c04559a5c26c6c0e4c2441dae31278e85a Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 10 Mar 2025 15:20:51 +0100 Subject: [PATCH 03/11] error management: showError API for async error handling --- src/room/GroupCallErrorBoundary.test.tsx | 31 ++++++++- src/room/GroupCallErrorBoundary.tsx | 24 +++++++ src/room/GroupCallErrorBoundaryContext.tsx | 18 +++++ ...pCallErrorBoundaryContextProvider.test.tsx | 54 +++++++++++++++ .../GroupCallErrorBoundaryContextProvider.tsx | 54 +++++++++++++++ src/room/GroupCallView.tsx | 66 ++++++++++--------- src/room/useCallErrorBoundary.ts | 58 ++++++++++++++++ 7 files changed, 274 insertions(+), 31 deletions(-) create mode 100644 src/room/GroupCallErrorBoundaryContext.tsx create mode 100644 src/room/GroupCallErrorBoundaryContextProvider.test.tsx create mode 100644 src/room/GroupCallErrorBoundaryContextProvider.tsx create mode 100644 src/room/useCallErrorBoundary.ts diff --git a/src/room/GroupCallErrorBoundary.test.tsx b/src/room/GroupCallErrorBoundary.test.tsx index 893389a7..f99f01fa 100644 --- a/src/room/GroupCallErrorBoundary.test.tsx +++ b/src/room/GroupCallErrorBoundary.test.tsx @@ -8,10 +8,10 @@ Please see LICENSE in the repository root for full details. import { describe, expect, test, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { - type FC, type ReactElement, type ReactNode, useCallback, + useEffect, useState, } from "react"; import { BrowserRouter } from "react-router-dom"; @@ -27,6 +27,8 @@ import { UnknownCallError, } from "../utils/errors.ts"; import { mockConfig } from "../utils/test.ts"; +import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; +import { GroupCallErrorBoundaryContextProvider } from "./GroupCallErrorBoundaryContextProvider.tsx"; test.each([ { @@ -210,3 +212,30 @@ describe("Rageshake button", () => { ).not.toBeInTheDocument(); }); }); + +test("should show async error with useElementCallErrorContext", async () => { + // const error = new MatrixRTCFocusMissingError("example.com"); + const TestComponent = (): ReactNode => { + const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary(); + useEffect(() => { + setTimeout(() => { + showGroupCallErrorBoundary(new ConnectionLostError()); + }); + }, [showGroupCallErrorBoundary]); + + return
Hello
; + }; + + const onErrorMock = vi.fn(); + render( + + + + + + + , + ); + + await screen.findByText("Connection lost"); +}); diff --git a/src/room/GroupCallErrorBoundary.tsx b/src/room/GroupCallErrorBoundary.tsx index 67b95733..a85bee9d 100644 --- a/src/room/GroupCallErrorBoundary.tsx +++ b/src/room/GroupCallErrorBoundary.tsx @@ -105,6 +105,30 @@ interface BoundaryProps { onError?: (error: unknown) => void; } +/** + * An ErrorBoundary component that handles ElementCalls errors that can occur during a group call. + * It is based on the sentry ErrorBoundary component, that will log the error to sentry. + * + * The error fallback will show an error page with: + * - a description of the error + * - a button to go back the home screen + * - optional call-to-action buttons (ex: reconnect for connection lost) + * - A rageshake button for unknown errors + * + * For async errors the `useCallErrorBoundary` hook should be used to show the error page + * ``` + * const { showGroupCallErrorBoundary } = useCallErrorBoundary(); + * ... some async code + * catch(error) { + * showGroupCallErrorBoundary(error); + * } + * ... + * ``` + * @param recoveryActionHandler + * @param onError + * @param children + * @constructor + */ export const GroupCallErrorBoundary = ({ recoveryActionHandler, onError, diff --git a/src/room/GroupCallErrorBoundaryContext.tsx b/src/room/GroupCallErrorBoundaryContext.tsx new file mode 100644 index 00000000..f1dcf461 --- /dev/null +++ b/src/room/GroupCallErrorBoundaryContext.tsx @@ -0,0 +1,18 @@ +/* +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 { createContext } from "react"; + +import { type ElementCallError } from "../utils/errors.ts"; + +export type GroupCallErrorBoundaryContextType = { + subscribe: (cb: (error: ElementCallError) => void) => () => void; + notifyHandled: (error: ElementCallError) => void; +}; + +export const GroupCallErrorBoundaryContext = + createContext(null); diff --git a/src/room/GroupCallErrorBoundaryContextProvider.test.tsx b/src/room/GroupCallErrorBoundaryContextProvider.test.tsx new file mode 100644 index 00000000..128e6ae8 --- /dev/null +++ b/src/room/GroupCallErrorBoundaryContextProvider.test.tsx @@ -0,0 +1,54 @@ +/* +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 { it } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { type ReactElement, useCallback } from "react"; +import userEvent from "@testing-library/user-event"; +import { BrowserRouter } from "react-router-dom"; + +import { GroupCallErrorBoundaryContextProvider } from "./GroupCallErrorBoundaryContextProvider.tsx"; +import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; +import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; +import { ConnectionLostError } from "../utils/errors.ts"; + +it("should show async error", async () => { + const user = userEvent.setup(); + + const TestComponent = (): ReactElement => { + const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary(); + + const onClick = useCallback((): void => { + showGroupCallErrorBoundary(new ConnectionLostError()); + }, [showGroupCallErrorBoundary]); + + return ( +
+

HELLO

+ +
+ ); + }; + + render( + + + + + + + , + ); + + await user.click(screen.getByRole("button", { name: "Click me" })); + + await screen.findByText("Connection lost"); + + await user.click(screen.getByRole("button", { name: "Reconnect" })); + + await screen.findByText("HELLO"); +}); diff --git a/src/room/GroupCallErrorBoundaryContextProvider.tsx b/src/room/GroupCallErrorBoundaryContextProvider.tsx new file mode 100644 index 00000000..b7292624 --- /dev/null +++ b/src/room/GroupCallErrorBoundaryContextProvider.tsx @@ -0,0 +1,54 @@ +/* +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, + type PropsWithChildren, + useCallback, + useMemo, + useRef, +} from "react"; + +import type { ElementCallError } from "../utils/errors.ts"; +import { + GroupCallErrorBoundaryContext, + type GroupCallErrorBoundaryContextType, +} from "./GroupCallErrorBoundaryContext.tsx"; + +export const GroupCallErrorBoundaryContextProvider: FC = ({ + children, +}) => { + const subscribers = useRef void>>(new Set()); + + // Register a component for updates + const subscribe = useCallback( + (cb: (error: ElementCallError) => void): (() => void) => { + subscribers.current.add(cb); + return (): boolean => subscribers.current.delete(cb); // Unsubscribe function + }, + [], + ); + + // Notify all subscribers + const notify = useCallback((error: ElementCallError) => { + subscribers.current.forEach((callback) => callback(error)); + }, []); + + const context: GroupCallErrorBoundaryContextType = useMemo( + () => ({ + notifyHandled: notify, + subscribe, + }), + [subscribe, notify], + ); + + return ( + + {children} + + ); +}; diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 226fb9f5..3adffba5 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -57,6 +57,8 @@ import { UnknownCallError, } from "../utils/errors.ts"; import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; +import { GroupCallErrorBoundaryContextProvider } from "./GroupCallErrorBoundaryContextProvider.tsx"; +import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; declare global { interface Window { @@ -77,7 +79,15 @@ interface Props { widget: WidgetHelpers | null; } -export const GroupCallView: FC = ({ +export const GroupCallView: FC = (props) => { + return ( + + + + ); +}; + +export const GroupCallViewInner: FC = ({ client, isPasswordlessUser, confineToRoom, @@ -156,25 +166,29 @@ export const GroupCallView: FC = ({ const latestDevices = useLatest(deviceContext); const latestMuteStates = useLatest(muteStates); - const enterRTCSessionOrError = async ( - rtcSession: MatrixRTCSession, - perParticipantE2EE: boolean, - ): Promise => { - try { - await enterRTCSession(rtcSession, perParticipantE2EE); - } catch (e) { - if (e instanceof ElementCallError) { - // e.code === ErrorCode.MISSING_LIVE_KIT_SERVICE_URL) - setEnterRTCError(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 }), - ); - setEnterRTCError(error); + const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary(); + + const enterRTCSessionOrError = useCallback( + async ( + rtcSession: MatrixRTCSession, + perParticipantE2EE: boolean, + ): Promise => { + try { + await enterRTCSession(rtcSession, perParticipantE2EE); + } catch (e) { + if (e instanceof ElementCallError) { + showGroupCallErrorBoundary(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); + } } - } - }; + }, + [showGroupCallErrorBoundary], + ); useEffect(() => { const defaultDeviceSetup = async ({ @@ -255,12 +269,11 @@ export const GroupCallView: FC = ({ perParticipantE2EE, latestDevices, latestMuteStates, + enterRTCSessionOrError, ]); const [left, setLeft] = useState(false); - const [enterRTCError, setEnterRTCError] = useState( - null, - ); + const navigate = useNavigate(); const onLeave = useCallback( @@ -378,14 +391,7 @@ export const GroupCallView: FC = ({ ); let body: ReactNode; - if (enterRTCError) { - // If an ElementCallError was recorded, then create a component that will fail to render and throw - // the error. This will then be handled by the ErrorBoundary component. - const ErrorComponent = (): ReactNode => { - throw enterRTCError; - }; - body = ; - } else if (isJoined) { + if (isJoined) { body = ( <> {shareModal} diff --git a/src/room/useCallErrorBoundary.ts b/src/room/useCallErrorBoundary.ts new file mode 100644 index 00000000..b8b0a034 --- /dev/null +++ b/src/room/useCallErrorBoundary.ts @@ -0,0 +1,58 @@ +/* +Copyright 2023, 2024 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 { useCallback, useContext, useEffect, useMemo, useState } from "react"; + +import type { ElementCallError } from "../utils/errors.ts"; +import { GroupCallErrorBoundaryContext } from "./GroupCallErrorBoundaryContext.tsx"; + +export type UseErrorBoundaryApi = { + showGroupCallErrorBoundary: (error: ElementCallError) => void; +}; + +export function useGroupCallErrorBoundary(): UseErrorBoundaryApi { + const context = useContext(GroupCallErrorBoundaryContext); + + if (!context) + throw new Error( + "useGroupCallErrorBoundary must be used within an GoupCallErrorBoundary", + ); + + const [error, setError] = useState(null); + + const resetErrorIfNeeded = useCallback( + (handled: ElementCallError): void => { + // There might be several useGroupCallErrorBoundary in the tree, + // so only clear our state if it's the one we're handling? + if (error && handled === error) { + // reset current state + setError(null); + } + }, + [error], + ); + + useEffect(() => { + // return a function to unsubscribe + return context.subscribe((error: ElementCallError): void => { + resetErrorIfNeeded(error); + }); + }, [resetErrorIfNeeded, context]); + + const memoized: UseErrorBoundaryApi = useMemo( + () => ({ + showGroupCallErrorBoundary: (error: ElementCallError) => setError(error), + }), + [], + ); + + if (error) { + throw error; + } + + return memoized; +} From 343da0db145a416a1115a7e7452dbcbb4e4372dd Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 10 Mar 2025 17:54:44 +0100 Subject: [PATCH 04/11] network: Utility to retry network operation with backoff --- src/utils/matrix.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/utils/matrix.ts b/src/utils/matrix.ts index c80fa7d9..8c1a47e2 100644 --- a/src/utils/matrix.ts +++ b/src/utils/matrix.ts @@ -8,6 +8,7 @@ Please see LICENSE in the repository root for full details. import { IndexedDBStore } from "matrix-js-sdk/src/store/indexeddb"; import { MemoryStore } from "matrix-js-sdk/src/store/memory"; import { + calculateRetryBackoff, createClient, type ICreateClientOpts, Preset, @@ -17,6 +18,7 @@ import { ClientEvent } from "matrix-js-sdk/src/client"; import { type ISyncStateData, type SyncState } from "matrix-js-sdk/src/sync"; import { logger } from "matrix-js-sdk/src/logger"; import { secureRandomBase64Url } from "matrix-js-sdk/src/randomstring"; +import { sleep } from "matrix-js-sdk/src/utils"; import type { MatrixClient } from "matrix-js-sdk/src/client"; import type { Room } from "matrix-js-sdk/src/models/room"; @@ -336,3 +338,30 @@ export function getRelativeRoomUrl( : ""; return `/room/#${roomPart}?${generateUrlSearchParams(roomId, encryptionSystem, viaServers).toString()}`; } + +/** + * Perfom a network operation with retries on ConnectionError. + * If the error is not retryable, or the max number of retries is reached, the error is rethrown. + * Supports handling of matrix quotas. + */ +export async function doNetworkOperationWithRetry( + operation: () => Promise, +): Promise { + let currentRetryCount = 0; + + // eslint-disable-next-line no-constant-condition + while (true) { + try { + return await operation(); + } catch (e) { + currentRetryCount++; + const backoff = calculateRetryBackoff(e, currentRetryCount, true); + if (backoff < 0) { + // Max number of retries reached, or error is not retryable. rethrow the error + throw e; + } + // wait for the specified time and then retry the request + await sleep(backoff); + } + } +} From 04a46ebabe66480e4e13526ae23aab98884b402c Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 11 Mar 2025 09:07:19 +0100 Subject: [PATCH 05/11] error management: Handle fail to get JWT token --- src/livekit/openIDSFU.ts | 18 ++++++++++++++++-- src/room/GroupCallErrorBoundary.test.tsx | 1 + src/utils/errors.ts | 16 +++++++++++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/livekit/openIDSFU.ts b/src/livekit/openIDSFU.ts index 0f455a38..4a82de23 100644 --- a/src/livekit/openIDSFU.ts +++ b/src/livekit/openIDSFU.ts @@ -12,6 +12,9 @@ import { useEffect, useState } from "react"; import { type LivekitFocus } from "matrix-js-sdk/src/matrixrtc/LivekitFocus"; import { useActiveLivekitFocus } from "../room/useActiveFocus"; +import { useGroupCallErrorBoundary } from "../room/useCallErrorBoundary.ts"; +import { FailToGetOpenIdToken } from "../utils/errors.ts"; +import { doNetworkOperationWithRetry } from "../utils/matrix.ts"; export interface SFUConfig { url: string; @@ -38,6 +41,7 @@ export function useOpenIDSFU( const [sfuConfig, setSFUConfig] = useState(undefined); const activeFocus = useActiveLivekitFocus(rtcSession); + const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary(); useEffect(() => { if (activeFocus) { @@ -46,13 +50,14 @@ export function useOpenIDSFU( setSFUConfig(sfuConfig); }, (e) => { + showGroupCallErrorBoundary(new FailToGetOpenIdToken(e)); logger.error("Failed to get SFU config", e); }, ); } else { setSFUConfig(undefined); } - }, [client, activeFocus]); + }, [client, activeFocus, showGroupCallErrorBoundary]); return sfuConfig; } @@ -61,7 +66,16 @@ export async function getSFUConfigWithOpenID( client: OpenIDClientParts, activeFocus: LivekitFocus, ): Promise { - const openIdToken = await client.getOpenIdToken(); + let openIdToken: IOpenIDToken; + try { + openIdToken = await doNetworkOperationWithRetry(async () => + client.getOpenIdToken(), + ); + } catch (error) { + throw new FailToGetOpenIdToken( + error instanceof Error ? error : new Error("Unknown error"), + ); + } logger.debug("Got openID token", openIdToken); try { diff --git a/src/room/GroupCallErrorBoundary.test.tsx b/src/room/GroupCallErrorBoundary.test.tsx index f99f01fa..c42e3ded 100644 --- a/src/room/GroupCallErrorBoundary.test.tsx +++ b/src/room/GroupCallErrorBoundary.test.tsx @@ -8,6 +8,7 @@ Please see LICENSE in the repository root for full details. import { describe, expect, test, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { + type FC, type ReactElement, type ReactNode, useCallback, diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 91943fb4..e8adbca1 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -16,6 +16,7 @@ export enum ErrorCode { /** LiveKit indicates that the server has hit its track limits */ INSUFFICIENT_CAPACITY_ERROR = "INSUFFICIENT_CAPACITY_ERROR", E2EE_NOT_SUPPORTED = "E2EE_NOT_SUPPORTED", + OPEN_ID_ERROR = "OPEN_ID_ERROR", UNKNOWN_ERROR = "UNKNOWN_ERROR", } @@ -41,7 +42,7 @@ export class ElementCallError extends Error { localisedTitle: string, code: ErrorCode, category: ErrorCategory, - localisedMessage: string, + localisedMessage?: string, cause?: Error, ) { super(localisedTitle, { cause }); @@ -105,6 +106,19 @@ export class UnknownCallError extends ElementCallError { } } +export class FailToGetOpenIdToken extends ElementCallError { + public constructor(error: Error) { + super( + t("error.generic"), + ErrorCode.OPEN_ID_ERROR, + ErrorCategory.CONFIGURATION_ISSUE, + undefined, + // Properly set it as a cause for a better reporting on sentry + error, + ); + } +} + export class InsufficientCapacityError extends ElementCallError { public constructor() { super( From 03b5f0f2f9705d1c65027d742bc28e18f237e0ad Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 17 Mar 2025 11:26:16 +0100 Subject: [PATCH 06/11] Fixup: error boundary context not needed, local error resets already --- src/room/GroupCallErrorBoundaryContext.tsx | 18 ------- .../GroupCallErrorBoundaryContextProvider.tsx | 54 ------------------- src/room/GroupCallView.tsx | 11 +--- ...test.tsx => useCallErrorBoundary.test.tsx} | 9 ++-- src/room/useCallErrorBoundary.ts | 29 +--------- 5 files changed, 5 insertions(+), 116 deletions(-) delete mode 100644 src/room/GroupCallErrorBoundaryContext.tsx delete mode 100644 src/room/GroupCallErrorBoundaryContextProvider.tsx rename src/room/{GroupCallErrorBoundaryContextProvider.test.tsx => useCallErrorBoundary.test.tsx} (80%) diff --git a/src/room/GroupCallErrorBoundaryContext.tsx b/src/room/GroupCallErrorBoundaryContext.tsx deleted file mode 100644 index f1dcf461..00000000 --- a/src/room/GroupCallErrorBoundaryContext.tsx +++ /dev/null @@ -1,18 +0,0 @@ -/* -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 { createContext } from "react"; - -import { type ElementCallError } from "../utils/errors.ts"; - -export type GroupCallErrorBoundaryContextType = { - subscribe: (cb: (error: ElementCallError) => void) => () => void; - notifyHandled: (error: ElementCallError) => void; -}; - -export const GroupCallErrorBoundaryContext = - createContext(null); diff --git a/src/room/GroupCallErrorBoundaryContextProvider.tsx b/src/room/GroupCallErrorBoundaryContextProvider.tsx deleted file mode 100644 index b7292624..00000000 --- a/src/room/GroupCallErrorBoundaryContextProvider.tsx +++ /dev/null @@ -1,54 +0,0 @@ -/* -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, - type PropsWithChildren, - useCallback, - useMemo, - useRef, -} from "react"; - -import type { ElementCallError } from "../utils/errors.ts"; -import { - GroupCallErrorBoundaryContext, - type GroupCallErrorBoundaryContextType, -} from "./GroupCallErrorBoundaryContext.tsx"; - -export const GroupCallErrorBoundaryContextProvider: FC = ({ - children, -}) => { - const subscribers = useRef void>>(new Set()); - - // Register a component for updates - const subscribe = useCallback( - (cb: (error: ElementCallError) => void): (() => void) => { - subscribers.current.add(cb); - return (): boolean => subscribers.current.delete(cb); // Unsubscribe function - }, - [], - ); - - // Notify all subscribers - const notify = useCallback((error: ElementCallError) => { - subscribers.current.forEach((callback) => callback(error)); - }, []); - - const context: GroupCallErrorBoundaryContextType = useMemo( - () => ({ - notifyHandled: notify, - subscribe, - }), - [subscribe, notify], - ); - - return ( - - {children} - - ); -}; diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index fad91065..49b3173d 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -68,7 +68,6 @@ import { useSetting, } from "../settings/settings"; import { useTypedEventEmitter } from "../useEvents"; -import { GroupCallErrorBoundaryContextProvider } from "./GroupCallErrorBoundaryContextProvider.tsx"; import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; declare global { @@ -90,15 +89,7 @@ interface Props { widget: WidgetHelpers | null; } -export const GroupCallView: FC = (props) => { - return ( - - - - ); -}; - -export const GroupCallViewInner: FC = ({ +export const GroupCallView: FC = ({ client, isPasswordlessUser, confineToRoom, diff --git a/src/room/GroupCallErrorBoundaryContextProvider.test.tsx b/src/room/useCallErrorBoundary.test.tsx similarity index 80% rename from src/room/GroupCallErrorBoundaryContextProvider.test.tsx rename to src/room/useCallErrorBoundary.test.tsx index 116aa7db..eccb8039 100644 --- a/src/room/GroupCallErrorBoundaryContextProvider.test.tsx +++ b/src/room/useCallErrorBoundary.test.tsx @@ -11,7 +11,6 @@ import { type ReactElement, useCallback } from "react"; import userEvent from "@testing-library/user-event"; import { BrowserRouter } from "react-router-dom"; -import { GroupCallErrorBoundaryContextProvider } from "./GroupCallErrorBoundaryContextProvider.tsx"; import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts"; import { ConnectionLostError } from "../utils/errors.ts"; @@ -36,11 +35,9 @@ it("should show async error", async () => { render( - - - - - + + + , ); diff --git a/src/room/useCallErrorBoundary.ts b/src/room/useCallErrorBoundary.ts index b8b0a034..f89abf77 100644 --- a/src/room/useCallErrorBoundary.ts +++ b/src/room/useCallErrorBoundary.ts @@ -5,44 +5,17 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { useCallback, useContext, useEffect, useMemo, useState } from "react"; +import { useMemo, useState } from "react"; import type { ElementCallError } from "../utils/errors.ts"; -import { GroupCallErrorBoundaryContext } from "./GroupCallErrorBoundaryContext.tsx"; export type UseErrorBoundaryApi = { showGroupCallErrorBoundary: (error: ElementCallError) => void; }; export function useGroupCallErrorBoundary(): UseErrorBoundaryApi { - const context = useContext(GroupCallErrorBoundaryContext); - - if (!context) - throw new Error( - "useGroupCallErrorBoundary must be used within an GoupCallErrorBoundary", - ); - const [error, setError] = useState(null); - const resetErrorIfNeeded = useCallback( - (handled: ElementCallError): void => { - // There might be several useGroupCallErrorBoundary in the tree, - // so only clear our state if it's the one we're handling? - if (error && handled === error) { - // reset current state - setError(null); - } - }, - [error], - ); - - useEffect(() => { - // return a function to unsubscribe - return context.subscribe((error: ElementCallError): void => { - resetErrorIfNeeded(error); - }); - }, [resetErrorIfNeeded, context]); - const memoized: UseErrorBoundaryApi = useMemo( () => ({ showGroupCallErrorBoundary: (error: ElementCallError) => setError(error), From 598152a3d7b36d4550a2d3610e2484a9933b720d Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 17 Mar 2025 12:15:16 +0100 Subject: [PATCH 07/11] Add playwright test for JWT token error --- playwright/errors.spec.ts | 58 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 playwright/errors.spec.ts diff --git a/playwright/errors.spec.ts b/playwright/errors.spec.ts new file mode 100644 index 00000000..e65ca740 --- /dev/null +++ b/playwright/errors.spec.ts @@ -0,0 +1,58 @@ +import { expect, test } from "@playwright/test"; + +test("Should show error screen if fails to get JWT token", async ({ page }) => { + await page.goto("/"); + + await page.getByTestId("home_callName").click(); + await page.getByTestId("home_callName").fill("HelloCall"); + await page.getByTestId("home_displayName").click(); + await page.getByTestId("home_displayName").fill("John Doe"); + await page.getByTestId("home_go").click(); + + await page.route("**/openid/request_token", (route) => + route.fulfill({ + // 418 is a non retryable error, so test will fail immediately + status: 418, + }), + ); + + // Join the call + await page.getByTestId("lobby_joinCall").click(); + + // Should fail + await expect(page.getByText("Something went wrong")).toBeVisible(); + await expect(page.getByText("OPEN_ID_ERROR")).toBeVisible(); +}); + +test("Should automatically retry non fatal JWT errors", async ({ page }) => { + await page.goto("/"); + + await page.getByTestId("home_callName").click(); + await page.getByTestId("home_callName").fill("HelloCall"); + await page.getByTestId("home_displayName").click(); + await page.getByTestId("home_displayName").fill("John Doe"); + await page.getByTestId("home_go").click(); + + let firstCall = true; + let hasRetriedCallback: (value: PromiseLike | void) => void; + let hasRetriedPromise = new Promise((resolve) => { + hasRetriedCallback = resolve; + }); + await page.route("**/openid/request_token", (route) => { + if (firstCall) { + firstCall = false; + route.fulfill({ + status: 429, + }); + } else { + route.continue(); + hasRetriedCallback(); + } + }); + + // Join the call + await page.getByTestId("lobby_joinCall").click(); + // Expect that the call has been retried + await hasRetriedPromise; + await expect(page.getByText("Something went wrong")).not.toBeVisible(); +}); From 007ea89cd79bd9939b2d24e4f53abe7e8610e02f Mon Sep 17 00:00:00 2001 From: Valere Date: Mon, 17 Mar 2025 12:36:14 +0100 Subject: [PATCH 08/11] fixup eslint --- playwright/errors.spec.ts | 7 +++++++ src/room/GroupCallView.tsx | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/playwright/errors.spec.ts b/playwright/errors.spec.ts index e65ca740..9e25d950 100644 --- a/playwright/errors.spec.ts +++ b/playwright/errors.spec.ts @@ -1,3 +1,10 @@ +/* +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("Should show error screen if fails to get JWT token", async ({ page }) => { diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 49b3173d..1b3d0f20 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -19,7 +19,6 @@ import { isE2EESupported as isE2EESupportedBrowser, } from "livekit-client"; import { logger } from "matrix-js-sdk/src/logger"; - import { MatrixRTCSessionEvent, type MatrixRTCSession, From 8c5f5b156c23620880798e87ae6efbfdb1d82e7c Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 18 Mar 2025 09:48:18 +0100 Subject: [PATCH 09/11] code review: improve test --- playwright/errors.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/errors.spec.ts b/playwright/errors.spec.ts index 9e25d950..9f3ad327 100644 --- a/playwright/errors.spec.ts +++ b/playwright/errors.spec.ts @@ -61,5 +61,5 @@ test("Should automatically retry non fatal JWT errors", async ({ page }) => { await page.getByTestId("lobby_joinCall").click(); // Expect that the call has been retried await hasRetriedPromise; - await expect(page.getByText("Something went wrong")).not.toBeVisible(); + await expect(page.getByTestId("video").first()).toBeVisible(); }); From 96ce6a2dc65b5af41cfb125f3997774936125460 Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 19 Mar 2025 09:22:07 +0100 Subject: [PATCH 10/11] ui test: Skip video visibility test on firefox --- playwright/errors.spec.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/playwright/errors.spec.ts b/playwright/errors.spec.ts index 9f3ad327..efceaf81 100644 --- a/playwright/errors.spec.ts +++ b/playwright/errors.spec.ts @@ -31,7 +31,14 @@ test("Should show error screen if fails to get JWT token", async ({ page }) => { await expect(page.getByText("OPEN_ID_ERROR")).toBeVisible(); }); -test("Should automatically retry non fatal JWT errors", async ({ page }) => { +test("Should automatically retry non fatal JWT errors", async ({ + page, + browserName, +}) => { + test.skip( + browserName === "firefox", + "The test to check the video visibility is not working in Firefox CI environment. looks like video is disabled?", + ); await page.goto("/"); await page.getByTestId("home_callName").click(); From b6ad6aee2aae37b0dafced6ea79d59acacb4029a Mon Sep 17 00:00:00 2001 From: Valere Date: Wed, 19 Mar 2025 09:33:06 +0100 Subject: [PATCH 11/11] post merge fix: es lint --- playwright/errors.spec.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/playwright/errors.spec.ts b/playwright/errors.spec.ts index efceaf81..7671c103 100644 --- a/playwright/errors.spec.ts +++ b/playwright/errors.spec.ts @@ -16,11 +16,13 @@ test("Should show error screen if fails to get JWT token", async ({ page }) => { await page.getByTestId("home_displayName").fill("John Doe"); await page.getByTestId("home_go").click(); - await page.route("**/openid/request_token", (route) => - route.fulfill({ - // 418 is a non retryable error, so test will fail immediately - status: 418, - }), + await page.route( + "**/openid/request_token", + async (route) => + await route.fulfill({ + // 418 is a non retryable error, so test will fail immediately + status: 418, + }), ); // Join the call @@ -49,17 +51,17 @@ test("Should automatically retry non fatal JWT errors", async ({ let firstCall = true; let hasRetriedCallback: (value: PromiseLike | void) => void; - let hasRetriedPromise = new Promise((resolve) => { + const hasRetriedPromise = new Promise((resolve) => { hasRetriedCallback = resolve; }); - await page.route("**/openid/request_token", (route) => { + await page.route("**/openid/request_token", async (route) => { if (firstCall) { firstCall = false; - route.fulfill({ + await route.fulfill({ status: 429, }); } else { - route.continue(); + await route.continue(); hasRetriedCallback(); } });