Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Provide a more detailed error message than "No known servers" #6048

Merged
merged 23 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
efd793c
Provide a more detailed error message than "No known servers"
aaronraimist May 17, 2021
513300a
Merge branch 'develop' into no-known-servers
aaronraimist Jun 23, 2021
bcb12af
Merge branch 'develop' into no-known-servers
aaronraimist Jun 29, 2021
9b33261
Fix PR since file was refactored
aaronraimist Jun 29, 2021
3f4ee2f
Fix formatting
aaronraimist Jun 29, 2021
72f9d7a
Merge branch 'develop' into no-known-servers
aaronraimist Jan 23, 2022
b6aa7e6
lint
aaronraimist Jan 25, 2022
bfb5707
Merge branch 'develop' into no-known-servers
aaronraimist Jan 25, 2022
170da1b
Update src/stores/RoomViewStore.tsx
aaronraimist Jan 26, 2022
26f974d
Merge branch 'develop' into no-known-servers
aaronraimist Jul 2, 2022
c58fe0b
Add example identifiers and a more detailed explanation
aaronraimist Jul 3, 2022
0c52759
Lint
aaronraimist Jul 3, 2022
0b3f050
Lint
aaronraimist Jul 3, 2022
78eb743
Merge branch 'develop' into no-known-servers
aaronraimist Feb 21, 2023
c72ac81
Revert back to original wording (except s/alias/address)
aaronraimist Feb 21, 2023
2791011
Prettier
aaronraimist Feb 21, 2023
030ced3
Merge branch 'develop' into no-known-servers
florianduros Feb 23, 2023
6c72541
Fix ts error
florianduros Feb 23, 2023
19ad1a2
Add snapshot test
florianduros Feb 23, 2023
ce8c0d9
Check the Modal props
florianduros Feb 23, 2023
a99f514
Add test case to reach quality gate
florianduros Feb 23, 2023
b2b4732
Merge branch 'develop' into no-known-servers
florianduros Feb 24, 2023
e3982da
Merge branch 'develop' into no-known-servers
florianduros Feb 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,12 @@
"Please contact your homeserver administrator.": "Please contact your homeserver administrator.",
"The person who invited you has already left.": "The person who invited you has already left.",
"The person who invited you has already left, or their server is offline.": "The person who invited you has already left, or their server is offline.",
"This Room ID (%(id)s) cannot be joined. Try to join using a Room Alias (#room:example.com) instead.": "This Room ID (%(id)s) cannot be joined. Try to join using a Room Alias (#room:example.com) instead.",
"Why?": "Why?",
"A Room ID alone cannot be used to join a room unless your homeserver already participates in that room.": "A Room ID alone cannot be used to join a room unless your homeserver already participates in that room.",
"For a Room ID to be joinable over federation, a list of homeservers to attempt to join through must be provided. You didn't provide any.": "For a Room ID to be joinable over federation, a list of homeservers to attempt to join through must be provided. You didn't provide any.",
"The sole purpose of the domain at the end of a Room ID is to ensure it is unique.": "The sole purpose of the domain at the end of a Room ID is to ensure it is unique.",
"For these reasons, Room Aliases are the recommended way to join rooms.": "For these reasons, Room Aliases are the recommended way to join rooms.",
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 not entirely sure how this needs to be formatted/escaped, but here goes:

Suggested change
"For a Room ID to be joinable over federation, a list of homeservers to attempt to join through must be provided. You didn't provide any.": "For a Room ID to be joinable over federation, a list of homeservers to attempt to join through must be provided. You didn't provide any.",
"The sole purpose of the domain at the end of a Room ID is to ensure it is unique.": "The sole purpose of the domain at the end of a Room ID is to ensure it is unique.",
"For these reasons, Room Aliases are the recommended way to join rooms.": "For these reasons, Room Aliases are the recommended way to join rooms.",
"In order to join a Room ID over federation, you need to explicitly provide a list of &quot;via&quot; homeservers which already know the room. One way to do so is using a <a href=\"https://github.com/matrix-org/matrix.to#optional-parameters\">matrix.to</a> URL. You can generate one in Element by right clicking on a room in your room list and selecting &quot;Copy room link&quot;.": "In order to join a Room ID over federation, you need to explicitly provide a list of &quot;via&quot; homeservers which already know the room. One way to do so is using a <a href=\"https://github.com/matrix-org/matrix.to#optional-parameters\">matrix.to</a>URL. You can generate one in Element by right clicking on a room in your room list and selecting &quot;Copy room link&quot;.",
"For these reasons, Room Aliases are the recommended way to join rooms.": "For these reasons, Room Aliases are the recommended way to join rooms.",

"Failed to join": "Failed to join",
"Connection lost": "Connection lost",
"You were disconnected from the call. (Error: %(message)s)": "You were disconnected from the call. (Error: %(message)s)",
Expand Down
29 changes: 28 additions & 1 deletion src/stores/RoomViewStore.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ export class RoomViewStore extends Store<ActionPayload> {
</div>;
} else if (err.httpStatus === 404) {
const invitingUserId = this.getInvitingUserId(roomId);
// only provide a better error message for invites
// 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.
if (invitingUserId.endsWith(`:${MatrixClientPeg.get().getDomain()}`)) {
Expand All @@ -446,6 +446,33 @@ export class RoomViewStore extends Store<ActionPayload> {
description = _t("The person who invited you has already left, or their server is offline.");
}
}

// if joined through room ID and didn't provide any via servers, provide
// a more detailed error than "No known servers"
if (roomId === this.state.roomId && !this.state.viaServers?.length) {
description = <div>
{ _t("This Room ID (%(id)s) cannot be joined. Try to join using a Room " +
"Alias (#room:example.com) instead.", {
id: this.state.roomId,
}) }<br /><br />

<details>
<summary>{ _t("Why?") }</summary><br />

{ _t("A Room ID alone cannot be used to join a room unless your " +
"homeserver already participates in that room.") }<br /><br />

{ _t("For a Room ID to be joinable over federation, a list of " +
"homeservers to attempt to join through must be provided. " +
"You didn't provide any.") }<br /><br />

{ _t("The sole purpose of the domain at the end of a Room ID " +
"is to ensure it is unique.") }<br /><br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ _t("For a Room ID to be joinable over federation, a list of " +
"homeservers to attempt to join through must be provided. " +
"You didn't provide any.") }<br /><br />
{ _t("The sole purpose of the domain at the end of a Room ID " +
"is to ensure it is unique.") }<br /><br />
{ _t("In order to join a Room ID over federation, you need to " +
"explicitly provide a list of &quot;via&quot; homeservers " +
"which already know the room. One way to do so is using a " +
"<a href=\"https://github.com/matrix-org/matrix.to#optional-parameters\">matrix.to</a> " +
"URL. You can generate one in Element by right clicking on " +
"a room in your room list and selecting &quot;Copy room link&quot;.") }<br /><br />


{ _t("For these reasons, Room Aliases are the recommended way to join rooms.") }
</details>
</div>;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to https://github.com/vector-im/element-web/issues/24475

Maybe some language inspiration in this:

Failed to join

The !JsbthXjlaqosxISjgl:gitter.im room isn't known to your homeserver (matrix.org).

You can try using a "via" server to join through which is supplied as the 2nd parameter in the /join slash command: /join !JsbthXjlaqosxISjgl:gitter.im gitter.im

Or if you're using a matrix.to link, you can try adding a ?via=gitter.im query parameter to the URL: https://matrix.to/#/!JsbthXjlaqosxISjgl:gitter.im?via=gitter.im

Alternatively, if you know a room alias (starts with a #) associated with this room, that will always be routable and allow you to join.

Error details (expand/collapse)

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I feel like using an alias is the more user friendly option and should be suggested first (move up to 2nd paragraph).
  2. If unavailable, the next best option is to have whoever provided the alias provide a matrix.to with vias (automatically, by using their client's share or "copy room link" function).
  3. Then the more advanced stuff about the join command can follow.

Copy link
Contributor

@MadLittleMods MadLittleMods Feb 9, 2023

Choose a reason for hiding this comment

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

Not all rooms have an alias and you can stumble upon a room ID in an unlimited number of ways. The user obviously tried to use a room ID here and we should make it possible for them to continue with what they have.

Leaving the user helpless to go ask someone else for a room alias isn't very empowering.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have any data to argue that point, except I think the user probably doesn't have that choice and can at best make a very educated guess.

However I do disagree with your previous suggestion:

You can try using a "via" server to join through which is supplied as the 2nd parameter in the /join slash command: /join !JsbthXjlaqosxISjgl:gitter.im gitter.im

Honestly, either

  • there is no way to progress at this point, because how are you supposed to guess a via server when the room ID does not hint at one (I think new IDs don't have them anymore?). Basically what I already said.
  • it is completely ridiculous to ask users to repeat part of the room ID (!room:domain.tld domain.tld) in some command instead of detecting and trying it automatically.

The best I can come up with is to "try using the server (latter part of matrix ID) or a user who you know is in the room as a via server)".

Copy link
Contributor

@MadLittleMods MadLittleMods Feb 9, 2023

Choose a reason for hiding this comment

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

(I think new IDs don't have them anymore?).

All room IDs include this information at the moment. And is why I created https://github.com/vector-im/element-web/issues/24475 to suggest to people how to do the right thing (derive the via server from the room ID)

it is completely ridiculous to ask users to repeat part of the room ID (!room:domain.tld domain.tld) in some command instead of detecting and trying it automatically.

Agreed that this should just be automatic, but I think we're at ideological odds with the decision makers since they would want to treat room IDs as opaque strings that shouldn't be parsed. Treating it as an opaque string is generally a good heuristic since there is some future-looking here where room IDs potentially won't include this information (wish I had some links for future desire).

The half-measure of suggesting the right thing at least gets them to the right place and teaches for next time. And I think has the potential to actually be merged.

}

Modal.createDialog(ErrorDialog, {
Expand Down