await promises (#10992)

matrix-org · May 28, 2023 · 9080f3d · 9080f3d
1 parent 3bba816
commit 9080f3d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 10 deletions.
diff --git a/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx b/src/components/views/settings/tabs/user/SidebarUserSettingsTab.tsx
@@ -34,9 +34,9 @@ import SettingsSubsection, { SettingsSubsectionText } from "../../shared/Setting
 type InteractionName = "WebSettingsSidebarTabSpacesCheckbox" | "WebQuickSettingsPinToSidebarCheckbox";
 
 export const onMetaSpaceChangeFactory =
-    (metaSpace: MetaSpace, interactionName: InteractionName) => (e: ChangeEvent<HTMLInputElement>) => {
+    (metaSpace: MetaSpace, interactionName: InteractionName) => async (e: ChangeEvent<HTMLInputElement>) => {
         const currentValue = SettingsStore.getValue("Spaces.enabledMetaSpaces");
-        SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.ACCOUNT, {
+        await SettingsStore.setValue("Spaces.enabledMetaSpaces", null, SettingLevel.ACCOUNT, {
             ...currentValue,
             [metaSpace]: e.target.checked,
         });

diff --git a/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx b/src/components/views/settings/tabs/user/VoiceUserSettingsTab.tsx
@@ -16,6 +16,7 @@ limitations under the License.
 */
 
 import React, { ReactNode } from "react";
+import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t } from "../../../../../languageHandler";
 import MediaDeviceHandler, { IMediaDevices, MediaDeviceKindEnum } from "../../../../../MediaDeviceHandler";
@@ -40,6 +41,21 @@ interface IState {
     audioNoiseSuppression: boolean;
 }
 
+/**
+ * Maps deviceKind to the right get method on MediaDeviceHandler
+ * Helpful for setting state
+ */
+const mapDeviceKindToHandlerValue = (deviceKind: MediaDeviceKindEnum): string | null => {
+    switch (deviceKind) {
+        case MediaDeviceKindEnum.AudioOutput:
+            return MediaDeviceHandler.getAudioOutput();
+        case MediaDeviceKindEnum.AudioInput:
+            return MediaDeviceHandler.getAudioInput();
+        case MediaDeviceKindEnum.VideoInput:
+            return MediaDeviceHandler.getVideoInput();
+    }
+};
+
 export default class VoiceUserSettingsTab extends React.Component<{}, IState> {
     public constructor(props: {}) {
         super(props);
@@ -58,16 +74,16 @@ export default class VoiceUserSettingsTab extends React.Component<{}, IState> {
     public async componentDidMount(): Promise<void> {
         const canSeeDeviceLabels = await MediaDeviceHandler.hasAnyLabeledDevices();
         if (canSeeDeviceLabels) {
-            this.refreshMediaDevices();
+            await this.refreshMediaDevices();
         }
     }
 
     private refreshMediaDevices = async (stream?: MediaStream): Promise<void> => {
         this.setState({
             mediaDevices: (await MediaDeviceHandler.getDevices()) ?? null,
-            [MediaDeviceKindEnum.AudioOutput]: MediaDeviceHandler.getAudioOutput(),
-            [MediaDeviceKindEnum.AudioInput]: MediaDeviceHandler.getAudioInput(),
-            [MediaDeviceKindEnum.VideoInput]: MediaDeviceHandler.getVideoInput(),
+            [MediaDeviceKindEnum.AudioOutput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.AudioOutput),
+            [MediaDeviceKindEnum.AudioInput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.AudioInput),
+            [MediaDeviceKindEnum.VideoInput]: mapDeviceKindToHandlerValue(MediaDeviceKindEnum.VideoInput),
         });
         if (stream) {
             // kill stream (after we've enumerated the devices, otherwise we'd get empty labels again)
@@ -80,13 +96,20 @@ export default class VoiceUserSettingsTab extends React.Component<{}, IState> {
     private requestMediaPermissions = async (): Promise<void> => {
         const stream = await requestMediaPermissions();
         if (stream) {
-            this.refreshMediaDevices(stream);
+            await this.refreshMediaDevices(stream);
         }
     };
 
-    private setDevice = (deviceId: string, kind: MediaDeviceKindEnum): void => {
-        MediaDeviceHandler.instance.setDevice(deviceId, kind);
+    private setDevice = async (deviceId: string, kind: MediaDeviceKindEnum): Promise<void> => {
+        // set state immediately so UI is responsive
         this.setState<any>({ [kind]: deviceId });
+        try {
+            await MediaDeviceHandler.instance.setDevice(deviceId, kind);
+        } catch (error) {
+            logger.error(`Failed to set device ${kind}: ${deviceId}`);
+            // reset state to current value
+            this.setState<any>({ [kind]: mapDeviceKindToHandlerValue(kind) });
+        }
     };
 
     private changeWebRtcMethod = (p2p: boolean): void => {

diff --git a/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/VoiceUserSettingsTab-test.tsx
@@ -17,6 +17,7 @@ limitations under the License.
 import React from "react";
 import { mocked } from "jest-mock";
 import { fireEvent, render, screen } from "@testing-library/react";
+import { logger } from "matrix-js-sdk/src/logger";
 
 import VoiceUserSettingsTab from "../../../../../../src/components/views/settings/tabs/user/VoiceUserSettingsTab";
 import MediaDeviceHandler, { IMediaDevices, MediaDeviceKindEnum } from "../../../../../../src/MediaDeviceHandler";
@@ -56,9 +57,10 @@ describe("<VoiceUserSettingsTab />", () => {
         jest.clearAllMocks();
         MediaDeviceHandlerMock.hasAnyLabeledDevices.mockResolvedValue(true);
         MediaDeviceHandlerMock.getDevices.mockResolvedValue(defaultMediaDevices);
+        MediaDeviceHandlerMock.getVideoInput.mockReturnValue(videoIn1.deviceId);
 
         // @ts-ignore bad mocking
-        MediaDeviceHandlerMock.instance = { setDevice: jest.fn() };
+        MediaDeviceHandlerMock.instance = { setDevice: jest.fn().mockResolvedValue(undefined) };
     });
 
     describe("devices", () => {
@@ -84,6 +86,29 @@ describe("<VoiceUserSettingsTab />", () => {
             expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn2.label);
         });
 
+        it("logs and resets device when update fails", async () => {
+            // stub to avoid littering console with expected error
+            jest.spyOn(logger, "error").mockImplementation(() => {});
+            MediaDeviceHandlerMock.instance.setDevice.mockRejectedValue("oups!");
+            render(getComponent());
+            await flushPromises();
+
+            fireEvent.change(screen.getByLabelText("Camera"), { target: { value: videoIn2.deviceId } });
+
+            expect(MediaDeviceHandlerMock.instance.setDevice).toHaveBeenCalledWith(
+                videoIn2.deviceId,
+                MediaDeviceKindEnum.VideoInput,
+            );
+
+            expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn2.label);
+
+            await flushPromises();
+
+            expect(logger.error).toHaveBeenCalledWith("Failed to set device videoinput: 3");
+            // reset to original
+            expect(screen.getByLabelText("Camera")).toHaveDisplayValue(videoIn1.label);
+        });
+
         it("does not render dropdown when no devices exist for type", async () => {
             render(getComponent());
             await flushPromises();