Merge pull request #3118 from element-hq/robin/errors-outside-boundary

Fix join errors mistakenly showing a generic error screen

src/livekit/openIDSFU.ts

@@ -12,9 +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 { useErrorBoundary } from "../useErrorBoundary";
-import { FailToGetOpenIdToken } from "../utils/errors.ts";
+import { FailToGetOpenIdToken } from "../utils/errors";
-import { doNetworkOperationWithRetry } from "../utils/matrix.ts";
+import { doNetworkOperationWithRetry } from "../utils/matrix";
 
 export interface SFUConfig {
   url: string;
@@ -41,7 +41,7 @@ export function useOpenIDSFU(
   const [sfuConfig, setSFUConfig] = useState<SFUConfig | undefined>(undefined);
 
   const activeFocus = useActiveLivekitFocus(rtcSession);
-  const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary();
+  const { showErrorBoundary } = useErrorBoundary();
 
   useEffect(() => {
     if (activeFocus) {
@@ -50,14 +50,14 @@ export function useOpenIDSFU(
           setSFUConfig(sfuConfig);
         },
         (e) => {
-          showGroupCallErrorBoundary(new FailToGetOpenIdToken(e));
+          showErrorBoundary(new FailToGetOpenIdToken(e));
           logger.error("Failed to get SFU config", e);
         },
       );
     } else {
       setSFUConfig(undefined);
     }
-  }, [client, activeFocus, showGroupCallErrorBoundary]);
+  }, [client, activeFocus, showErrorBoundary]);
 
   return sfuConfig;
 }

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");
+vi.mock("../soundUtils");
-vitest.mock("../useAudioContext");
+vi.mock("../useAudioContext");
-vitest.mock("./InCallView");
+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<typeof import("../rtcSessionHelpers")>();
-  vitest.spyOn(orig, "leaveRTCSession");
+  return { ...orig, enterRTCSession, leaveRTCSession };
-  return orig;
 });
 
 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<typeof prefetchSounds>).mockResolvedValue({
     sound: new ArrayBuffer(0),
   });
-  playSound = vitest.fn();
+  playSound = vi.fn();
   (useAudioContext as MockedFunction<typeof useAudioContext>).mockReturnValue({
     playSound,
   });
@@ -75,7 +96,10 @@ beforeEach(() => {
   );
 });
 
-function createGroupCallView(widget: WidgetHelpers | null): {
+function createGroupCallView(
+  widget: WidgetHelpers | null,
+  joined = true,
+): {
   rtcSession: MockRTCSession;
   getByText: ReturnType<typeof render>["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(
     <BrowserRouter>
-      <GroupCallView
+      <TooltipProvider>
-        client={client}
+        <GroupCallView
-        isPasswordlessUser={false}
+          client={client}
-        confineToRoom={false}
+          isPasswordlessUser={false}
-        preload={false}
+          confineToRoom={false}
-        skipLobby={false}
+          preload={false}
-        hideHeader={true}
+          skipLobby={false}
-        rtcSession={rtcSession as unknown as MatrixRTCSession}
+          hideHeader={true}
-        isJoined
+          rtcSession={rtcSession as unknown as MatrixRTCSession}
-        muteStates={muteState}
+          isJoined={joined}
-        widget={widget}
+          muteStates={muteState}
-      />
+          widget={widget}
+        />
+      </TooltipProvider>
     </BrowserRouter>,
   );
   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<void>((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
+test("GroupCallView shows errors that occur during joining", async () => {
-  await waitFor(() => expect(leaveRTCSession).toHaveResolved());
+  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");
 });

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<Props> = ({
   muteStates,
   widget,
 }) => {
+  // Used to thread through any errors that occur outside the error boundary
+  const [externalError, setExternalError] = useState<ElementCallError | null>(
+    null,
+  );
+
   const memberships = useMatrixRTCSessionMemberships(rtcSession);
   const leaveSoundContext = useLatest(
     useAudioContext({
@@ -121,13 +125,11 @@ export const GroupCallView: FC<Props> = ({
     };
   }, [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<Props> = ({
         );
       } 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<Props> = ({
   );
 
   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 = <ErrorComponent />;
+  } else if (isJoined) {
     body = (
       <>
         {shareModal}

src/useErrorBoundary.test.tsx

@@ -11,19 +11,19 @@ import { type ReactElement, useCallback } from "react";
 import userEvent from "@testing-library/user-event";
 import { BrowserRouter } from "react-router-dom";
 
-import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx";
+import { GroupCallErrorBoundary } from "./room/GroupCallErrorBoundary";
-import { useGroupCallErrorBoundary } from "./useCallErrorBoundary.ts";
+import { useErrorBoundary } from "./useErrorBoundary";
-import { ConnectionLostError } from "../utils/errors.ts";
+import { ConnectionLostError } from "./utils/errors";
 
 it("should show async error", async () => {
   const user = userEvent.setup();
 
   const TestComponent = (): ReactElement => {
-    const { showGroupCallErrorBoundary } = useGroupCallErrorBoundary();
+    const { showErrorBoundary } = useErrorBoundary();
 
     const onClick = useCallback((): void => {
-      showGroupCallErrorBoundary(new ConnectionLostError());
+      showErrorBoundary(new ConnectionLostError());
-    }, [showGroupCallErrorBoundary]);
+    }, [showErrorBoundary]);
 
     return (
       <div>

src/useErrorBoundary.ts

@@ -7,18 +7,16 @@ Please see LICENSE in the repository root for full details.
 
 import { useMemo, useState } from "react";
 
-import type { ElementCallError } from "../utils/errors.ts";
-
 export type UseErrorBoundaryApi = {
-  showGroupCallErrorBoundary: (error: ElementCallError) => void;
+  showErrorBoundary: (error: Error) => void;
 };
 
-export function useGroupCallErrorBoundary(): UseErrorBoundaryApi {
+export function useErrorBoundary(): UseErrorBoundaryApi {
-  const [error, setError] = useState<ElementCallError | null>(null);
+  const [error, setError] = useState<Error | null>(null);
 
   const memoized: UseErrorBoundaryApi = useMemo(
     () => ({
-      showGroupCallErrorBoundary: (error: ElementCallError) => setError(error),
+      showErrorBoundary: (error: Error) => setError(error),
     }),
     [],
   );

src/utils/test.ts

@@ -286,8 +286,9 @@ export class MockRTCSession extends TypedEventEmitter<
     super();
   }
 
-  public isJoined(): true {
+  public joined = true;
-    return true;
+  public isJoined(): boolean {
+    return this.joined;
   }
 
   public withMemberships(