Bail out of RoomSettingsDialog when room is not found (#10662)

* hack to fix console noise from unfaked timers and clearAllModals

* remove old debug logging in AsyncWrapper

* pass room to room settings tabs

* add errorboundary for roomsettingsdialog

* apply strictnullchecks to tabs/room

* dedupe code to set toom in roomsettingdialog

* add unit tests

* test SecurityRoomSettingsTab

* remove snapshot

* strict fixes

* more tests

* 2% more test coverage

* remove roomName from RoomSettingsDialogs state

src/components/views/dialogs/RoomSettingsDialog.tsx

@@ -1,6 +1,8 @@
 /*
 Copyright 2019 New Vector Ltd
 Copyright 2019 Michael Telatynski <7t3chguy@gmail.com>
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -16,7 +18,7 @@ limitations under the License.
 */
 
 import React from "react";
-import { RoomEvent } from "matrix-js-sdk/src/models/room";
+import { RoomEvent, Room } from "matrix-js-sdk/src/models/room";
 
 import TabbedView, { Tab } from "../../structures/TabbedView";
 import { _t, _td } from "../../../languageHandler";
@@ -36,6 +38,7 @@ import { VoipRoomSettingsTab } from "../settings/tabs/room/VoipRoomSettingsTab";
 import { ActionPayload } from "../../../dispatcher/payloads";
 import { NonEmptyArray } from "../../../@types/common";
 import { PollHistoryTab } from "../settings/tabs/room/PollHistoryTab";
+import ErrorBoundary from "../elements/ErrorBoundary";
 
 export const ROOM_GENERAL_TAB = "ROOM_GENERAL_TAB";
 export const ROOM_VOIP_TAB = "ROOM_VOIP_TAB";
@@ -53,15 +56,17 @@ interface IProps {
 }
 
 interface IState {
-    roomName: string;
+    room: Room;
 }
 
-export default class RoomSettingsDialog extends React.Component<IProps, IState> {
+class RoomSettingsDialog extends React.Component<IProps, IState> {
     private dispatcherRef: string;
 
     public constructor(props: IProps) {
         super(props);
-        this.state = { roomName: "" };
+
+        const room = this.getRoom();
+        this.state = { room };
     }
 
     public componentDidMount(): void {
@@ -70,6 +75,13 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
         this.onRoomName();
     }
 
+    public componentDidUpdate(): void {
+        if (this.props.roomId !== this.state.room.roomId) {
+            const room = this.getRoom();
+            this.setState({ room });
+        }
+    }
+
     public componentWillUnmount(): void {
         if (this.dispatcherRef) {
             dis.unregister(this.dispatcherRef);
@@ -78,6 +90,21 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
         MatrixClientPeg.get().removeListener(RoomEvent.Name, this.onRoomName);
     }
 
+    /**
+     * Get room from client
+     * @returns Room
+     * @throws when room is not found
+     */
+    private getRoom(): Room {
+        const room = MatrixClientPeg.get().getRoom(this.props.roomId)!;
+
+        // something is really wrong if we encounter this
+        if (!room) {
+            throw new Error(`Cannot find room ${this.props.roomId}`);
+        }
+        return room;
+    }
+
     private onAction = (payload: ActionPayload): void => {
         // When view changes below us, close the room settings
         // whilst the modal is open this can only be triggered when someone hits Leave Room
@@ -87,9 +114,8 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
     };
 
     private onRoomName = (): void => {
-        this.setState({
-            roomName: MatrixClientPeg.get().getRoom(this.props.roomId)?.name ?? "",
-        });
+        // rerender when the room name changes
+        this.forceUpdate();
     };
 
     private getTabs(): NonEmptyArray<Tab> {
@@ -100,7 +126,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                 ROOM_GENERAL_TAB,
                 _td("General"),
                 "mx_RoomSettingsDialog_settingsIcon",
-                <GeneralRoomSettingsTab roomId={this.props.roomId} />,
+                <GeneralRoomSettingsTab room={this.state.room} />,
                 "RoomSettingsGeneral",
             ),
         );
@@ -110,7 +136,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                     ROOM_VOIP_TAB,
                     _td("Voice & Video"),
                     "mx_RoomSettingsDialog_voiceIcon",
-                    <VoipRoomSettingsTab roomId={this.props.roomId} />,
+                    <VoipRoomSettingsTab room={this.state.room} />,
                 ),
             );
         }
@@ -119,12 +145,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                 ROOM_SECURITY_TAB,
                 _td("Security & Privacy"),
                 "mx_RoomSettingsDialog_securityIcon",
-                (
-                    <SecurityRoomSettingsTab
-                        roomId={this.props.roomId}
-                        closeSettingsFn={() => this.props.onFinished(true)}
-                    />
-                ),
+                <SecurityRoomSettingsTab room={this.state.room} closeSettingsFn={() => this.props.onFinished(true)} />,
                 "RoomSettingsSecurityPrivacy",
             ),
         );
@@ -133,7 +154,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                 ROOM_ROLES_TAB,
                 _td("Roles & Permissions"),
                 "mx_RoomSettingsDialog_rolesIcon",
-                <RolesRoomSettingsTab roomId={this.props.roomId} />,
+                <RolesRoomSettingsTab room={this.state.room} />,
                 "RoomSettingsRolesPermissions",
             ),
         );
@@ -144,7 +165,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                 "mx_RoomSettingsDialog_notificationsIcon",
                 (
                     <NotificationSettingsTab
-                        roomId={this.props.roomId}
+                        roomId={this.state.room.roomId}
                         closeSettingsFn={() => this.props.onFinished(true)}
                     />
                 ),
@@ -158,7 +179,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                     ROOM_BRIDGES_TAB,
                     _td("Bridges"),
                     "mx_RoomSettingsDialog_bridgesIcon",
-                    <BridgeSettingsTab roomId={this.props.roomId} />,
+                    <BridgeSettingsTab room={this.state.room} />,
                     "RoomSettingsBridges",
                 ),
             );
@@ -169,7 +190,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                 ROOM_POLL_HISTORY_TAB,
                 _td("Poll history"),
                 "mx_RoomSettingsDialog_pollsIcon",
-                <PollHistoryTab roomId={this.props.roomId} onFinished={() => this.props.onFinished(true)} />,
+                <PollHistoryTab room={this.state.room} onFinished={() => this.props.onFinished(true)} />,
             ),
         );
 
@@ -181,7 +202,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
                     "mx_RoomSettingsDialog_warningIcon",
                     (
                         <AdvancedRoomSettingsTab
-                            roomId={this.props.roomId}
+                            room={this.state.room}
                             closeSettingsFn={() => this.props.onFinished(true)}
                         />
                     ),
@@ -194,7 +215,7 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
     }
 
     public render(): React.ReactNode {
-        const roomName = this.state.roomName;
+        const roomName = this.state.room.name;
         return (
             <BaseDialog
                 className="mx_RoomSettingsDialog"
@@ -213,3 +234,11 @@ export default class RoomSettingsDialog extends React.Component<IProps, IState>
         );
     }
 }
+
+const WrappedRoomSettingsDialog: React.FC<IProps> = (props) => (
+    <ErrorBoundary>
+        <RoomSettingsDialog {...props} />
+    </ErrorBoundary>
+);
+
+export default WrappedRoomSettingsDialog;

src/components/views/dialogs/SpaceSettingsDialog.tsx

@@ -70,14 +70,14 @@ const SpaceSettingsDialog: React.FC<IProps> = ({ matrixClient: cli, space, onFin
                 SpaceSettingsTab.Roles,
                 _td("Roles & Permissions"),
                 "mx_RoomSettingsDialog_rolesIcon",
-                <RolesRoomSettingsTab roomId={space.roomId} />,
+                <RolesRoomSettingsTab room={space} />,
             ),
             SettingsStore.getValue(UIFeature.AdvancedSettings)
                 ? new Tab(
                       SpaceSettingsTab.Advanced,
                       _td("Advanced"),
                       "mx_RoomSettingsDialog_warningIcon",
-                      <AdvancedRoomSettingsTab roomId={space.roomId} closeSettingsFn={onFinished} />,
+                      <AdvancedRoomSettingsTab room={space} closeSettingsFn={onFinished} />,
                   )
                 : null,
         ].filter(Boolean) as NonEmptyArray<Tab>;

src/components/views/settings/tabs/room/AdvancedRoomSettingsTab.tsx

@@ -16,9 +16,9 @@ limitations under the License.
 
 import React from "react";
 import { EventType } from "matrix-js-sdk/src/@types/event";
+import { Room } from "matrix-js-sdk/src/matrix";
 
 import { _t } from "../../../../../languageHandler";
-import { MatrixClientPeg } from "../../../../../MatrixClientPeg";
 import AccessibleButton, { ButtonEvent } from "../../../elements/AccessibleButton";
 import RoomUpgradeDialog from "../../../dialogs/RoomUpgradeDialog";
 import Modal from "../../../../../Modal";
@@ -29,7 +29,7 @@ import { ViewRoomPayload } from "../../../../../dispatcher/payloads/ViewRoomPayl
 import SettingsStore from "../../../../../settings/SettingsStore";
 
 interface IProps {
-    roomId: string;
+    room: Room;
     closeSettingsFn(): void;
 }
 
@@ -64,8 +64,8 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
         this.state = {};
 
         // we handle lack of this object gracefully later, so don't worry about it failing here.
-        const room = MatrixClientPeg.get().getRoom(this.props.roomId);
-        room?.getRecommendedVersion().then((v) => {
+        const room = this.props.room;
+        room.getRecommendedVersion().then((v) => {
             const tombstone = room.currentState.getStateEvents(EventType.RoomTombstone, "");
 
             const additionalStateChanges: Partial<IState> = {};
@@ -85,8 +85,7 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
     }
 
     private upgradeRoom = (): void => {
-        const room = MatrixClientPeg.get().getRoom(this.props.roomId);
-        if (room) Modal.createDialog(RoomUpgradeDialog, { room });
+        Modal.createDialog(RoomUpgradeDialog, { room: this.props.room });
     };
 
     private onOldRoomClicked = (e: ButtonEvent): void => {
@@ -105,12 +104,11 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
     };
 
     public render(): React.ReactNode {
-        const client = MatrixClientPeg.get();
-        const room = client.getRoom(this.props.roomId);
-        const isSpace = room?.isSpaceRoom();
+        const room = this.props.room;
+        const isSpace = room.isSpaceRoom();
 
         let unfederatableSection: JSX.Element | undefined;
-        if (room?.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()["m.federate"] === false) {
+        if (room.currentState.getStateEvents(EventType.RoomCreate, "")?.getContent()["m.federate"] === false) {
             unfederatableSection = <div>{_t("This room is not accessible by remote Matrix servers")}</div>;
         }
 
@@ -143,9 +141,9 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
         if (this.state.oldRoomId) {
             let copy: string;
             if (isSpace) {
-                copy = _t("View older version of %(spaceName)s.", { spaceName: room?.name ?? this.state.oldRoomId });
+                copy = _t("View older version of %(spaceName)s.", { spaceName: room.name ?? this.state.oldRoomId });
             } else {
-                copy = _t("View older messages in %(roomName)s.", { roomName: room?.name ?? this.state.oldRoomId });
+                copy = _t("View older messages in %(roomName)s.", { roomName: room.name ?? this.state.oldRoomId });
             }
 
             oldRoomLink = (
@@ -160,19 +158,21 @@ export default class AdvancedRoomSettingsTab extends React.Component<IProps, ISt
                 <div className="mx_SettingsTab_heading">{_t("Advanced")}</div>
                 <div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
                     <span className="mx_SettingsTab_subheading">
-                        {room?.isSpaceRoom() ? _t("Space information") : _t("Room information")}
+                        {room.isSpaceRoom() ? _t("Space information") : _t("Room information")}
                     </span>
                     <div>
                         <span>{_t("Internal room ID")}</span>
-                        <CopyableText getTextToCopy={() => this.props.roomId}>{this.props.roomId}</CopyableText>
+                        <CopyableText getTextToCopy={() => this.props.room.roomId}>
+                            {this.props.room.roomId}
+                        </CopyableText>
                     </div>
                     {unfederatableSection}
                 </div>
                 <div className="mx_SettingsTab_section mx_SettingsTab_subsectionText">
                     <span className="mx_SettingsTab_subheading">{_t("Room version")}</span>
                     <div>
                         <span>{_t("Room version:")}</span>&nbsp;
-                        {room?.getVersion()}
+                        {room.getVersion()}
                     </div>
                     {oldRoomLink}
                     {roomUpgradeButton}

src/components/views/settings/tabs/room/BridgeSettingsTab.tsx

@@ -30,7 +30,7 @@ const BRIDGE_EVENT_TYPES = [
 const BRIDGES_LINK = "https://matrix.org/bridges/";
 
 interface IProps {
-    roomId: string;
+    room: Room;
 }
 
 export default class BridgeSettingsTab extends React.Component<IProps> {
@@ -51,9 +51,8 @@ export default class BridgeSettingsTab extends React.Component<IProps> {
     public render(): React.ReactNode {
         // This settings tab will only be invoked if the following function returns more
         // than 0 events, so no validation is needed at this stage.
-        const bridgeEvents = BridgeSettingsTab.getBridgeStateEvents(this.props.roomId);
-        const client = MatrixClientPeg.get();
-        const room = client.getRoom(this.props.roomId);
+        const bridgeEvents = BridgeSettingsTab.getBridgeStateEvents(this.props.room.roomId);
+        const room = this.props.room;
 
         let content: JSX.Element;
         if (bridgeEvents.length > 0) {

src/components/views/settings/tabs/room/GeneralRoomSettingsTab.tsx

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import React, { ContextType } from "react";
+import { Room } from "matrix-js-sdk/src/matrix";
 
 import { _t } from "../../../../../languageHandler";
 import RoomProfileSettings from "../../../room_settings/RoomProfileSettings";
@@ -28,7 +29,7 @@ import AliasSettings from "../../../room_settings/AliasSettings";
 import PosthogTrackers from "../../../../../PosthogTrackers";
 
 interface IProps {
-    roomId: string;
+    room: Room;
 }
 
 interface IState {
@@ -50,25 +51,26 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
     private onLeaveClick = (ev: ButtonEvent): void => {
         dis.dispatch({
             action: "leave_room",
-            room_id: this.props.roomId,
+            room_id: this.props.room.roomId,
         });
 
         PosthogTrackers.trackInteraction("WebRoomSettingsLeaveButton", ev);
     };
 
     public render(): React.ReactNode {
         const client = this.context;
-        const room = client.getRoom(this.props.roomId);
+        const room = this.props.room;
 
         const canSetAliases = true; // Previously, we arbitrarily only allowed admins to do this
-        const canSetCanonical = room?.currentState.mayClientSendStateEvent("m.room.canonical_alias", client);
-        const canonicalAliasEv = room?.currentState.getStateEvents("m.room.canonical_alias", "") ?? undefined;
+        const canSetCanonical = room.currentState.mayClientSendStateEvent("m.room.canonical_alias", client);
+        const canonicalAliasEv = room.currentState.getStateEvents("m.room.canonical_alias", "") ?? undefined;
 
-        const urlPreviewSettings =
-            room && SettingsStore.getValue(UIFeature.URLPreviews) ? <UrlPreviewSettings room={room} /> : null;
+        const urlPreviewSettings = SettingsStore.getValue(UIFeature.URLPreviews) ? (
+            <UrlPreviewSettings room={room} />
+        ) : null;
 
         let leaveSection;
-        if (room?.getMyMembership() === "join") {
+        if (room.getMyMembership() === "join") {
             leaveSection = (
                 <>
                     <span className="mx_SettingsTab_subheading">{_t("Leave room")}</span>
@@ -85,12 +87,12 @@ export default class GeneralRoomSettingsTab extends React.Component<IProps, ISta
             <div className="mx_SettingsTab mx_GeneralRoomSettingsTab">
                 <div className="mx_SettingsTab_heading">{_t("General")}</div>
                 <div className="mx_SettingsTab_section mx_GeneralRoomSettingsTab_profileSection">
-                    <RoomProfileSettings roomId={this.props.roomId} />
+                    <RoomProfileSettings roomId={room.roomId} />
                 </div>
 
                 <div className="mx_SettingsTab_heading">{_t("Room Addresses")}</div>
                 <AliasSettings
-                    roomId={this.props.roomId}
+                    roomId={room.roomId}
                     canSetCanonicalAlias={canSetCanonical}
                     canSetAliases={canSetAliases}
                     canonicalAliasEvent={canonicalAliasEv}

src/components/views/settings/tabs/room/NotificationSettingsTab.tsx

@@ -53,7 +53,7 @@ export default class NotificationsSettingsTab extends React.Component<IProps, IS
     public constructor(props: IProps, context: React.ContextType<typeof MatrixClientContext>) {
         super(props, context);
 
-        this.roomProps = EchoChamber.forRoom(context.getRoom(this.props.roomId));
+        this.roomProps = EchoChamber.forRoom(context.getRoom(this.props.roomId)!);
 
         let currentSound = "default";
         const soundData = Notifier.getSoundForRoom(this.props.roomId);