Merge pull request #3595 from element-hq/toger5/dont-trap-in-invalid-config

Reset overwrite url if it is invalid (does fail to reach sfu)

src/settings/DeveloperSettingsTab.test.tsx

@@ -5,13 +5,16 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { describe, expect, it, vi } from "vitest";
+import { afterEach, describe, expect, it, type Mock, vi } from "vitest";
-import { render, waitFor } from "@testing-library/react";
+import { render, waitFor, screen } from "@testing-library/react";
-import { type Room as LivekitRoom } from "livekit-client";
+import userEvent from "@testing-library/user-event";
+import { TooltipProvider } from "@vector-im/compound-web";
 
 import type { MatrixClient } from "matrix-js-sdk";
+import type { Room as LivekitRoom } from "livekit-client";
 import { DeveloperSettingsTab } from "./DeveloperSettingsTab";
-
+import { getSFUConfigWithOpenID } from "../livekit/openIDSFU";
+import { customLivekitUrl as customLivekitUrlSetting } from "./settings";
 // Mock url params hook to avoid environment-dependent snapshot churn.
 vi.mock("../UrlParams", () => ({
   useUrlParams: (): { mocked: boolean; answer: number } => ({
@@ -20,6 +23,14 @@ vi.mock("../UrlParams", () => ({
   }),
 }));
 
+// IMPORTANT: mock the same specifier used by DeveloperSettingsTab
+vi.mock("../livekit/openIDSFU", () => ({
+  getSFUConfigWithOpenID: vi.fn().mockResolvedValue({
+    url: "mock-url",
+    jwt: "mock-jwt",
+  }),
+}));
+
 // Provide a minimal mock of a Livekit Room structure used by the component.
 function createMockLivekitRoom(
   wsUrl: string,
@@ -86,6 +97,7 @@ describe("DeveloperSettingsTab", () => {
     const { container } = render(
       <DeveloperSettingsTab
         client={client}
+        roomId={"#room:example.org"}
         livekitRooms={livekitRooms}
         env={{ MY_MOCK_ENV: 10, ENV: "test" } as unknown as ImportMetaEnv}
       />,
@@ -99,4 +111,141 @@ describe("DeveloperSettingsTab", () => {
 
     expect(container).toMatchSnapshot();
   });
+  describe("custom livekit url", () => {
+    afterEach(() => {
+      customLivekitUrlSetting.setValue(null);
+    });
+    const client = {
+      doesServerSupportUnstableFeature: vi.fn().mockResolvedValue(true),
+      getCrypto: () => ({ getVersion: (): string => "x" }),
+      getUserId: () => "@u:hs",
+      getDeviceId: () => "DEVICE",
+    } as unknown as MatrixClient;
+    it("will not update custom livekit url without roomId", async () => {
+      const user = userEvent.setup();
+
+      render(
+        <TooltipProvider>
+          <DeveloperSettingsTab
+            client={client}
+            env={{} as unknown as ImportMetaEnv}
+          />
+        </TooltipProvider>,
+      );
+
+      const input = screen.getByLabelText("Custom Livekit-url");
+      await user.clear(input);
+      await user.type(input, "wss://example.livekit.invalid");
+
+      const saveButton = screen.getByRole("button", { name: "Save" });
+      await user.click(saveButton);
+      expect(getSFUConfigWithOpenID).not.toHaveBeenCalled();
+
+      expect(customLivekitUrlSetting.getValue()).toBe(null);
+    });
+    it("will not update custom livekit url without text in input", async () => {
+      const user = userEvent.setup();
+
+      render(
+        <TooltipProvider>
+          <DeveloperSettingsTab
+            client={client}
+            roomId="#testRoom"
+            env={{} as unknown as ImportMetaEnv}
+          />
+        </TooltipProvider>,
+      );
+
+      const input = screen.getByLabelText("Custom Livekit-url");
+      await user.clear(input);
+
+      const saveButton = screen.getByRole("button", { name: "Save" });
+      await user.click(saveButton);
+      expect(getSFUConfigWithOpenID).not.toHaveBeenCalled();
+
+      expect(customLivekitUrlSetting.getValue()).toBe(null);
+    });
+    it("will not update custom livekit url when pressing cancel", async () => {
+      const user = userEvent.setup();
+
+      render(
+        <TooltipProvider>
+          <DeveloperSettingsTab
+            client={client}
+            roomId="#testRoom"
+            env={{} as unknown as ImportMetaEnv}
+          />
+        </TooltipProvider>,
+      );
+
+      const input = screen.getByLabelText("Custom Livekit-url");
+      await user.clear(input);
+      await user.type(input, "wss://example.livekit.invalid");
+
+      const cancelButton = screen.getByRole("button", {
+        name: "Reset overwrite",
+      });
+      await user.click(cancelButton);
+      expect(getSFUConfigWithOpenID).not.toHaveBeenCalled();
+
+      expect(customLivekitUrlSetting.getValue()).toBe(null);
+    });
+    it("will update custom livekit url", async () => {
+      const user = userEvent.setup();
+
+      render(
+        <TooltipProvider>
+          <DeveloperSettingsTab
+            client={client}
+            roomId="#testRoom"
+            env={{} as unknown as ImportMetaEnv}
+          />
+        </TooltipProvider>,
+      );
+
+      const input = screen.getByLabelText("Custom Livekit-url");
+      await user.clear(input);
+      await user.type(input, "wss://example.livekit.valid");
+
+      const saveButton = screen.getByRole("button", { name: "Save" });
+      await user.click(saveButton);
+      expect(getSFUConfigWithOpenID).toHaveBeenCalledWith(
+        expect.anything(),
+        expect.anything(),
+        "wss://example.livekit.valid",
+        "#testRoom",
+      );
+
+      expect(customLivekitUrlSetting.getValue()).toBe(
+        "wss://example.livekit.valid",
+      );
+    });
+    it("will show error on invalid url", async () => {
+      const user = userEvent.setup();
+
+      render(
+        <TooltipProvider>
+          <DeveloperSettingsTab
+            client={client}
+            roomId="#testRoom"
+            env={{} as unknown as ImportMetaEnv}
+          />
+        </TooltipProvider>,
+      );
+
+      const input = screen.getByLabelText("Custom Livekit-url");
+      await user.clear(input);
+      await user.type(input, "wss://example.livekit.valid");
+
+      const saveButton = screen.getByRole("button", { name: "Save" });
+      (getSFUConfigWithOpenID as Mock).mockImplementation(() => {
+        throw new Error("Invalid URL");
+      });
+      await user.click(saveButton);
+      expect(
+        screen.getByText("invalid URL (did not update)"),
+      ).toBeInTheDocument();
+      expect(customLivekitUrlSetting.getValue()).toBe(null);
+    });
+  });
 });

src/settings/DeveloperSettingsTab.tsx

@@ -22,6 +22,7 @@ import {
 import { logger } from "matrix-js-sdk/lib/logger";
 import {
   EditInPlace,
+  ErrorMessage,
   Root as Form,
   Heading,
   HelpMessage,
@@ -45,9 +46,11 @@ import {
 } from "./settings";
 import styles from "./DeveloperSettingsTab.module.css";
 import { useUrlParams } from "../UrlParams";
+import { getSFUConfigWithOpenID } from "../livekit/openIDSFU";
 
 interface Props {
   client: MatrixClient;
+  roomId?: string;
   livekitRooms?: {
     room: LivekitRoom;
     url: string;
@@ -60,6 +63,7 @@ interface Props {
 export const DeveloperSettingsTab: FC<Props> = ({
   client,
   livekitRooms,
+  roomId,
   env,
 }) => {
   const { t } = useTranslation();
@@ -97,6 +101,8 @@ export const DeveloperSettingsTab: FC<Props> = ({
     alwaysShowIphoneEarpieceSetting,
   );
 
+  const [customLivekitUrlUpdateError, setCustomLivekitUrlUpdateError] =
+    useState<string | null>(null);
   const [customLivekitUrl, setCustomLivekitUrl] = useSetting(
     customLivekitUrlSetting,
   );
@@ -234,14 +240,36 @@ export const DeveloperSettingsTab: FC<Props> = ({
         savingLabel={t("developer_mode.custom_livekit_url.saving")}
         cancelButtonLabel={t("developer_mode.custom_livekit_url.reset")}
         onSave={useCallback(
-          (e: React.FormEvent<HTMLFormElement>) => {
+          async (e: React.FormEvent<HTMLFormElement>): Promise<void> => {
-            setCustomLivekitUrl(
+            if (
-              customLivekitUrlTextBuffer === ""
+              roomId === undefined ||
-                ? null
+              customLivekitUrlTextBuffer === "" ||
-                : customLivekitUrlTextBuffer,
+              customLivekitUrlTextBuffer === null
+            ) {
+              setCustomLivekitUrl(null);
+              return;
+            }
+
+            try {
+              const userId = client.getUserId();
+              const deviceId = client.getDeviceId();
+
+              if (userId === null || deviceId === null) {
+                throw new Error("Invalid user or device ID");
+              }
+              await getSFUConfigWithOpenID(
+                client,
+                { userId, deviceId, memberId: "" },
+                customLivekitUrlTextBuffer,
+                roomId,
               );
+              setCustomLivekitUrlUpdateError(null);
+              setCustomLivekitUrl(customLivekitUrlTextBuffer);
+            } catch {
+              setCustomLivekitUrlUpdateError("invalid URL (did not update)");
+            }
           },
-          [setCustomLivekitUrl, customLivekitUrlTextBuffer],
+          [customLivekitUrlTextBuffer, setCustomLivekitUrl, client, roomId],
         )}
         value={customLivekitUrlTextBuffer ?? ""}
         onChange={useCallback(
@@ -256,7 +284,12 @@ export const DeveloperSettingsTab: FC<Props> = ({
           },
           [setCustomLivekitUrl],
         )}
-      />
+        serverInvalid={customLivekitUrlUpdateError !== null}
+      >
+        {customLivekitUrlUpdateError !== null && (
+          <ErrorMessage>{customLivekitUrlUpdateError}</ErrorMessage>
+        )}
+      </EditInPlace>
       <Heading as="h3" type="body" weight="semibold" size="lg">
         {t("developer_mode.matrixRTCMode.title")}
       </Heading>

src/settings/SettingsModal.tsx

@@ -213,6 +213,7 @@ export const SettingsModal: FC<Props> = ({
         env={import.meta.env}
         client={client}
         livekitRooms={livekitRooms}
+        roomId={roomId}
       />
     ),
   };

src/settings/settings.ts

@@ -34,15 +34,20 @@ export class Setting<T> {
 
     this._value$ = new BehaviorSubject(initialValue);
     this.value$ = this._value$;
+    this._lastUpdateReason$ = new BehaviorSubject<string | null>(null);
+    this.lastUpdateReason$ = this._lastUpdateReason$;
   }
 
   private readonly key: string;
 
   private readonly _value$: BehaviorSubject<T>;
+  private readonly _lastUpdateReason$: BehaviorSubject<string | null>;
   public readonly value$: Behavior<T>;
+  public readonly lastUpdateReason$: Behavior<string | null>;
 
-  public readonly setValue = (value: T): void => {
+  public readonly setValue = (value: T, reason?: string): void => {
     this._value$.next(value);
+    this._lastUpdateReason$.next(reason ?? null);
     localStorage.setItem(this.key, JSON.stringify(value));
   };
   public readonly getValue = (): T => {