-
-
Notifications
You must be signed in to change notification settings - Fork 833
Better error handling in jump to date #10405
Changes from all commits
e13e311
054688d
70f65f4
072d878
2c247c9
72b2ebd
d5fd664
8210828
bc13247
d90f4f6
93d200f
1c724e1
df2b4d2
8c42581
bac5a89
78cbc06
5ca2bb4
6e921da
7079c69
bcb51b4
99f8ff3
d3b0790
5fefd64
b633a74
d901c90
9cecaa9
52e6f30
0902215
2aa7f91
7c4a398
23268af
1447b76
222fcc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -18,16 +18,19 @@ limitations under the License. | |||||||
import React from "react"; | ||||||||
import { Direction } from "matrix-js-sdk/src/models/event-timeline"; | ||||||||
import { logger } from "matrix-js-sdk/src/logger"; | ||||||||
import { ConnectionError, MatrixError, HTTPError } from "matrix-js-sdk/src/http-api"; | ||||||||
|
||||||||
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, { ButtonEvent } from "../elements/AccessibleButton"; | ||||||||
import { contextMenuBelow } from "../rooms/RoomTile"; | ||||||||
import { ContextMenuTooltipButton } from "../../structures/ContextMenu"; | ||||||||
import IconizedContextMenu, { | ||||||||
|
@@ -36,6 +39,7 @@ import IconizedContextMenu, { | |||||||
} 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")]; | ||||||||
|
@@ -76,7 +80,7 @@ export default class DateSeparator extends React.Component<IProps, IState> { | |||||||
if (this.settingWatcherRef) SettingsStore.unwatchSetting(this.settingWatcherRef); | ||||||||
} | ||||||||
|
||||||||
private onContextMenuOpenClick = (e: React.MouseEvent): void => { | ||||||||
private onContextMenuOpenClick = (e: ButtonEvent): void => { | ||||||||
e.preventDefault(); | ||||||||
e.stopPropagation(); | ||||||||
const target = e.target as HTMLButtonElement; | ||||||||
|
@@ -118,12 +122,12 @@ export default class DateSeparator extends React.Component<IProps, IState> { | |||||||
|
||||||||
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 +136,113 @@ export default class DateSeparator extends React.Component<IProps, IState> { | |||||||
`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 | ||||||||
Comment on lines
-144
to
-146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This previous comment was bogus. Just copy/paste cruft from matrix-react-sdk/src/components/views/dialogs/ConfirmRedactDialog.tsx Lines 96 to 98 in bc60e59
|
||||||||
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 instanceof ConnectionError) { | ||||||||
friendlyErrorMessage = _t( | ||||||||
"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.", | ||||||||
); | ||||||||
} else if (err instanceof MatrixError) { | ||||||||
if (err?.errcode === "M_NOT_FOUND") { | ||||||||
friendlyErrorMessage = _t( | ||||||||
"We were unable to find an event looking forwards from %(dateString)s. " + | ||||||||
MadLittleMods marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
"Try choosing an earlier date.", | ||||||||
{ dateString: formatFullDateNoDay(new Date(unixTimestamp)) }, | ||||||||
); | ||||||||
} else { | ||||||||
friendlyErrorMessage = _t("Server returned %(statusCode)s with error code %(errorCode)s", { | ||||||||
statusCode: err?.httpStatus || _t("unknown status code"), | ||||||||
errorCode: err?.errcode || _t("unavailable"), | ||||||||
}); | ||||||||
} | ||||||||
} else if (err instanceof HTTPError) { | ||||||||
friendlyErrorMessage = err.message; | ||||||||
} 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 instanceof Error ? err : undefined)} | ||||||||
data-testid="jump-to-date-error-submit-debug-logs-button" | ||||||||
> | ||||||||
{sub} | ||||||||
</AccessibleButton> | ||||||||
), | ||||||||
}, | ||||||||
)} | ||||||||
</p> | ||||||||
); | ||||||||
} | ||||||||
|
||||||||
Modal.createDialog(ErrorDialog, { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how to fix this TypeScript error:
Same pattern we use everywhere else in the codebase so I don't know of a better example to work from either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alunturner Any tips on how to fix this error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I think I've probably done the same as you and checked the other uses of this, seeing that they all have the same issue. The problem must lie in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've created an issue to track this element-hq/element-web#24899 I guess we will just go with an exception here and worry about the problem generally in a separate PR down the line. |
||||||||
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?: Error): 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,11 +278,20 @@ export default class DateSeparator extends React.Component<IProps, IState> { | |||||||
onFinished={this.onContextMenuCloseClick} | ||||||||
> | ||||||||
<IconizedContextMenuOptionList first> | ||||||||
<IconizedContextMenuOption label={_t("Last week")} onClick={this.onLastWeekClicked} /> | ||||||||
<IconizedContextMenuOption label={_t("Last month")} onClick={this.onLastMonthClicked} /> | ||||||||
<IconizedContextMenuOption | ||||||||
label={_t("Last week")} | ||||||||
onClick={this.onLastWeekClicked} | ||||||||
data-testid="jump-to-date-last-week" | ||||||||
/> | ||||||||
<IconizedContextMenuOption | ||||||||
label={_t("Last month")} | ||||||||
onClick={this.onLastMonthClicked} | ||||||||
data-testid="jump-to-date-last-month" | ||||||||
/> | ||||||||
<IconizedContextMenuOption | ||||||||
label={_t("The beginning of the room")} | ||||||||
onClick={this.onTheBeginningClicked} | ||||||||
data-testid="jump-to-date-beginning" | ||||||||
/> | ||||||||
</IconizedContextMenuOptionList> | ||||||||
|
||||||||
|
@@ -207,6 +305,7 @@ export default class DateSeparator extends React.Component<IProps, IState> { | |||||||
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")} | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning a boolean so we can keep looping until all modals are closed. See
clearAllModals