From e0b8343088153642cd5a383666ce1ca57dac750d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Aug 2020 19:21:40 -0600 Subject: [PATCH 1/4] Run all room leaving behaviour through a single function Fixes https://github.com/vector-im/element-web/issues/14999 Fixes https://github.com/vector-im/element-web/issues/10380 We were failing to handle errors when `/part`ing a room, though the leave room button was fine. This runs both the button and command through the same function for handling, including the 'view next room' behaviour. --- src/SlashCommands.tsx | 8 +-- src/components/structures/MatrixChat.tsx | 42 ++------------- src/i18n/strings/en_EN.json | 9 ++-- src/utils/membership.ts | 68 ++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/SlashCommands.tsx b/src/SlashCommands.tsx index 50a49ccf1c..d674634109 100644 --- a/src/SlashCommands.tsx +++ b/src/SlashCommands.tsx @@ -43,7 +43,7 @@ import SdkConfig from "./SdkConfig"; import { ensureDMExists } from "./createRoom"; import { ViewUserPayload } from "./dispatcher/payloads/ViewUserPayload"; import { Action } from "./dispatcher/actions"; -import { EffectiveMembership, getEffectiveMembership } from "./utils/membership"; +import { EffectiveMembership, getEffectiveMembership, leaveRoomBehaviour } from "./utils/membership"; // XXX: workaround for https://github.com/microsoft/TypeScript/issues/31816 interface HTMLInputEvent extends Event { @@ -601,11 +601,7 @@ export const Commands = [ } if (!targetRoomId) targetRoomId = roomId; - return success( - cli.leaveRoomChain(targetRoomId).then(function() { - dis.dispatch({action: 'view_next_room'}); - }), - ); + return success(leaveRoomBehaviour(targetRoomId)); }, category: CommandCategories.actions, }), diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index ce96847d28..460148045c 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -76,6 +76,7 @@ import { OpenToTabPayload } from "../../dispatcher/payloads/OpenToTabPayload"; import ErrorDialog from "../views/dialogs/ErrorDialog"; import { RoomNotificationStateStore } from "../../stores/notifications/RoomNotificationStateStore"; import { SettingLevel } from "../../settings/SettingLevel"; +import { leaveRoomBehaviour } from "../../utils/membership"; /** constants for MatrixChat.state.view */ export enum Views { @@ -1082,50 +1083,13 @@ export default class MatrixChat extends React.PureComponent { button: _t("Leave"), onFinished: (shouldLeave) => { if (shouldLeave) { - const d = MatrixClientPeg.get().leaveRoomChain(roomId); + const d = leaveRoomBehaviour(roomId); // FIXME: controller shouldn't be loading a view :( const Loader = sdk.getComponent("elements.Spinner"); const modal = Modal.createDialog(Loader, null, 'mx_Dialog_spinner'); - d.then((errors) => { - modal.close(); - - for (const leftRoomId of Object.keys(errors)) { - const err = errors[leftRoomId]; - if (!err) continue; - - console.error("Failed to leave room " + leftRoomId + " " + err); - let title = _t("Failed to leave room"); - let message = _t("Server may be unavailable, overloaded, or you hit a bug."); - if (err.errcode === 'M_CANNOT_LEAVE_SERVER_NOTICE_ROOM') { - title = _t("Can't leave Server Notices room"); - message = _t( - "This room is used for important messages from the Homeserver, " + - "so you cannot leave it.", - ); - } else if (err && err.message) { - message = err.message; - } - Modal.createTrackedDialog('Failed to leave room', '', ErrorDialog, { - title: title, - description: message, - }); - return; - } - - if (this.state.currentRoomId === roomId) { - dis.dispatch({action: 'view_next_room'}); - } - }, (err) => { - // This should only happen if something went seriously wrong with leaving the chain. - modal.close(); - console.error("Failed to leave room " + roomId + " " + err); - Modal.createTrackedDialog('Failed to leave room', '', ErrorDialog, { - title: _t("Failed to leave room"), - description: _t("Unknown error"), - }); - }); + d.finally(() => modal.close()); } }, }); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 8bfc3ed703..4c0557ac70 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -347,6 +347,10 @@ "Your browser does not support the required cryptography extensions": "Your browser does not support the required cryptography extensions", "Not a valid %(brand)s keyfile": "Not a valid %(brand)s keyfile", "Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?", + "Unexpected server error trying to leave the room": "Unexpected server error trying to leave the room", + "Can't leave Server Notices room": "Can't leave Server Notices room", + "This room is used for important messages from the Homeserver, so you cannot leave it.": "This room is used for important messages from the Homeserver, so you cannot leave it.", + "Error leaving room": "Error leaving room", "Unrecognised address": "Unrecognised address", "You do not have permission to invite people to this room.": "You do not have permission to invite people to this room.", "User %(userId)s is already in the room": "User %(userId)s is already in the room", @@ -2028,10 +2032,6 @@ "Failed to reject invitation": "Failed to reject invitation", "This room is not public. You will not be able to rejoin without an invite.": "This room is not public. You will not be able to rejoin without an invite.", "Are you sure you want to leave the room '%(roomName)s'?": "Are you sure you want to leave the room '%(roomName)s'?", - "Failed to leave room": "Failed to leave room", - "Can't leave Server Notices room": "Can't leave Server Notices room", - "This room is used for important messages from the Homeserver, so you cannot leave it.": "This room is used for important messages from the Homeserver, so you cannot leave it.", - "Unknown error": "Unknown error", "Signed Out": "Signed Out", "For security, this session has been signed out. Please sign in again.": "For security, this session has been signed out. Please sign in again.", "Terms and Conditions": "Terms and Conditions", @@ -2203,6 +2203,7 @@ "User Autocomplete": "User Autocomplete", "Passphrases must match": "Passphrases must match", "Passphrase must not be empty": "Passphrase must not be empty", + "Unknown error": "Unknown error", "Export room keys": "Export room keys", "This process allows you to export the keys for messages you have received in encrypted rooms to a local file. You will then be able to import the file into another Matrix client in the future, so that client will also be able to decrypt these messages.": "This process allows you to export the keys for messages you have received in encrypted rooms to a local file. You will then be able to import the file into another Matrix client in the future, so that client will also be able to decrypt these messages.", "The exported file will allow anyone who can read it to decrypt any encrypted messages that you can see, so you should be careful to keep it secure. To help with this, you should enter a passphrase below, which will be used to encrypt the exported data. It will only be possible to import the data by using the same passphrase.": "The exported file will allow anyone who can read it to decrypt any encrypted messages that you can see, so you should be careful to keep it secure. To help with this, you should enter a passphrase below, which will be used to encrypt the exported data. It will only be possible to import the data by using the same passphrase.", diff --git a/src/utils/membership.ts b/src/utils/membership.ts index 9534623d62..a8abacbeaa 100644 --- a/src/utils/membership.ts +++ b/src/utils/membership.ts @@ -15,6 +15,13 @@ limitations under the License. */ import { Room } from "matrix-js-sdk/src/models/room"; +import { MatrixClientPeg } from "../MatrixClientPeg"; +import { _t } from "../languageHandler"; +import Modal from "../Modal"; +import ErrorDialog from "../components/views/dialogs/ErrorDialog"; +import React from "react"; +import dis from "../dispatcher/dispatcher"; +import RoomViewStore from "../stores/RoomViewStore"; /** * Approximation of a membership status for a given room. @@ -70,3 +77,64 @@ export function getEffectiveMembership(membership: string): EffectiveMembership return EffectiveMembership.Leave; } } + +export async function leaveRoomBehaviour(roomId: string): Promise { + let leavingAllVersions = true; + const history = await MatrixClientPeg.get().getRoomUpgradeHistory(roomId); + if (history && history.length > 0) { + const currentRoom = history[history.length - 1]; + if (currentRoom.roomId !== roomId) { + // The user is trying to leave an older version of the room. Let them through + // without making them leave the current version of the room. + leavingAllVersions = false; + } + } + + let results: { [roomId: string]: Error & { errcode: string, message: string } } = {}; + if (!leavingAllVersions || true) { + try { + await MatrixClientPeg.get().leave(roomId); + } catch (e) { + if (e && e.data && e.data.errcode) { + const message = e.data.error || _t("Unexpected server error trying to leave the room"); + results[roomId] = Object.assign(new Error(message), {errcode: e.data.errcode}); + } else { + results[roomId] = e || new Error("Failed to leave room for unknown causes"); + } + } + } else { + results = await MatrixClientPeg.get().leaveRoomChain(roomId); + } + + const errors = Object.entries(results).filter(r => !!r[1]); + if (errors.length > 0) { + let messages = []; + for (const err of errors) { + let message = _t("Unexpected server error trying to leave the room"); + if (results[roomId].errcode && results[roomId].message) { + if (results[roomId].errcode === 'M_CANNOT_LEAVE_SERVER_NOTICE_ROOM') { + Modal.createTrackedDialog('Error Leaving Room', '', ErrorDialog, { + title: _t("Can't leave Server Notices room"), + description: _t( + "This room is used for important messages from the Homeserver, " + + "so you cannot leave it.", + ), + }); + return false; + } + message = results[roomId].message; + } + messages.push(message, React.createElement('BR')); // createElement to avoid using a tsx file in utils + } + Modal.createTrackedDialog('Error Leaving Room', '', ErrorDialog, { + title: _t("Error leaving room"), + description: messages, + }); + return false; + } + + if (RoomViewStore.getRoomId() === roomId) { + dis.dispatch({action: 'view_next_room'}); + } + return true; +} From 309c32700b32e06ce8c282014b258fe5553a1c08 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Aug 2020 19:23:27 -0600 Subject: [PATCH 2/4] Remove unused success state --- src/utils/membership.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/utils/membership.ts b/src/utils/membership.ts index a8abacbeaa..2693d816a5 100644 --- a/src/utils/membership.ts +++ b/src/utils/membership.ts @@ -78,7 +78,7 @@ export function getEffectiveMembership(membership: string): EffectiveMembership } } -export async function leaveRoomBehaviour(roomId: string): Promise { +export async function leaveRoomBehaviour(roomId: string) { let leavingAllVersions = true; const history = await MatrixClientPeg.get().getRoomUpgradeHistory(roomId); if (history && history.length > 0) { @@ -120,7 +120,7 @@ export async function leaveRoomBehaviour(roomId: string): Promise { "so you cannot leave it.", ), }); - return false; + return; } message = results[roomId].message; } @@ -130,11 +130,10 @@ export async function leaveRoomBehaviour(roomId: string): Promise { title: _t("Error leaving room"), description: messages, }); - return false; + return; } if (RoomViewStore.getRoomId() === roomId) { dis.dispatch({action: 'view_next_room'}); } - return true; } From 8fffce8a303c764ce61a968109e0c5ddb70024d7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Aug 2020 19:42:36 -0600 Subject: [PATCH 3/4] Appease the linter --- src/utils/membership.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/utils/membership.ts b/src/utils/membership.ts index 2693d816a5..14a3ced739 100644 --- a/src/utils/membership.ts +++ b/src/utils/membership.ts @@ -108,11 +108,12 @@ export async function leaveRoomBehaviour(roomId: string) { const errors = Object.entries(results).filter(r => !!r[1]); if (errors.length > 0) { - let messages = []; - for (const err of errors) { + const messages = []; + for (const roomErr of errors) { + const err = roomErr[1]; // [0] is the roomId let message = _t("Unexpected server error trying to leave the room"); - if (results[roomId].errcode && results[roomId].message) { - if (results[roomId].errcode === 'M_CANNOT_LEAVE_SERVER_NOTICE_ROOM') { + if (err.errcode && err.message) { + if (err.errcode === 'M_CANNOT_LEAVE_SERVER_NOTICE_ROOM') { Modal.createTrackedDialog('Error Leaving Room', '', ErrorDialog, { title: _t("Can't leave Server Notices room"), description: _t( From 42988d373bb030d44d707c07ff80ef17ffb8fc93 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 19 Aug 2020 19:42:58 -0600 Subject: [PATCH 4/4] Remove debugging --- src/utils/membership.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/membership.ts b/src/utils/membership.ts index 14a3ced739..68ac958490 100644 --- a/src/utils/membership.ts +++ b/src/utils/membership.ts @@ -91,7 +91,7 @@ export async function leaveRoomBehaviour(roomId: string) { } let results: { [roomId: string]: Error & { errcode: string, message: string } } = {}; - if (!leavingAllVersions || true) { + if (!leavingAllVersions) { try { await MatrixClientPeg.get().leave(roomId); } catch (e) {