Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error handling in jump to date #10405

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
e13e311
Better error handling in jump to date
MadLittleMods Mar 17, 2023
054688d
Actually find current room ID when comparing
MadLittleMods Mar 18, 2023
70f65f4
Merge branch 'develop' into madlittlemods/21263-better-error-handling…
MadLittleMods Mar 20, 2023
072d878
Add fallback for non HTTP errors
MadLittleMods Mar 20, 2023
2c247c9
Fix some strict errors
MadLittleMods Mar 20, 2023
72b2ebd
Add a way to submit debug logs for actual errors
MadLittleMods Mar 21, 2023
d5fd664
Add some tests
MadLittleMods Mar 21, 2023
8210828
Fix modals in tests leaking across
MadLittleMods Mar 21, 2023
bc13247
Generalize test
MadLittleMods Mar 21, 2023
d90f4f6
Remove duplicated info
MadLittleMods Mar 21, 2023
93d200f
Generalize modal problems into utilities
MadLittleMods Mar 21, 2023
1c724e1
Remove currentDate
MadLittleMods Mar 21, 2023
df2b4d2
Better language
MadLittleMods Mar 21, 2023
8c42581
Fix some imports
MadLittleMods Mar 21, 2023
bac5a89
Type error
MadLittleMods Mar 21, 2023
78cbc06
Try to type errors
MadLittleMods Mar 21, 2023
5ca2bb4
Fix event type
MadLittleMods Mar 21, 2023
6e921da
Update translations
MadLittleMods Mar 21, 2023
7079c69
Remove cruft
MadLittleMods Mar 21, 2023
bcb51b4
Fix test/components/structures/auth/ForgotPassword-test.tsx running f…
MadLittleMods Mar 21, 2023
99f8ff3
Merge branch 'develop' into madlittlemods/21263-better-error-handling…
MadLittleMods Mar 21, 2023
d3b0790
Fix export
MadLittleMods Mar 21, 2023
5fefd64
Add better comment
MadLittleMods Mar 21, 2023
b633a74
Merge branch 'develop' into madlittlemods/21263-better-error-handling…
MadLittleMods Mar 22, 2023
d901c90
No string interpolation needed
MadLittleMods Mar 22, 2023
9cecaa9
Better language
MadLittleMods Mar 22, 2023
52e6f30
Be more specific when we show the network problem language
MadLittleMods Mar 22, 2023
0902215
Add expects to show intention
MadLittleMods Mar 22, 2023
2aa7f91
Add comment about potential better future
MadLittleMods Mar 22, 2023
7c4a398
Update i18n
MadLittleMods Mar 23, 2023
23268af
Test other date picks
MadLittleMods Mar 23, 2023
1447b76
Add test for MatrixError
MadLittleMods Mar 23, 2023
222fcc4
Merge branch 'develop' into madlittlemods/21263-better-error-handling…
MadLittleMods Mar 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 64 additions & 20 deletions src/components/views/messages/DateSeparator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Direction } from "matrix-js-sdk/src/models/event-timeline";
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 { Action } from "../../../dispatcher/actions";
Expand Down Expand Up @@ -118,12 +118,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,
);
Expand All @@ -132,23 +132,67 @@ 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This previous comment was bogus. Just copy/paste cruft from

// 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 roomIdBeforeNavigation = this.props.roomId;
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
if (roomIdBeforeNavigation === roomIdForJumpRequest) {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
event_id: eventId,
highlighted: true,
room_id: roomIdBeforeNavigation,
metricsTrigger: undefined, // room doesn't change
});
}
} 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 roomIdBeforeDisplayingError = this.props.roomId;
if (roomIdBeforeDisplayingError === roomIdForJumpRequest) {
let friendlyErrorMessage = `An error occured while trying to find and jump to the given date.`;
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
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)) },
);
}
if (err.name === "ConnectionError") {
friendlyErrorMessage = _t(
"Your homeserver was unreachable and was not able to log you in. Please try again. " +
"If this continues, please contact your homeserver administrator.",
);
}

Modal.createDialog(ErrorDialog, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to fix this TypeScript error:

src/components/views/messages/DateSeparator.tsx:214:36 - Argument of type 'typeof ErrorDialog' is not assignable to parameter of type 'ComponentType'.

Same pattern we use everywhere else in the codebase so I don't know of a better example to work from either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alunturner Any tips on how to fix this error?

Copy link
Contributor

Choose a reason for hiding this comment

The 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 Modal.createDialog (so it seems to me) given the consistency of the TS errors throughout the codebase. If I had tips, I'd give them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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: (
<>
<p>{friendlyErrorMessage}</p>
<details>
<summary>{_t("Error details")}</summary>

<ul>
<li>{_t("Request status code: %(statusCode)s", { statusCode: err.httpStatus })}</li>
<li>
{_t("Error code: %(errorCode)s", {
errorCode: err.errcode || _t("Error code not available"),
})}
</li>
</ul>
<p>{String(err)}</p>
</details>
</>
),
});
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -2350,7 +2350,12 @@
"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.",
"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",
"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",
Expand Down