Merge pull request #4232 from element-hq/matthew/screenshare-watchdog

Surface screen share failures instead of silently doing nothing

src/state/CallViewModel/CallViewModel.ts

@@ -267,6 +267,11 @@ export interface CallViewModel {
    * Whether we are sharing our screen.
    */
   sharingScreen$: Behavior<boolean>;
+  /**
+   * The last error from toggling screen sharing, until dismissed.
+   */
+  screenShareError$: Behavior<Error | null>;
+  dismissScreenShareError: () => void;
 
   // UI interactions
   /**
@@ -1858,6 +1863,8 @@ export function createCallViewModel$(
     reconnecting$: localMembership.reconnecting$,
     livekitRoomItems$,
     connected$: localMembership.connected$,
+    screenShareError$: localMembership.screenShareError$,
+    dismissScreenShareError: localMembership.dismissScreenShareError,
   };
 }
 

src/state/CallViewModel/localMember/LocalMember.test.ts

@@ -31,6 +31,7 @@ import {
   flushPromises,
   mockConfig,
   mockLivekitRoom,
+  mockLocalParticipant,
   mockMuteStates,
   withTestScheduler,
   ownMemberMock,
@@ -41,6 +42,7 @@ import {
   enterRTCSession,
   PublishState,
   TrackState,
+  watchScreenShareToggle,
 } from "./LocalMember";
 import {
   FailToGetOpenIdToken,
@@ -68,6 +70,31 @@ vi.mock("@livekit/components-core", () => ({
     .mockReturnValue(of({ isScreenShareEnabled: false })),
 }));
 
+describe("watchScreenShareToggle", () => {
+  it("reports nothing when the toggle completes", async () => {
+    const onError = vi.fn();
+    watchScreenShareToggle(Promise.resolve(), true, logger, onError);
+    await flushPromises();
+    expect(onError).not.toHaveBeenCalled();
+  });
+
+  it("reports failures other than the user cancelling", async () => {
+    const onError = vi.fn();
+    const e = new Error("NotReadableError");
+    watchScreenShareToggle(Promise.reject(e), true, logger, onError);
+    await flushPromises();
+    expect(onError).toHaveBeenCalledWith(e);
+  });
+
+  it("does not report the user cancelling the picker", async () => {
+    const onError = vi.fn();
+    const cancelled = new DOMException("Permission denied", "NotAllowedError");
+    watchScreenShareToggle(Promise.reject(cancelled), true, logger, onError);
+    await flushPromises();
+    expect(onError).not.toHaveBeenCalled();
+  });
+});
+
 describe("LocalMembership", () => {
   describe("enterRTCSession", () => {
     it("It joins the correct Session", () => {
@@ -854,4 +881,98 @@ describe("LocalMembership", () => {
       scope.end();
     });
   });
+
+  describe("toggleScreenSharing", () => {
+    let originalMediaDevices: MediaDevices | undefined;
+
+    beforeAll(() => {
+      mockConfig();
+      // Screen sharing is only offered when getDisplayMedia is available.
+      originalMediaDevices = navigator.mediaDevices;
+      Object.defineProperty(navigator, "mediaDevices", {
+        configurable: true,
+        value: { getDisplayMedia: vi.fn() },
+      });
+    });
+
+    afterAll(() => {
+      Object.defineProperty(navigator, "mediaDevices", {
+        configurable: true,
+        value: originalMediaDevices,
+      });
+    });
+
+    const createMembershipWithConnection = (
+      connection: Connection | null,
+    ): {
+      scope: ObservableScope;
+      localMembership: ReturnType<typeof createLocalMembership$>;
+    } => {
+      const scope = new ObservableScope();
+      const connectionManagerData = new ConnectionManagerData();
+      if (connection) connectionManagerData.add(connection, []);
+      const localMembership = createLocalMembership$({
+        scope,
+        ...defaultCreateLocalMemberValues,
+        connectionManager: {
+          connectionManagerData$: constant(new Epoch(connectionManagerData)),
+        },
+        localTransport$: new BehaviorSubject({
+          advertised$: new BehaviorSubject(aTransport),
+          active$: new BehaviorSubject(aTransportWithSFUConfig),
+        }),
+      });
+      return { scope, localMembership };
+    };
+
+    it("surfaces a failure and clears it on dismiss", async () => {
+      const error = new Error("NotReadableError");
+      const setScreenShareEnabled = vi.fn().mockRejectedValue(error);
+      const connection = {
+        state$: constant(ConnectionState.LivekitConnected),
+        transport: aTransport,
+        livekitRoom: mockLivekitRoom({
+          localParticipant: mockLocalParticipant({
+            isScreenShareEnabled: false,
+            setScreenShareEnabled,
+          }),
+        }),
+      } as unknown as Connection;
+      const { scope, localMembership } =
+        createMembershipWithConnection(connection);
+      await flushPromises();
+
+      expect(localMembership.toggleScreenSharing).not.toBeNull();
+      expect(localMembership.screenShareError$.value).toBeNull();
+
+      localMembership.toggleScreenSharing!();
+      await flushPromises();
+
+      expect(setScreenShareEnabled).toHaveBeenCalledWith(
+        true,
+        expect.any(Object),
+        undefined,
+      );
+      expect(localMembership.screenShareError$.value).toBe(error);
+
+      localMembership.dismissScreenShareError();
+      expect(localMembership.screenShareError$.value).toBeNull();
+
+      scope.end();
+    });
+
+    it("does nothing when there is no local participant", async () => {
+      // No connection means participant$ never resolves to a participant.
+      const { scope, localMembership } = createMembershipWithConnection(null);
+      await flushPromises();
+
+      expect(localMembership.toggleScreenSharing).not.toBeNull();
+      localMembership.toggleScreenSharing!();
+      await flushPromises();
+
+      expect(localMembership.screenShareError$.value).toBeNull();
+
+      scope.end();
+    });
+  });
 });

src/state/CallViewModel/localMember/LocalMember.ts

@@ -196,6 +196,11 @@ export const createLocalMembership$ = ({
    * Callback to toggle screen sharing. If null, screen sharing is not possible.
    */
   toggleScreenSharing: (() => void) | null;
+  /**
+   * The last error from toggling screen sharing, until dismissed.
+   */
+  screenShareError$: Behavior<Error | null>;
+  dismissScreenShareError: () => void;
   // tracks$: Behavior<LocalTrack[]>;
   participant$: Behavior<LocalParticipant | null>;
   connection$: Behavior<Connection | null>;
@@ -706,6 +711,7 @@ export const createLocalMembership$ = ({
     ),
   );
 
+  const screenShareError$ = new BehaviorSubject<Error | null>(null);
   let toggleScreenSharing: (() => void) | null = null;
   if (
     "getDisplayMedia" in (navigator.mediaDevices ?? {}) &&
@@ -777,13 +783,18 @@ export const createLocalMembership$ = ({
       // We also allow screen sharing to be toggled even if the connection
       // is still initializing or publishing tracks, because there's no
       // technical reason to disallow this. LiveKit will publish if it can.
-      participant$.value
-        ?.setScreenShareEnabled(
+      const participant = participant$.value;
+      if (!participant) return;
+      watchScreenShareToggle(
+        participant.setScreenShareEnabled(
           targetScreenshareState,
           screenshareSettings,
           publishOptions,
-        )
-        .catch(logger.error);
+        ),
+        targetScreenshareState,
+        logger,
+        (e) => screenShareError$.next(e),
+      );
     };
   }
 
@@ -802,11 +813,44 @@ export const createLocalMembership$ = ({
     ),
     sharingScreen$,
     toggleScreenSharing,
+    screenShareError$,
+    dismissScreenShareError: () => screenShareError$.next(null),
     connection$: localConnection$,
     internalLoggerRef: logger,
   };
 };
 
+/**
+ * Logs the outcome of a screen share toggle and reports failures.
+ *
+ * getDisplayMedia may legitimately take a long time (the user is choosing
+ * what to share) or never settle at all, so nothing is inferred from silence:
+ * the request and its completion are logged with the elapsed time so that a
+ * hang is visible in the logs, and only an explicit rejection is reported.
+ *
+ * The user cancelling the picker rejects with a NotAllowedError; that is
+ * logged but not reported.
+ */
+export function watchScreenShareToggle(
+  toggle: Promise<unknown>,
+  enable: boolean,
+  logger: Logger,
+  onError: (e: Error) => void,
+): void {
+  const what = `Screen share ${enable ? "start" : "stop"}`;
+  const started = Date.now();
+  const elapsed = (): string => `${Date.now() - started} ms`;
+  logger.info(`${what} requested`);
+  toggle.then(
+    () => logger.info(`${what} completed in ${elapsed()}`),
+    (e: unknown) => {
+      logger.error(`${what} failed after ${elapsed()}:`, e);
+      if (e instanceof DOMException && e.name === "NotAllowedError") return;
+      onError(e instanceof Error ? e : new Error(String(e)));
+    },
+  );
+}
+
 export function observeSharingScreen$(p: Participant): Observable<boolean> {
   return observeParticipantEvents(
     p,