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

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<ActionPayload, "action"> {
     action: Action.JoinRoomError;
 
-    roomId: string;
+    roomId: string | null;
-    err: MatrixError;
+    err: Error;
-
     canAskToJoin?: boolean;
 }

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",

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<JoinRoomErrorPayload>({
                 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 {
+    public showJoinRoomError(err: unknown, roomId: string | null): void {
-        let description: ReactNode = err.message ? err.message : JSON.stringify(err);
+        let description: ReactNode = err instanceof Error && err.message ? err.message : JSON.stringify(err);
-        logger.log("Failed to join room:", description);
+        if (err instanceof MatrixError === false) {
-
+            // This isn't a MatrixError so just show the error verbatim.
-        if (err.name === "ConnectionError") {
+        } 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 {
                 </div>
             );
         } 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 = (
                     <div>
                         {_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);

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),

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({
+        room.getAccountData.mockReturnValue(
-            getContent: jest.fn().mockReturnValue({ unread: true }),
+            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", {

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",