Better error handling in jump to date

src/Modal.tsx

@@ -149,13 +149,19 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
         return this.appendDialogAsync<C>(Promise.resolve(Element), props, className);
     }
 
-    public closeCurrentModal(reason: string): void {
+    /**
+     * @callback onBeforeClose
+     * @param reason either "backgroundClick" or undefined
+     * @return whether a modal was closed
+     */
+    public closeCurrentModal(reason?: string): boolean {
         const modal = this.getCurrentModal();
         if (!modal) {
-            return;
+            return false;
         }
         modal.closeReason = reason;
         modal.close();
+        return true;
     }
 
     private buildModal<C extends ComponentType>(
@@ -346,6 +352,8 @@ export class ModalManager extends TypedEventEmitter<ModalManagerEvent, HandlerMa
     }
 
     private async reRender(): Promise<void> {
+        // TODO: We should remove this weird sleep. It also makes testing harder
+        //
         // await next tick because sometimes ReactDOM can race with itself and cause the modal to wrongly stick around
         await sleep(0);
 

src/components/views/messages/DateSeparator.tsx

@@ -20,14 +20,16 @@
 import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t } from "../../../languageHandler";
-import { formatFullDateNoTime } from "../../../DateUtils";
+import { formatFullDateNoDay, formatFullDateNoTime } from "../../../DateUtils";
 import { MatrixClientPeg } from "../../../MatrixClientPeg";
-import dis from "../../../dispatcher/dispatcher";
+import dispatcher from "../../../dispatcher/dispatcher";
 import { Action } from "../../../dispatcher/actions";
 import SettingsStore from "../../../settings/SettingsStore";
 import { UIFeature } from "../../../settings/UIFeature";
 import Modal from "../../../Modal";
 import ErrorDialog from "../dialogs/ErrorDialog";
+import BugReportDialog from "../dialogs/BugReportDialog";
+import AccessibleButton from "../elements/AccessibleButton";
 import { contextMenuBelow } from "../rooms/RoomTile";
 import { ContextMenuTooltipButton } from "../../structures/ContextMenu";
 import IconizedContextMenu, {
@@ -36,6 +38,7 @@
 } from "../context_menus/IconizedContextMenu";
 import JumpToDatePicker from "./JumpToDatePicker";
 import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
+import { SdkContextClass } from "../../../contexts/SDKContext";
 
 function getDaysArray(): string[] {
     return [_t("Sunday"), _t("Monday"), _t("Tuesday"), _t("Wednesday"), _t("Thursday"), _t("Friday"), _t("Saturday")];
@@ -118,12 +121,12 @@
 
     private pickDate = async (inputTimestamp: number | string | Date): Promise<void> => {
         const unixTimestamp = new Date(inputTimestamp).getTime();
+        const roomIdForJumpRequest = this.props.roomId;
 
-        const cli = MatrixClientPeg.get();
         try {
-            const roomId = this.props.roomId;
+            const cli = MatrixClientPeg.get();
             const { event_id: eventId, origin_server_ts: originServerTs } = await cli.timestampToEvent(
-                roomId,
+                roomIdForJumpRequest,
                 unixTimestamp,
                 Direction.Forward,
             );
@@ -132,28 +135,106 @@
                     `found ${eventId} (${originServerTs}) for timestamp=${unixTimestamp} (looking forward)`,
             );
 
-            dis.dispatch<ViewRoomPayload>({
-                action: Action.ViewRoom,
-                event_id: eventId,
-                highlighted: true,
-                room_id: roomId,
-                metricsTrigger: undefined, // room doesn't change
-            });
-        } catch (e) {
-            const code = e.errcode || e.statusCode;
-            // only show the dialog if failing for something other than a network error
-            // (e.g. no errcode or statusCode) as in that case the redactions end up in the
-            // detached queue and we show the room status bar to allow retry
-            if (typeof code !== "undefined") {
-                // display error message stating you couldn't delete this.
+            // Only try to navigate to the room if the user is still viewing the same
+            // room. We don't want to jump someone back to a room after a slow request
+            // if they've already navigated away to another room.
+            const currentRoomId = SdkContextClass.instance.roomViewStore.getRoomId();
+            if (currentRoomId === roomIdForJumpRequest) {
+                dispatcher.dispatch<ViewRoomPayload>({
+                    action: Action.ViewRoom,
+                    event_id: eventId,
+                    highlighted: true,
+                    room_id: roomIdForJumpRequest,
+                    metricsTrigger: undefined, // room doesn't change
+                });
+            } else {
+                logger.debug(
+                    `No longer navigating to date in room (jump to date) because the user already switched ` +
+                        `to another room: currentRoomId=${currentRoomId}, roomIdForJumpRequest=${roomIdForJumpRequest}`,
+                );
+            }
+        } catch (err) {
+            logger.error(
+                `Error occured while trying to find event in ${roomIdForJumpRequest} ` +
+                    `at timestamp=${unixTimestamp}:`,
+                err,
+            );
+
+            // Only display an error if the user is still viewing the same room. We
+            // don't want to worry someone about an error in a room they no longer care
+            // about after a slow request if they've already navigated away to another
+            // room.
+            const currentRoomId = SdkContextClass.instance.roomViewStore.getRoomId();
+            if (currentRoomId === roomIdForJumpRequest) {
+                let friendlyErrorMessage = `An error occured while trying to find and jump to the given date.`;
+                let submitDebugLogsContent: JSX.Element = <></>;
+                if (err?.name === "ConnectionError" || err?.httpStatus) {
+                    if (err?.errcode === "M_NOT_FOUND") {
+                        friendlyErrorMessage = _t(
+                            "We were unable to find an event looking forwards from %(dateString)s. " +
+                                "Try choosing an earlier date.",
+                            { dateString: formatFullDateNoDay(new Date(unixTimestamp)) },
+                        );
+                    } else {
+                        friendlyErrorMessage = _t(
+                            "A network error occurred while trying to find and jump to the given date. " +
+                                "Your homeserver might be down or was just a temporary problem with your " +
+                                "internet connection. Please try again. If this continues, please " +
+                                "contact your homeserver administrator.",
+                        );
+                    }
+                } else {
+                    // We only give the option to submit logs for actual errors, not network problems.
+                    submitDebugLogsContent = (
+                        <p>
+                            {_t(
+                                "Please submit <debugLogsLink>debug logs</debugLogsLink> to help us " +
+                                    "track down the problem.",
+                                {},
+                                {
+                                    debugLogsLink: (sub) => (
+                                        <AccessibleButton
+                                            // This is by default a `<div>` which we
+                                            // can't nest within a `<p>` here so update
+                                            // this to a be a inline anchor element.
+                                            element="a"
+                                            kind="link"
+                                            onClick={() => this.onBugReport(err)}
+                                            data-testid="jump-to-date-error-submit-debug-logs-button"
+                                        >
+                                            {sub}
+                                        </AccessibleButton>
+                                    ),
+                                },
+                            )}
+                        </p>
+                    );
+                }
+
                 Modal.createDialog(ErrorDialog, {
-                    title: _t("Error"),
-                    description: _t("Unable to find event at that date. (%(code)s)", { code }),
+                    title: _t("Unable to find event at that date"),
+                    description: (
+                        <div data-testid="jump-to-date-error-content">
+                            <p>{friendlyErrorMessage}</p>
+                            {submitDebugLogsContent}
+                            <details>
+                                <summary>{_t("Error details")}</summary>
+                                <p>{String(err)}</p>
+                            </details>
+                        </div>
+                    ),
                 });
             }
         }
     };
 
+    private onBugReport = (err): void => {
+        Modal.createDialog(BugReportDialog, {
+            error: err,
+            initialText: "Error occured while using jump to date #jump-to-date",
+        });
+    };
+
     private onLastWeekClicked = (): void => {
         const date = new Date();
         date.setDate(date.getDate() - 7);
@@ -189,7 +270,11 @@
                     onFinished={this.onContextMenuCloseClick}
                 >
                     <IconizedContextMenuOptionList first>
-                        <IconizedContextMenuOption label={_t("Last week")} onClick={this.onLastWeekClicked} />
+                        <IconizedContextMenuOption
+                            label={_t("Last week")}
+                            onClick={this.onLastWeekClicked}
+                            data-testid="jump-to-date-last-week"
+                        />
                         <IconizedContextMenuOption label={_t("Last month")} onClick={this.onLastMonthClicked} />
                         <IconizedContextMenuOption
                             label={_t("The beginning of the room")}
@@ -207,7 +292,8 @@
         return (
             <ContextMenuTooltipButton
                 className="mx_DateSeparator_jumpToDateMenu mx_DateSeparator_dateContent"
+                data-testid="jump-to-date-separator-button"
                 onClick={this.onContextMenuOpenClick}
                 isExpanded={!!this.state.contextMenuPosition}
                 title={_t("Jump to date")}
             >

src/i18n/strings/en_EN.json

@@ -2350,7 +2350,15 @@
     "Saturday": "Saturday",
     "Today": "Today",
     "Yesterday": "Yesterday",
-    "Unable to find event at that date. (%(code)s)": "Unable to find event at that date. (%(code)s)",
+    "We were unable to find an event looking forwards from %(dateString)s. Try choosing an earlier date.": "We were unable to find an event looking forwards from %(dateString)s. Try choosing an earlier date.",
+    "A network error occurred while trying to find and jump to the given date. Your homeserver might be down or was just a temporary problem with your internet connection. Please try again. If this continues, please contact your homeserver administrator.": "A network error occurred while trying to find and jump to the given date. Your homeserver might be down or was just a temporary problem with your internet connection. Please try again. If this continues, please contact your homeserver administrator.",
+    "Please submit <debugLogsLink>debug logs</debugLogsLink> to help us track down the problem.": "Please submit <debugLogsLink>debug logs</debugLogsLink> to help us track down the problem.",
+    "Unable to find event at that date": "Unable to find event at that date",
+    "Error details": "Error details",
+    "Request status code: %(statusCode)s": "Request status code: %(statusCode)s",
+    "HTTP status code not available": "HTTP status code not available",
+    "Error code: %(errorCode)s": "Error code: %(errorCode)s",
+    "Error code not available": "Error code not available",
     "Last week": "Last week",
     "Last month": "Last month",
     "The beginning of the room": "The beginning of the room",

test/components/structures/auth/ForgotPassword-test.tsx

@@ -22,8 +22,13 @@ import { MatrixClient, createClient } from "matrix-js-sdk/src/matrix";
 
 import ForgotPassword from "../../../../src/components/structures/auth/ForgotPassword";
 import { ValidatedServerConfig } from "../../../../src/utils/ValidatedServerConfig";
-import { filterConsole, flushPromisesWithFakeTimers, stubClient } from "../../../test-utils";
-import Modal from "../../../../src/Modal";
+import {
+    clearAllModals,
+    filterConsole,
+    flushPromisesWithFakeTimers,
+    stubClient,
+    waitEnoughCyclesForModal,
+} from "../../../test-utils";
 import AutoDiscoveryUtils from "../../../../src/utils/AutoDiscoveryUtils";
 
 jest.mock("matrix-js-sdk/src/matrix", () => ({
@@ -55,11 +60,6 @@ describe("<ForgotPassword>", () => {
         });
     };
 
-    const waitForDialog = async (): Promise<void> => {
-        await flushPromisesWithFakeTimers();
-        await flushPromisesWithFakeTimers();
-    };
-
     const itShouldCloseTheDialogAndShowThePasswordInput = (): void => {
         it("should close the dialog and show the password input", () => {
             expect(screen.queryByText("Verify your email to continue")).not.toBeInTheDocument();
@@ -88,9 +88,9 @@ describe("<ForgotPassword>", () => {
         jest.spyOn(AutoDiscoveryUtils, "authComponentStateForError");
     });
 
-    afterEach(() => {
+    afterEach(async () => {
         // clean up modals
-        Modal.closeCurrentModal("force");
+        await clearAllModals();
     });
 
     beforeAll(() => {
@@ -322,7 +322,9 @@ describe("<ForgotPassword>", () => {
                     describe("and submitting it", () => {
                         beforeEach(async () => {
                             await click(screen.getByText("Reset password"));
-                            await waitForDialog();
+                            await waitEnoughCyclesForModal({
+                                useFakeTimers: true,
+                            });
                         });
 
                         it("should send the new password and show the click validation link dialog", () => {
@@ -350,7 +352,9 @@ describe("<ForgotPassword>", () => {
                                 await act(async () => {
                                     await userEvent.click(screen.getByTestId("dialog-background"), { delay: null });
                                 });
-                                await waitForDialog();
+                                await waitEnoughCyclesForModal({
+                                    useFakeTimers: true,
+                                });
                             });
 
                             itShouldCloseTheDialogAndShowThePasswordInput();
@@ -359,7 +363,9 @@ describe("<ForgotPassword>", () => {
                         describe("and dismissing the dialog", () => {
                             beforeEach(async () => {
                                 await click(screen.getByLabelText("Close dialog"));
-                                await waitForDialog();
+                                await waitEnoughCyclesForModal({
+                                    useFakeTimers: true,
+                                });
                             });
 
                             itShouldCloseTheDialogAndShowThePasswordInput();
@@ -368,7 +374,9 @@ describe("<ForgotPassword>", () => {
                         describe("and clicking »Re-enter email address«", () => {
                             beforeEach(async () => {
                                 await click(screen.getByText("Re-enter email address"));
-                                await waitForDialog();
+                                await waitEnoughCyclesForModal({
+                                    useFakeTimers: true,
+                                });
                             });
 
                             it("should close the dialog and go back to the email input", () => {
@@ -400,7 +408,9 @@ describe("<ForgotPassword>", () => {
                         beforeEach(async () => {
                             await click(screen.getByText("Sign out of all devices"));
                             await click(screen.getByText("Reset password"));
-                            await waitForDialog();
+                            await waitEnoughCyclesForModal({
+                                useFakeTimers: true,
+                            });
                         });
 
                         it("should show the sign out warning dialog", async () => {