From a75952cf77cea0fc67058defb43850d7ff153163 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 17 Feb 2025 19:19:31 +0700 Subject: [PATCH 1/2] Send a 'close' action when the widget is ready to close By keeping 'hangup' and 'close' as separate actions, we can allow Element Call widgets to stay on an error screen after the user has been disconnected without the widget completely disappearing from the host's UI. We don't have to request any additional capabilities to use a custom widget action like this one. --- src/room/GroupCallView.test.tsx | 37 +++++++++++++--- src/room/GroupCallView.tsx | 22 ++++++---- ...lper.test.ts => rtcSessionHelpers.test.ts} | 43 ++++++++++++++++++- src/rtcSessionHelpers.ts | 16 ++++--- src/utils/test.ts | 4 ++ src/widget.ts | 3 +- tsconfig.json | 2 +- 7 files changed, 105 insertions(+), 22 deletions(-) rename src/{rtcSessionHelper.test.ts => rtcSessionHelpers.test.ts} (64%) diff --git a/src/room/GroupCallView.test.tsx b/src/room/GroupCallView.test.tsx index ed975d14..7d53194d 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 } from "@testing-library/react"; +import { render, waitFor } 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"; @@ -20,6 +20,7 @@ import { prefetchSounds } from "../soundUtils"; import { useAudioContext } from "../useAudioContext"; import { ActiveCall } from "./InCallView"; import { + flushPromises, mockMatrixRoom, mockMatrixRoomMember, mockRtcMembership, @@ -51,13 +52,13 @@ const carol = mockMatrixRoomMember(localRtcMember); const roomMembers = new Map([carol].map((p) => [p.userId, p])); const roomId = "!foo:bar"; -const soundPromise = Promise.resolve(true); beforeEach(() => { + vitest.clearAllMocks(); (prefetchSounds as MockedFunction).mockResolvedValue({ sound: new ArrayBuffer(0), }); - playSound = vitest.fn().mockReturnValue(soundPromise); + playSound = vitest.fn(); (useAudioContext as MockedFunction).mockReturnValue({ playSound, }); @@ -136,8 +137,15 @@ test("will play a leave sound asynchronously in SPA mode", async () => { const leaveButton = getByText("Leave"); await user.click(leaveButton); expect(playSound).toHaveBeenCalledWith("left"); - expect(leaveRTCSession).toHaveBeenCalledWith(rtcSession, undefined); + expect(leaveRTCSession).toHaveBeenCalledWith( + rtcSession, + "user", + 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()); }); test("will play a leave sound synchronously in widget mode", async () => { @@ -148,12 +156,31 @@ test("will play a leave sound synchronously in widget mode", async () => { } as Partial, lazyActions: new LazyEventEmitter(), }; + let resolvePlaySound: () => void; + playSound = vitest + .fn() + .mockReturnValue( + new Promise((resolve) => (resolvePlaySound = resolve)), + ); + (useAudioContext as MockedFunction).mockReturnValue({ + playSound, + }); + const { getByText, rtcSession } = createGroupCallView( widget as WidgetHelpers, ); const leaveButton = getByText("Leave"); await user.click(leaveButton); + await flushPromises(); + expect(leaveRTCSession).not.toHaveResolved(); + resolvePlaySound!(); + await flushPromises(); + expect(playSound).toHaveBeenCalledWith("left"); - expect(leaveRTCSession).toHaveBeenCalledWith(rtcSession, soundPromise); + expect(leaveRTCSession).toHaveBeenCalledWith( + rtcSession, + "user", + expect.any(Promise), + ); expect(rtcSession.leaveRoomSession).toHaveBeenCalledOnce(); }); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 705f29bf..8bd7a622 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -246,17 +246,23 @@ export const GroupCallView: FC = ({ const sendInstantly = !!widget; setLeaveError(leaveError); setLeft(true); - PosthogAnalytics.instance.eventCallEnded.track( - rtcSession.room.roomId, - rtcSession.memberships.length, - sendInstantly, - rtcSession, - ); + // we need to wait until the callEnded event is tracked on posthog. + // Otherwise the iFrame gets killed before the callEnded event got tracked. + const posthogRequest = new Promise((resolve) => { + PosthogAnalytics.instance.eventCallEnded.track( + rtcSession.room.roomId, + rtcSession.memberships.length, + sendInstantly, + rtcSession, + ); + window.setTimeout(resolve, 10); + }); leaveRTCSession( rtcSession, + leaveError === undefined ? "user" : "error", // Wait for the sound in widget mode (it's not long) - sendInstantly && audioPromise ? audioPromise : undefined, + Promise.all([audioPromise, posthogRequest]), ) // Only sends matrix leave event. The Livekit session will disconnect once the ActiveCall-view unmounts. .then(async () => { @@ -292,7 +298,7 @@ export const GroupCallView: FC = ({ const onHangup = (ev: CustomEvent): void => { widget.api.transport.reply(ev.detail, {}); // Only sends matrix leave event. The Livekit session will disconnect once the ActiveCall-view unmounts. - leaveRTCSession(rtcSession).catch((e) => { + leaveRTCSession(rtcSession, "user").catch((e) => { logger.error("Failed to leave RTC session", e); }); }; diff --git a/src/rtcSessionHelper.test.ts b/src/rtcSessionHelpers.test.ts similarity index 64% rename from src/rtcSessionHelper.test.ts rename to src/rtcSessionHelpers.test.ts index b5d4dbb8..57f73f8e 100644 --- a/src/rtcSessionHelper.test.ts +++ b/src/rtcSessionHelpers.test.ts @@ -8,9 +8,20 @@ Please see LICENSE in the repository root for full details. import { type MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; import { expect, test, vi } from "vitest"; import { AutoDiscovery } from "matrix-js-sdk/src/autodiscovery"; +import EventEmitter from "events"; -import { enterRTCSession } from "../src/rtcSessionHelpers"; +import { enterRTCSession, leaveRTCSession } from "../src/rtcSessionHelpers"; import { mockConfig } from "./utils/test"; +import { ElementWidgetActions, widget } from "./widget"; + +const actualWidget = await vi.hoisted(async () => vi.importActual("./widget")); +vi.mock("./widget", () => ({ + ...actualWidget, + widget: { + api: { transport: { send: vi.fn(), reply: vi.fn(), stop: vi.fn() } }, + lazyActions: new EventEmitter(), + }, +})); test("It joins the correct Session", async () => { const focusFromOlderMembership = { @@ -96,3 +107,33 @@ test("It joins the correct Session", async () => { }, ); }); + +test("leaveRTCSession closes the widget on a normal hangup", async () => { + vi.clearAllMocks(); + const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; + await leaveRTCSession(session, "user"); + expect(session.leaveRoomSession).toHaveBeenCalled(); + expect(widget!.api.transport.send).toHaveBeenCalledWith( + ElementWidgetActions.HangupCall, + expect.anything(), + ); + expect(widget!.api.transport.send).toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); +}); + +test("leaveRTCSession doesn't close the widget on a fatal error", async () => { + vi.clearAllMocks(); + const session = { leaveRoomSession: vi.fn() } as unknown as MatrixRTCSession; + await leaveRTCSession(session, "error"); + expect(session.leaveRoomSession).toHaveBeenCalled(); + expect(widget!.api.transport.send).toHaveBeenCalledWith( + ElementWidgetActions.HangupCall, + expect.anything(), + ); + expect(widget!.api.transport.send).not.toHaveBeenCalledWith( + ElementWidgetActions.Close, + expect.anything(), + ); +}); diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 99a4fee6..13cfae1e 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -130,13 +130,9 @@ export async function enterRTCSession( const widgetPostHangupProcedure = async ( widget: WidgetHelpers, + cause: "user" | "error", promiseBeforeHangup?: Promise, ): Promise => { - // we need to wait until the callEnded event is tracked on posthog. - // Otherwise the iFrame gets killed before the callEnded event got tracked. - await new Promise((resolve) => window.setTimeout(resolve, 10)); // 10ms - PosthogAnalytics.instance.logout(); - try { await widget.api.setAlwaysOnScreen(false); } catch (e) { @@ -149,15 +145,23 @@ const widgetPostHangupProcedure = async ( // calling leaveRTCSession. // We need to wait because this makes the client hosting this widget killing the IFrame. await widget.api.transport.send(ElementWidgetActions.HangupCall, {}); + // On a normal user hangup we can shut down and close the widget. But if an + // error occurs we should keep the widget open until the user reads it. + if (cause === "user") { + await widget.api.transport.send(ElementWidgetActions.Close, {}); + widget.api.transport.stop(); + PosthogAnalytics.instance.logout(); + } }; export async function leaveRTCSession( rtcSession: MatrixRTCSession, + cause: "user" | "error", promiseBeforeHangup?: Promise, ): Promise { await rtcSession.leaveRoomSession(); if (widget) { - await widgetPostHangupProcedure(widget, promiseBeforeHangup); + await widgetPostHangupProcedure(widget, cause, promiseBeforeHangup); } else { await promiseBeforeHangup; } diff --git a/src/utils/test.ts b/src/utils/test.ts index 00a1a97d..b6f0ecc3 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -50,6 +50,10 @@ export function withFakeTimers(continuation: () => void): void { } } +export async function flushPromises(): Promise { + await new Promise((resolve) => window.setTimeout(resolve)); +} + export interface OurRunHelpers extends RunHelpers { /** * Schedules a sequence of actions to happen, as described by a marble diff --git a/src/widget.ts b/src/widget.ts index dd769248..e9b931fa 100644 --- a/src/widget.ts +++ b/src/widget.ts @@ -21,10 +21,11 @@ import { getUrlParams } from "./UrlParams"; import { Config } from "./config/Config"; import { ElementCallReactionEventType } from "./reactions"; -// Subset of the actions in matrix-react-sdk +// Subset of the actions in element-web export enum ElementWidgetActions { JoinCall = "io.element.join", HangupCall = "im.vector.hangup", + Close = "io.element.close", TileLayout = "io.element.tile_layout", SpotlightLayout = "io.element.spotlight_layout", // This can be sent as from or to widget diff --git a/tsconfig.json b/tsconfig.json index 0f0c9c94..12814c82 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "target": "es2022", - "module": "es2020", + "module": "es2022", "jsx": "react-jsx", "lib": ["es2022", "dom", "dom.iterable"], From 518c8eadca630e06e7c2ebc30e1a89c8cb5be812 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 24 Feb 2025 11:43:15 +0700 Subject: [PATCH 2/2] Finish the hangup procedure even if widget API throws errors --- src/rtcSessionHelpers.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/rtcSessionHelpers.ts b/src/rtcSessionHelpers.ts index 13cfae1e..52498516 100644 --- a/src/rtcSessionHelpers.ts +++ b/src/rtcSessionHelpers.ts @@ -144,11 +144,19 @@ const widgetPostHangupProcedure = async ( // We send the hangup event after the memberships have been updated // calling leaveRTCSession. // We need to wait because this makes the client hosting this widget killing the IFrame. - await widget.api.transport.send(ElementWidgetActions.HangupCall, {}); + try { + await widget.api.transport.send(ElementWidgetActions.HangupCall, {}); + } catch (e) { + logger.error("Failed to send hangup action", e); + } // On a normal user hangup we can shut down and close the widget. But if an // error occurs we should keep the widget open until the user reads it. if (cause === "user") { - await widget.api.transport.send(ElementWidgetActions.Close, {}); + try { + await widget.api.transport.send(ElementWidgetActions.Close, {}); + } catch (e) { + logger.error("Failed to send close action", e); + } widget.api.transport.stop(); PosthogAnalytics.instance.logout(); }