Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Proactively fix stuck devices in video rooms (#8587)
Browse files Browse the repository at this point in the history
* Proactively fix stuck devices in video rooms

* Fix tests

* Explain why we're disabling the lint rule

* Apply code review suggestions

* Back VideoChannelStore's flags by SettingsStore instead of localStorage
  • Loading branch information
robintown authored May 16, 2022
1 parent 6f85110 commit ceda77d
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 49 deletions.
7 changes: 7 additions & 0 deletions src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ import StorageEvictedDialog from "./components/views/dialogs/StorageEvictedDialo
import { setSentryUser } from "./sentry";
import SdkConfig from "./SdkConfig";
import { DialogOpener } from "./utils/DialogOpener";
import VideoChannelStore from "./stores/VideoChannelStore";
import { fixStuckDevices } from "./utils/VideoChannelUtils";
import { Action } from "./dispatcher/actions";
import AbstractLocalStorageSettingsHandler from "./settings/handlers/AbstractLocalStorageSettingsHandler";

Expand Down Expand Up @@ -835,6 +837,11 @@ async function startMatrixClient(startSyncing = true): Promise<void> {
// Now that we have a MatrixClientPeg, update the Jitsi info
Jitsi.getInstance().start();

// In case we disconnected uncleanly from a video room, clean up the stuck device
if (VideoChannelStore.instance.roomId) {
fixStuckDevices(MatrixClientPeg.get().getRoom(VideoChannelStore.instance.roomId), false);
}

// dispatch that we finished starting up to wire up any other bits
// of the matrix client that cannot be set prior to starting up.
dis.dispatch({ action: 'client_started' });
Expand Down
10 changes: 8 additions & 2 deletions src/components/structures/VideoRoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { FC, useContext, useState, useMemo } from "react";
import React, { FC, useContext, useState, useMemo, useEffect } from "react";
import { logger } from "matrix-js-sdk/src/logger";
import { Room } from "matrix-js-sdk/src/models/room";

import MatrixClientContext from "../../contexts/MatrixClientContext";
import { useEventEmitter } from "../../hooks/useEventEmitter";
import WidgetUtils from "../../utils/WidgetUtils";
import { addVideoChannel, getVideoChannel } from "../../utils/VideoChannelUtils";
import { addVideoChannel, getVideoChannel, fixStuckDevices } from "../../utils/VideoChannelUtils";
import WidgetStore, { IApp } from "../../stores/WidgetStore";
import { UPDATE_EVENT } from "../../stores/AsyncStore";
import VideoChannelStore, { VideoChannelEvent } from "../../stores/VideoChannelStore";
Expand Down Expand Up @@ -62,6 +62,12 @@ const VideoRoomView: FC<IProps> = ({ room, resizing }) => {
}
}, [room, widgetStoreReady, widgetLoaded]); // eslint-disable-line react-hooks/exhaustive-deps

// We'll also take this opportunity to fix any stuck devices.
// The linter thinks that store.connected should be a dependency, but we explicitly
// *only* want this to happen at mount to avoid racing with normal device updates.
// eslint-disable-next-line react-hooks/exhaustive-deps
useEffect(() => { fixStuckDevices(room, store.connected); }, [room]);

const [connected, setConnected] = useState(store.connected && store.roomId === room.roomId);
useEventEmitter(store, VideoChannelEvent.Connect, () => setConnected(store.roomId === room.roomId));
useEventEmitter(store, VideoChannelEvent.Disconnect, () => setConnected(false));
Expand Down
12 changes: 12 additions & 0 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,18 @@ export const SETTINGS: {[setting: string]: ISetting} = {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
default: false,
},
"audioInputMuted": {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
default: false,
},
"videoInputMuted": {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
default: false,
},
"videoChannelRoomId": {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
default: null,
},
[UIFeature.RoomHistorySettings]: {
supportedLevels: LEVELS_UI_FEATURE,
default: true,
Expand Down
55 changes: 19 additions & 36 deletions src/stores/VideoChannelStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ import EventEmitter from "events";
import { Room, RoomEvent } from "matrix-js-sdk/src/models/room";
import { ClientWidgetApi, IWidgetApiRequest } from "matrix-widget-api";

import SettingsStore from "../settings/SettingsStore";
import { SettingLevel } from "../settings/SettingLevel";
import defaultDispatcher from "../dispatcher/dispatcher";
import { ActionPayload } from "../dispatcher/payloads";
import { ElementWidgetActions } from "./widgets/ElementWidgetActions";
import { WidgetMessagingStore, WidgetMessagingStoreEvent } from "./widgets/WidgetMessagingStore";
import {
VIDEO_CHANNEL_MEMBER,
IVideoChannelMemberContent,
getVideoChannel,
} from "../utils/VideoChannelUtils";
import { getVideoChannel, addOurDevice, removeOurDevice } from "../utils/VideoChannelUtils";
import { timeout } from "../utils/promise";
import WidgetUtils from "../utils/WidgetUtils";
import { AsyncStoreWithClient } from "./AsyncStoreWithClient";
Expand Down Expand Up @@ -83,9 +81,13 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {

private activeChannel: ClientWidgetApi;

private _roomId: string;
public get roomId(): string { return this._roomId; }
private set roomId(value: string) { this._roomId = value; }
// This is persisted to settings so we can detect unclean disconnects
public get roomId(): string | null { return SettingsStore.getValue("videoChannelRoomId"); }
private set roomId(value: string | null) {
SettingsStore.setValue("videoChannelRoomId", null, SettingLevel.DEVICE, value);
}

private get room(): Room { return this.matrixClient.getRoom(this.roomId); }

private _connected = false;
public get connected(): boolean { return this._connected; }
Expand All @@ -95,18 +97,14 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {
public get participants(): IJitsiParticipant[] { return this._participants; }
private set participants(value: IJitsiParticipant[]) { this._participants = value; }

private _audioMuted = localStorage.getItem("mx_audioMuted") === "true";
public get audioMuted(): boolean { return this._audioMuted; }
public get audioMuted(): boolean { return SettingsStore.getValue("audioInputMuted"); }
public set audioMuted(value: boolean) {
this._audioMuted = value;
localStorage.setItem("mx_audioMuted", value.toString());
SettingsStore.setValue("audioInputMuted", null, SettingLevel.DEVICE, value);
}

private _videoMuted = localStorage.getItem("mx_videoMuted") === "true";
public get videoMuted(): boolean { return this._videoMuted; }
public get videoMuted(): boolean { return SettingsStore.getValue("videoInputMuted"); }
public set videoMuted(value: boolean) {
this._videoMuted = value;
localStorage.setItem("mx_videoMuted", value.toString());
SettingsStore.setValue("videoInputMuted", null, SettingLevel.DEVICE, value);
}

public connect = async (roomId: string, audioDevice: MediaDeviceInfo, videoDevice: MediaDeviceInfo) => {
Expand Down Expand Up @@ -198,13 +196,13 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {
}

this.connected = true;
this.matrixClient.getRoom(roomId).on(RoomEvent.MyMembership, this.onMyMembership);
this.room.on(RoomEvent.MyMembership, this.onMyMembership);
window.addEventListener("beforeunload", this.setDisconnected);

this.emit(VideoChannelEvent.Connect, roomId);

// Tell others that we're connected, by adding our device to room state
this.updateDevices(roomId, devices => Array.from(new Set(devices).add(this.matrixClient.getDeviceId())));
await addOurDevice(this.room);
};

public disconnect = async () => {
Expand All @@ -221,10 +219,11 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {

public setDisconnected = async () => {
const roomId = this.roomId;
const room = this.room;

this.activeChannel.off(`action:${ElementWidgetActions.HangupCall}`, this.onHangup);
this.activeChannel.off(`action:${ElementWidgetActions.CallParticipants}`, this.onParticipants);
this.matrixClient.getRoom(roomId).off(RoomEvent.MyMembership, this.onMyMembership);
room.off(RoomEvent.MyMembership, this.onMyMembership);
window.removeEventListener("beforeunload", this.setDisconnected);

this.activeChannel = null;
Expand All @@ -235,11 +234,7 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {
this.emit(VideoChannelEvent.Disconnect, roomId);

// Tell others that we're disconnected, by removing our device from room state
await this.updateDevices(roomId, devices => {
const devicesSet = new Set(devices);
devicesSet.delete(this.matrixClient.getDeviceId());
return Array.from(devicesSet);
});
await removeOurDevice(room);
};

private ack = (ev: CustomEvent<IWidgetApiRequest>) => {
Expand All @@ -248,18 +243,6 @@ export default class VideoChannelStore extends AsyncStoreWithClient<null> {
this.activeChannel.transport.reply(ev.detail, {});
};

private updateDevices = async (roomId: string, fn: (devices: string[]) => string[]) => {
const room = this.matrixClient.getRoom(roomId);
if (room.getMyMembership() !== "join") return;

const devicesState = room.currentState.getStateEvents(VIDEO_CHANNEL_MEMBER, this.matrixClient.getUserId());
const devices = devicesState?.getContent<IVideoChannelMemberContent>()?.devices ?? [];

await this.matrixClient.sendStateEvent(
roomId, VIDEO_CHANNEL_MEMBER, { devices: fn(devices) }, this.matrixClient.getUserId(),
);
};

private onHangup = async (ev: CustomEvent<IWidgetApiRequest>) => {
this.ack(ev);
// In case this hangup is caused by Jitsi Meet crashing at startup,
Expand Down
61 changes: 58 additions & 3 deletions src/utils/VideoChannelUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ limitations under the License.

import { useState } from "react";
import { throttle } from "lodash";
import { Optional } from "matrix-events-sdk";
import { IMyDevice } from "matrix-js-sdk/src/client";
import { CallType } from "matrix-js-sdk/src/webrtc/call";
import { Room } from "matrix-js-sdk/src/models/room";
import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
Expand All @@ -26,14 +28,16 @@ import WidgetStore, { IApp } from "../stores/WidgetStore";
import { WidgetType } from "../widgets/WidgetType";
import WidgetUtils from "./WidgetUtils";

export const VIDEO_CHANNEL = "io.element.video";
export const VIDEO_CHANNEL_MEMBER = "io.element.video.member";
const STUCK_DEVICE_TIMEOUT_MS = 1000 * 60 * 60;

export interface IVideoChannelMemberContent {
interface IVideoChannelMemberContent {
// Connected device IDs
devices: string[];
}

export const VIDEO_CHANNEL = "io.element.video";
export const VIDEO_CHANNEL_MEMBER = "io.element.video.member";

export const getVideoChannel = (roomId: string): IApp => {
const apps = WidgetStore.instance.getApps(roomId);
return apps.find(app => WidgetType.JITSI.matches(app.type) && app.id === VIDEO_CHANNEL);
Expand Down Expand Up @@ -72,3 +76,54 @@ export const useConnectedMembers = (
}, throttleMs, { leading: true, trailing: true }));
return members;
};

const updateDevices = async (room: Optional<Room>, fn: (devices: string[] | null) => string[]) => {
if (room?.getMyMembership() !== "join") return;

const devicesState = room.currentState.getStateEvents(VIDEO_CHANNEL_MEMBER, room.client.getUserId());
const devices = devicesState?.getContent<IVideoChannelMemberContent>()?.devices ?? [];
const newDevices = fn(devices);

if (newDevices) {
await room.client.sendStateEvent(
room.roomId, VIDEO_CHANNEL_MEMBER, { devices: newDevices }, room.client.getUserId(),
);
}
};

export const addOurDevice = async (room: Room) => {
await updateDevices(room, devices => Array.from(new Set(devices).add(room.client.getDeviceId())));
};

export const removeOurDevice = async (room: Room) => {
await updateDevices(room, devices => {
const devicesSet = new Set(devices);
devicesSet.delete(room.client.getDeviceId());
return Array.from(devicesSet);
});
};

/**
* Fixes devices that may have gotten stuck in video channel member state after
* an unclean disconnection, by filtering out logged out devices, inactive
* devices, and our own device (if we're disconnected).
* @param {Room} room The room to fix
* @param {boolean} connectedLocalEcho Local echo of whether this device is connected
*/
export const fixStuckDevices = async (room: Room, connectedLocalEcho: boolean) => {
const now = new Date().valueOf();
const { devices: myDevices } = await room.client.getDevices();
const deviceMap = new Map<string, IMyDevice>(myDevices.map(d => [d.device_id, d]));

await updateDevices(room, devices => {
const newDevices = devices.filter(d => {
const device = deviceMap.get(d);
return device?.last_seen_ts
&& !(d === room.client.getDeviceId() && !connectedLocalEcho)
&& (now - device.last_seen_ts) < STUCK_DEVICE_TIMEOUT_MS;
});

// Skip the update if the devices are unchanged
return newDevices.length === devices.length ? null : newDevices;
});
};
38 changes: 36 additions & 2 deletions test/components/structures/VideoRoomView-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ limitations under the License.
import React from "react";
import { mount } from "enzyme";
import { act } from "react-dom/test-utils";
import { MatrixClient } from "matrix-js-sdk/src/client";
import { mocked } from "jest-mock";
import { MatrixClient, IMyDevice } from "matrix-js-sdk/src/client";
import { Room } from "matrix-js-sdk/src/models/room";
import { MatrixWidgetType } from "matrix-widget-api";

Expand All @@ -27,9 +28,11 @@ import {
StubVideoChannelStore,
mkRoom,
wrapInMatrixClientContext,
mockStateEventImplementation,
mkVideoChannelMember,
} from "../../test-utils";
import { MatrixClientPeg } from "../../../src/MatrixClientPeg";
import { VIDEO_CHANNEL } from "../../../src/utils/VideoChannelUtils";
import { VIDEO_CHANNEL, VIDEO_CHANNEL_MEMBER } from "../../../src/utils/VideoChannelUtils";
import WidgetStore from "../../../src/stores/WidgetStore";
import _VideoRoomView from "../../../src/components/structures/VideoRoomView";
import VideoLobby from "../../../src/components/views/voip/VideoLobby";
Expand Down Expand Up @@ -64,6 +67,37 @@ describe("VideoRoomView", () => {
room = mkRoom(cli, "!1:example.org");
});

it("removes stuck devices on mount", async () => {
// Simulate an unclean disconnect
store.roomId = "!1:example.org";

const devices: IMyDevice[] = [
{
device_id: cli.getDeviceId(),
last_seen_ts: new Date().valueOf(),
},
{
device_id: "went offline 2 hours ago",
last_seen_ts: new Date().valueOf() - 1000 * 60 * 60 * 2,
},
];
mocked(cli).getDevices.mockResolvedValue({ devices });

// Make both devices be stuck
mocked(room.currentState).getStateEvents.mockImplementation(mockStateEventImplementation([
mkVideoChannelMember(cli.getUserId(), devices.map(d => d.device_id)),
]));

mount(<VideoRoomView room={room} resizing={false} />);
// Wait for state to settle
await act(() => Promise.resolve());

// All devices should have been removed
expect(cli.sendStateEvent).toHaveBeenLastCalledWith(
"!1:example.org", VIDEO_CHANNEL_MEMBER, { devices: [] }, cli.getUserId(),
);
});

it("shows lobby and keeps widget loaded when disconnected", async () => {
const view = mount(<VideoRoomView room={room} resizing={false} />);
// Wait for state to settle
Expand Down
3 changes: 2 additions & 1 deletion test/stores/VideoChannelStore-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ limitations under the License.
import { mocked } from "jest-mock";
import { Widget, ClientWidgetApi, MatrixWidgetType, IWidgetApiRequest } from "matrix-widget-api";

import { stubClient, setupAsyncStoreWithClient } from "../test-utils";
import { stubClient, setupAsyncStoreWithClient, mkRoom } from "../test-utils";
import { MatrixClientPeg } from "../../src/MatrixClientPeg";
import WidgetStore, { IApp } from "../../src/stores/WidgetStore";
import { WidgetMessagingStore } from "../../src/stores/widgets/WidgetMessagingStore";
Expand Down Expand Up @@ -51,6 +51,7 @@ describe("VideoChannelStore", () => {
const cli = MatrixClientPeg.get();
setupAsyncStoreWithClient(WidgetMessagingStore.instance, cli);
setupAsyncStoreWithClient(store, cli);
mocked(cli).getRoom.mockReturnValue(mkRoom(cli, "!1:example.org"));

let resolveMessageSent: () => void;
messageSent = new Promise(resolve => resolveMessageSent = resolve);
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export function createTestClient(): MatrixClient {
getUserId: jest.fn().mockReturnValue("@userId:matrix.rog"),
getUser: jest.fn().mockReturnValue({ on: jest.fn() }),
getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"),
getDevices: jest.fn().mockResolvedValue({ devices: [{ device_id: "ABCDEFGHI" }] }),
credentials: { userId: "@userId:matrix.rog" },

getPushActionsForEvent: jest.fn(),
Expand Down
11 changes: 6 additions & 5 deletions test/test-utils/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,25 @@ import { VIDEO_CHANNEL_MEMBER } from "../../src/utils/VideoChannelUtils";
import VideoChannelStore, { VideoChannelEvent, IJitsiParticipant } from "../../src/stores/VideoChannelStore";

export class StubVideoChannelStore extends EventEmitter {
private _roomId: string;
public get roomId(): string { return this._roomId; }
private _roomId: string | null;
public get roomId(): string | null { return this._roomId; }
public set roomId(value: string | null) { this._roomId = value; }
private _connected: boolean;
public get connected(): boolean { return this._connected; }
public get participants(): IJitsiParticipant[] { return []; }

public startConnect = (roomId: string) => {
this._roomId = roomId;
this.roomId = roomId;
this.emit(VideoChannelEvent.StartConnect, roomId);
};
public connect = jest.fn((roomId: string) => {
this._roomId = roomId;
this.roomId = roomId;
this._connected = true;
this.emit(VideoChannelEvent.Connect, roomId);
});
public disconnect = jest.fn(() => {
const roomId = this._roomId;
this._roomId = null;
this.roomId = null;
this._connected = false;
this.emit(VideoChannelEvent.Disconnect, roomId);
});
Expand Down

0 comments on commit ceda77d

Please sign in to comment.