From 89fb388e5711d9f446bf641eb011d39a80c866a8 Mon Sep 17 00:00:00 2001 From: Will Hunt <2072976+Half-Shot@users.noreply.github.com> Date: Fri, 27 Feb 2026 11:01:51 +0000 Subject: [PATCH] Fix invite-specific join errors not being shown (#32621) * Fix invite-specific join errors not being shown * remove horrible non-jest function of getAccountData * Use an Error * Always dispatch an Error * Throw a UserFriendlyError --- .../payloads/JoinRoomErrorPayload.ts | 11 ++---- apps/web/src/i18n/strings/en_EN.json | 1 + apps/web/src/stores/RoomViewStore.tsx | 25 ++++++------ apps/web/test/test-utils/test-utils.ts | 2 +- .../unit-tests/stores/RoomViewStore-test.ts | 39 ++++++++++++++++--- .../__snapshots__/RoomViewStore-test.ts.snap | 14 +++++++ 6 files changed, 67 insertions(+), 25 deletions(-) diff --git a/apps/web/src/dispatcher/payloads/JoinRoomErrorPayload.ts b/apps/web/src/dispatcher/payloads/JoinRoomErrorPayload.ts index 283a3c6915..6beb722a4f 100644 --- a/apps/web/src/dispatcher/payloads/JoinRoomErrorPayload.ts +++ b/apps/web/src/dispatcher/payloads/JoinRoomErrorPayload.ts @@ -6,16 +6,13 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { type MatrixError } from "matrix-js-sdk/src/matrix"; - -import { type ActionPayload } from "../payloads"; -import { type Action } from "../actions"; +import type { ActionPayload } from "../payloads"; +import type { Action } from "../actions"; export interface JoinRoomErrorPayload extends Pick { action: Action.JoinRoomError; - roomId: string; - err: MatrixError; - + roomId: string | null; + err: Error; canAskToJoin?: boolean; } diff --git a/apps/web/src/i18n/strings/en_EN.json b/apps/web/src/i18n/strings/en_EN.json index f31931b7b0..6350993c56 100644 --- a/apps/web/src/i18n/strings/en_EN.json +++ b/apps/web/src/i18n/strings/en_EN.json @@ -1990,6 +1990,7 @@ "error_join_incompatible_version_1": "Sorry, your homeserver is too old to participate here.", "error_join_incompatible_version_2": "Please contact your homeserver administrator.", "error_join_title": "Failed to join", + "error_join_unknown": "An unknonw error occured.", "error_jump_to_date": "Server returned %(statusCode)s with error code %(errorCode)s", "error_jump_to_date_connection": "A network error occurred while trying to find and jump to the given date. Your homeserver might be down or there was just a temporary problem with your internet connection. Please try again. If this continues, please contact your homeserver administrator.", "error_jump_to_date_details": "Error details", diff --git a/apps/web/src/stores/RoomViewStore.tsx b/apps/web/src/stores/RoomViewStore.tsx index 743f6c4f74..7d06391e32 100644 --- a/apps/web/src/stores/RoomViewStore.tsx +++ b/apps/web/src/stores/RoomViewStore.tsx @@ -24,7 +24,7 @@ import { import { type MatrixDispatcher } from "../dispatcher/dispatcher"; import { MatrixClientPeg } from "../MatrixClientPeg"; import Modal from "../Modal"; -import { _t } from "../languageHandler"; +import { _t, UserFriendlyError } from "../languageHandler"; import { getCachedRoomIdForAlias, storeRoomAliasInCache } from "../RoomAliasCache"; import { Action } from "../dispatcher/actions"; import { retry } from "../utils/promise"; @@ -569,10 +569,11 @@ export class RoomViewStore extends EventEmitter { metricsTrigger: payload.metricsTrigger, }); } catch (err) { - this.dis?.dispatch({ + logger.error("Error thrown while handling joinRoom", err); + this.dis?.dispatch({ action: Action.JoinRoomError, roomId, - err, + err: err instanceof Error ? err : new UserFriendlyError("room|error_join_unknown", { cause: err }), canAskToJoin: payload.canAskToJoin, }); @@ -592,11 +593,11 @@ export class RoomViewStore extends EventEmitter { } } - public showJoinRoomError(err: MatrixError, roomId: string): void { - let description: ReactNode = err.message ? err.message : JSON.stringify(err); - logger.log("Failed to join room:", description); - - if (err.name === "ConnectionError") { + public showJoinRoomError(err: unknown, roomId: string | null): void { + let description: ReactNode = err instanceof Error && err.message ? err.message : JSON.stringify(err); + if (err instanceof MatrixError === false) { + // This isn't a MatrixError so just show the error verbatim. + } else if (err.name === "ConnectionError") { description = _t("room|error_join_connection"); } else if (err.errcode === "M_INCOMPATIBLE_ROOM_VERSION") { description = ( @@ -607,7 +608,7 @@ export class RoomViewStore extends EventEmitter { ); } else if (err.httpStatus === 404) { - const invitingUserId = this.getInvitingUserId(roomId); + const invitingUserId = roomId && this.getInvitingUserId(roomId); // provide a better error message for invites if (invitingUserId) { // if the inviting user is on the same HS, there can only be one cause: they left. @@ -617,10 +618,9 @@ export class RoomViewStore extends EventEmitter { description = _t("room|error_join_404_invite"); } } - // provide a more detailed error than "No known servers" when attempting to // join using a room ID and no via servers - if (roomId === this.state.roomId && this.state.viaServers.length === 0) { + else if (roomId === this.state.roomId && this.state.viaServers.length === 0) { description = (
{_t("room|error_join_404_1")} @@ -631,6 +631,7 @@ export class RoomViewStore extends EventEmitter { ); } } + logger.log("Failed to join room:", description); Modal.createDialog(ErrorDialog, { title: _t("room|error_join_title"), @@ -641,7 +642,7 @@ export class RoomViewStore extends EventEmitter { private joinRoomError(payload: JoinRoomErrorPayload): void { this.setState({ joining: false, - joinError: payload.err, + joinError: payload.err instanceof Error ? payload.err : null, }); if (payload.err && !payload.canAskToJoin) { this.showJoinRoomError(payload.err, payload.roomId); diff --git a/apps/web/test/test-utils/test-utils.ts b/apps/web/test/test-utils/test-utils.ts index f15c21130b..09d37e50c3 100644 --- a/apps/web/test/test-utils/test-utils.ts +++ b/apps/web/test/test-utils/test-utils.ts @@ -688,7 +688,6 @@ export function mkStubRoom( fetchRoomThreads: jest.fn().mockReturnValue(Promise.resolve()), findEventById: jest.fn().mockReturnValue(undefined), findPredecessor: jest.fn().mockReturnValue({ roomId: "", eventId: null }), - getAccountData: (_: EventType | string) => undefined as MatrixEvent | undefined, getAltAliases: jest.fn().mockReturnValue([]), getAvatarUrl: () => "mxc://avatar.url/room.png", getCanonicalAlias: jest.fn(), @@ -725,6 +724,7 @@ export function mkStubRoom( getRoomUnreadNotificationCount: jest.fn().mockReturnValue(0), getVersion: jest.fn().mockReturnValue("1"), getBumpStamp: jest.fn().mockReturnValue(0), + getAccountData: jest.fn(), hasMembershipState: () => false, isElementVideoRoom: jest.fn().mockReturnValue(false), isSpaceRoom: jest.fn().mockReturnValue(false), diff --git a/apps/web/test/unit-tests/stores/RoomViewStore-test.ts b/apps/web/test/unit-tests/stores/RoomViewStore-test.ts index b2130fcb2c..69e806bb04 100644 --- a/apps/web/test/unit-tests/stores/RoomViewStore-test.ts +++ b/apps/web/test/unit-tests/stores/RoomViewStore-test.ts @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import { mocked } from "jest-mock"; -import { MatrixError, Room } from "matrix-js-sdk/src/matrix"; +import { KnownMembership, MatrixError, Room } from "matrix-js-sdk/src/matrix"; import { sleep } from "matrix-js-sdk/src/utils"; import { RoomViewLifecycle, @@ -20,6 +20,9 @@ import { Action } from "../../../src/dispatcher/actions"; import { flushPromises, getMockClientWithEventEmitter, + mkEvent, + mkRoom, + mkRoomMember, setupAsyncStoreWithClient, untilDispatch, untilEmission, @@ -126,6 +129,7 @@ describe("RoomViewStore", function () { getUserId: jest.fn().mockReturnValue(userId), getSafeUserId: jest.fn().mockReturnValue(userId), getDeviceId: jest.fn().mockReturnValue("ABC123"), + getDomain: jest.fn().mockReturnValue("server"), sendStateEvent: jest.fn().mockResolvedValue({}), supportsThreads: jest.fn(), isInitialSyncComplete: jest.fn().mockResolvedValue(false), @@ -144,7 +148,7 @@ describe("RoomViewStore", function () { } })(), }); - const room = new Room(roomId, mockClient, userId); + const room = mkRoom(mockClient, roomId); const room2 = new Room(roomId2, mockClient, userId); getRooms.mockReturnValue([room, room2]); @@ -439,6 +443,31 @@ describe("RoomViewStore", function () { expect(mocked(Modal).createDialog.mock.calls[0][1]).toMatchSnapshot(); }); + // The server bob is on will affect the message we send. + it.each(["server", "another-server"])( + "should display an invite-specific error message when the room is unreachable", + async (bobsServer) => { + room.getMyMembership.mockReturnValue(KnownMembership.Invite); + room.getMember.mockImplementationOnce((memberUserId) => { + if (userId === memberUserId) { + const member = mkRoomMember(roomId, userId, KnownMembership.Invite); + member.events.member!.getSender = () => `@bob:${bobsServer}`; + return member; + } + return null; + }); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); + + // Generate error to display the expected error message + const error = new MatrixError(undefined, 404); + roomViewStore.showJoinRoomError(error, roomId); + + // Check the modal props + expect(mocked(Modal).createDialog.mock.calls[0][1]).toMatchSnapshot(); + }, + ); + it("should display the generic error message when the roomId doesnt match", async () => { // When // Generate error to display the expected error message @@ -450,9 +479,9 @@ describe("RoomViewStore", function () { }); it("clears the unread flag when viewing a room", async () => { - room.getAccountData = jest.fn().mockReturnValue({ - getContent: jest.fn().mockReturnValue({ unread: true }), - }); + room.getAccountData.mockReturnValue( + mkEvent({ type: "m.marked_unread", user: "@anyone:example.org", content: { unread: true }, event: true }), + ); dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); await untilDispatch(Action.ActiveRoomChanged, dis); expect(mockClient.setRoomAccountData).toHaveBeenCalledWith(roomId, "m.marked_unread", { diff --git a/apps/web/test/unit-tests/stores/__snapshots__/RoomViewStore-test.ts.snap b/apps/web/test/unit-tests/stores/__snapshots__/RoomViewStore-test.ts.snap index 3379087e76..b00ffdbfe2 100644 --- a/apps/web/test/unit-tests/stores/__snapshots__/RoomViewStore-test.ts.snap +++ b/apps/web/test/unit-tests/stores/__snapshots__/RoomViewStore-test.ts.snap @@ -12,6 +12,20 @@ exports[`RoomViewStore should display an error message when the room is unreacha } `; +exports[`RoomViewStore should display an invite-specific error message when the room is unreachable 1`] = ` +{ + "description": "The person who invited you has already left.", + "title": "Failed to join", +} +`; + +exports[`RoomViewStore should display an invite-specific error message when the room is unreachable 2`] = ` +{ + "description": "The person who invited you has already left, or their server is offline.", + "title": "Failed to join", +} +`; + exports[`RoomViewStore should display the generic error message when the roomId doesnt match 1`] = ` { "description": "MatrixError: [404] my 404 error",