Return 404 instead of 500 when client tries to join non-existent room #1579

ezera · 2026-03-27T00:05:40Z

ezera commented

2026-03-27 00:05:40 +00:00

This pull request changes the error returned by the server when the user attempts to join a non-existent room (see #1443). Previously, the server returned error 500. Now, we return error 404, with a generic error message, which is closer to what Synapse does (as stated in #1443).

I considered returning

403 M_UNKNOWN or other generic message (prevent malicious users to enumerate room ids this way)

because I agree that it would make sense to prevent someone from enumerating which rooms exist. However, I settled on 404, because I thought it would make sense to mimic Synapse for the sake of client compatibility. Please let me know if you disagree.

I also defined a new err! macro arm. I wasn't sure that was the best idea, but there didn't seem to be an existing arm that fit for returning this kind of error.

I ran the tests locally, and everything passed, but I haven't added a test for this specific case.

Fixes: #1443

Pull request checklist:

This pull request targets the main branch, and the branch is named something other than
main.
I have written an appropriate pull request title and my description is clear.
I understand I am responsible for the contents of this pull request.
I have followed the contributing guidelines:
- My contribution follows the code style, if applicable.
- I ran pre-commit checks before opening/drafting this pull request.
- I have tested my contribution (or proof-read it for documentation-only changes)
  myself, if applicable. This includes ensuring code compiles.
- My commit messages follow the commit message format and are descriptive.
- I have written a news fragment for this PR, if applicable.

This pull request changes the error returned by the server when the user attempts to join a non-existent room (see https://forgejo.ellis.link/continuwuation/continuwuity/issues/1443). Previously, the server returned error 500. Now, we return error 404, with a generic error message, which is closer to what Synapse does (as stated in #1443). I considered returning > 403 M_UNKNOWN or other generic message (prevent malicious users to enumerate room ids this way) because I agree that it would make sense to prevent someone from enumerating which rooms exist. However, I settled on 404, because I thought it would make sense to mimic Synapse for the sake of client compatibility. Please let me know if you disagree. I also defined a new `err!` macro arm. I wasn't sure that was the best idea, but there didn't seem to be an existing arm that fit for returning this kind of error. I ran the tests locally, and everything passed, but I haven't added a test for this specific case.   Fixes: #1443  **Pull request checklist:**  - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. - [ ] I have written a [news fragment][n1] for this PR, if applicable.  [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages [n1]: https://towncrier.readthedocs.io/en/stable/tutorial.html#creating-news-fragments

nex requested changes

2026-03-27 00:23:40 +00:00

src/api/client/membership/join.rs Outdated

					
				@ -786,3 +786,3 @@

					if servers.is_empty() || servers.len() == 1 && services.globals.server_is_ours(&servers[0]) {

						return Err(error);

						return Err!(Request(Unknown("Unknown error"), NOT_FOUND));

This masks Forbidden errors such as "not invited to room" and incorrectly returns 404 not found instead of 403 forbidden etc, which may (and will in future) hide important details from the end user. 404/M_UNKNOWN should only be returned if we genuinely don't know the room exists

Thanks! Does the new code match the behaviour you would expect?

nex added the

Enhancement

Matrix/Client

labels

2026-03-27 00:23:57 +00:00

ezera changed title from ~~Return 404 instead of 500 when client tries to join non-existent room~~ to WIP: Return 404 instead of 500 when client tries to join non-existent room

2026-03-27 00:47:31 +00:00

ezera force-pushed main from 2344e04107

Check Changelog / Check for changelog (pull_request_target) Successful in 11s

Details

Documentation / Build and Deploy Documentation (pull_request) Has been skipped

Details

Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m14s

Details

Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 21m45s

Details

to f740be422d

Check Changelog / Check for changelog (pull_request_target) Waiting to run

Details

Documentation / Build and Deploy Documentation (pull_request) Has been skipped

Details

Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m3s

Details

Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 19m52s

Details

2026-03-27 01:29:01 +00:00

Compare

ezera changed title from ~~WIP: Return 404 instead of 500 when client tries to join non-existent room~~ to Return 404 instead of 500 when client tries to join non-existent room

2026-03-27 01:31:21 +00:00

ezera requested review from nex

2026-03-27 01:44:20 +00:00

nex self-assigned this

2026-03-29 16:48:20 +00:00

nex requested changes

2026-04-01 16:20:58 +00:00

nex left a comment

Looks good now, I just think the wording of the 404 could be improved

src/api/client/membership/join.rs Outdated

					
				@ -786,2 +786,4 @@

					if servers.is_empty() || servers.len() == 1 && services.globals.server_is_ours(&servers[0]) {

						if !services.rooms.metadata.exists(room_id).await {

							return Err!(Request(Unknown("Unknown error"), NOT_FOUND));

I think since we now know that we just don't have the room locally, an error message of Room was not found locally and no servers were found to help us discover it may be more descriptive than Unknown error (thinking about all the people who may join our support room confused by "unknown error" when joining a room via ID with no vias)

I think since we now know that we just don't have the room locally, an error message of `Room was not found locally and no servers were found to help us discover it` may be more descriptive than `Unknown error` (thinking about all the people who may join our support room confused by "unknown error" when joining a room via ID with no vias)

👍 1