mirror of
https://github.com/vector-im/element-call.git
synced 2026-09-13 21:59:30 +00:00
Surface screen share failures instead of silently doing nothing
toggleScreenSharing only had `.catch(logger.error)`, so a getDisplayMedia request that hangs (element-call-rageshakes#17152: Element Desktop on Windows, the user pressed the screen share button 14 times in 25 seconds and the log shows nothing but the toggle lines and livekit-client's "waiting for pending publication promise timed out") left the user with a button that does nothing and us with no evidence of why. Log when a toggle is requested and when it completes or fails, with the elapsed time, so a hang is visible in the logs. Explicit failures other than the user cancelling the picker show as a non-modal "Could not start screen sharing" toast. Nothing is inferred from a toggle taking a long time: the user may simply be choosing what to share.
This commit is contained in:
@@ -270,6 +270,7 @@ export const InCallView: FC<InCallViewProps> = ({
|
||||
const audioParticipants = useBehavior(vm.livekitRoomItems$);
|
||||
const participantCount = useBehavior(vm.participantCount$);
|
||||
const reconnecting = useBehavior(vm.reconnecting$);
|
||||
const screenShareError = useBehavior(vm.screenShareError$);
|
||||
const layout = useBehavior(vm.layout$);
|
||||
const edgeToEdge = useBehavior(vm.edgeToEdge$);
|
||||
const overflowing = useBehavior(vm.overflowing$);
|
||||
@@ -399,6 +400,21 @@ export const InCallView: FC<InCallViewProps> = ({
|
||||
);
|
||||
}
|
||||
|
||||
const onDismissScreenShareToast = useCallback(
|
||||
() => vm.dismissScreenShareError(),
|
||||
[vm],
|
||||
);
|
||||
const screenShareToast = (
|
||||
<Toast
|
||||
onDismiss={onDismissScreenShareToast}
|
||||
open={screenShareError !== null}
|
||||
autoDismiss={5000}
|
||||
modal={false}
|
||||
>
|
||||
{t("error.screen_share_failed")}
|
||||
</Toast>
|
||||
);
|
||||
|
||||
// The reconnecting toast cannot be dismissed
|
||||
const onDismissReconnectingToast = useCallback(() => {}, []);
|
||||
// We need to use a non-modal toast to avoid trapping focus within the toast.
|
||||
@@ -634,6 +650,7 @@ export const InCallView: FC<InCallViewProps> = ({
|
||||
<ReactionsAudioRenderer vm={vm} muted={muteAllAudio} />
|
||||
<RingingAudioRenderer vm={ringingVm} muted={muteAllAudio} />
|
||||
{reconnectingToast}
|
||||
{screenShareToast}
|
||||
{earpieceOverlay}
|
||||
<ReactionsOverlay vm={vm} />
|
||||
{footer}
|
||||
|
||||
@@ -171,7 +171,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
class="_settingsLogoContainer_20b7b4"
|
||||
>
|
||||
<button
|
||||
aria-labelledby="_r_8_"
|
||||
aria-labelledby="_r_b_"
|
||||
class="_icon-button_1215g_8 _settingsOnlyShowWide_20b7b4"
|
||||
data-kind="secondary"
|
||||
data-testid="settings-bottom-left"
|
||||
@@ -306,7 +306,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
class="_buttons_20b7b4"
|
||||
>
|
||||
<button
|
||||
aria-labelledby="_r_d_"
|
||||
aria-labelledby="_r_g_"
|
||||
class="_button_1nw83_8 _settingsOnlyShowNarrow_20b7b4 _has-icon_1nw83_60 _icon-only_1nw83_53"
|
||||
data-kind="secondary"
|
||||
data-size="lg"
|
||||
@@ -331,7 +331,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
aria-busy="false"
|
||||
aria-checked="false"
|
||||
aria-disabled="true"
|
||||
aria-labelledby="_r_i_"
|
||||
aria-labelledby="_r_l_"
|
||||
class="_button_1nw83_8 _has-icon_1nw83_60 _icon-only_1nw83_53"
|
||||
data-kind="primary"
|
||||
data-size="lg"
|
||||
@@ -356,7 +356,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
aria-busy="false"
|
||||
aria-checked="false"
|
||||
aria-disabled="true"
|
||||
aria-labelledby="_r_n_"
|
||||
aria-labelledby="_r_q_"
|
||||
class="_button_1nw83_8 _has-icon_1nw83_60 _icon-only_1nw83_53"
|
||||
data-kind="primary"
|
||||
data-size="lg"
|
||||
@@ -381,7 +381,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
aria-disabled="false"
|
||||
aria-expanded="false"
|
||||
aria-haspopup="true"
|
||||
aria-labelledby="_r_s_"
|
||||
aria-labelledby="_r_v_"
|
||||
class="_button_1nw83_8 _raiseHand_20b7b4 _has-icon_1nw83_60 _icon-only_1nw83_53"
|
||||
data-kind="secondary"
|
||||
data-size="lg"
|
||||
@@ -404,7 +404,7 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
</svg>
|
||||
</button>
|
||||
<button
|
||||
aria-labelledby="_r_14_"
|
||||
aria-labelledby="_r_17_"
|
||||
class="_button_1nw83_8 _has-icon_1nw83_60 _icon-only_1nw83_53 _destructive_1nw83_110"
|
||||
data-kind="primary"
|
||||
data-size="lg"
|
||||
@@ -432,8 +432,8 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
data-size="lg"
|
||||
>
|
||||
<input
|
||||
aria-labelledby="_r_1a_"
|
||||
name="_r_19_"
|
||||
aria-labelledby="_r_1d_"
|
||||
name="_r_1c_"
|
||||
type="radio"
|
||||
value="spotlight"
|
||||
/>
|
||||
@@ -452,9 +452,9 @@ exports[`InCallView > rendering > renders 1`] = `
|
||||
/>
|
||||
</svg>
|
||||
<input
|
||||
aria-labelledby="_r_1f_"
|
||||
aria-labelledby="_r_1i_"
|
||||
checked=""
|
||||
name="_r_19_"
|
||||
name="_r_1c_"
|
||||
type="radio"
|
||||
value="grid"
|
||||
/>
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -41,6 +41,7 @@ import {
|
||||
enterRTCSession,
|
||||
PublishState,
|
||||
TrackState,
|
||||
watchScreenShareToggle,
|
||||
} from "./LocalMember";
|
||||
import {
|
||||
FailToGetOpenIdToken,
|
||||
@@ -68,6 +69,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", () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user