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

Move standard error schema to appendices and share between all APIs #2026

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

olivia-fl
Copy link

@olivia-fl olivia-fl commented Dec 9, 2024

I asked about this on matrix, and tulir confirmed that the error schema was intended to be shared between all APIs. I would like the shared schema to be explicit in the spec text so that ruma can move to using the same error type for client-server and server-server endpoints. This would simplify error handling in server implementations.

I'm unsure what the correct thing to do with the error codes that were named in the identity service API but not the client-server API. M_MISSING_PARAMS (distinct from M_MISSING_PARAM from C2S) is particularly weird. I checked that these are actually used in sydent, and not used in synapse, so I just left them in the identity service API spec.

Pull Request Checklist

@olivia-fl olivia-fl requested a review from a team as a code owner December 9, 2024 08:29
@richvdh
Copy link
Member

richvdh commented Dec 10, 2024

@olivia-fl apologies, this has conflicted with #2016. Could you merge latest main into it?

Comment on lines 42 to 178
code if the endpoint is implemented, but the incorrect HTTP method is used.

`M_UNKNOWN`
An unknown error has occurred.

### Other error codes

The following error codes are specific to certain endpoints.

<!-- TODO: move them to the endpoints that return them -->

`M_UNAUTHORIZED`
The request was not correctly authorized. Usually due to login failures.

`M_USER_DEACTIVATED`
The user ID associated with the request has been deactivated. Typically
for endpoints that prove authentication, such as `/login`.

`M_USER_IN_USE`
Encountered when trying to register a user ID which has been taken.

`M_INVALID_USERNAME`
Encountered when trying to register a user ID which is not valid.

`M_ROOM_IN_USE`
Sent when the room alias given to the `createRoom` API is already in
use.

`M_INVALID_ROOM_STATE`
Sent when the initial state given to the `createRoom` API is invalid.

`M_THREEPID_IN_USE`
Sent when a threepid given to an API cannot be used because the same
threepid is already in use.

`M_THREEPID_NOT_FOUND`
Sent when a threepid given to an API cannot be used because no record
matching the threepid was found.

`M_THREEPID_AUTH_FAILED`
Authentication could not be performed on the third-party identifier.

`M_THREEPID_DENIED`
The server does not permit this third-party identifier. This may happen
if the server only permits, for example, email addresses from a
particular domain.

`M_SERVER_NOT_TRUSTED`
The client's request used a third-party server, e.g. identity server,
that this server does not trust.

`M_UNSUPPORTED_ROOM_VERSION`
The client's request to create a room used a room version that the
server does not support.

`M_INCOMPATIBLE_ROOM_VERSION`
The client attempted to join a room that has a version the server does
not support. Inspect the `room_version` property of the error response
for the room's version.

`M_BAD_STATE`
The state change requested cannot be performed, such as attempting to
unban a user who is not banned.

`M_GUEST_ACCESS_FORBIDDEN`
The room or resource does not permit guests to access it.

`M_CAPTCHA_NEEDED`
A Captcha is required to complete the request.

`M_CAPTCHA_INVALID`
The Captcha provided did not match what was expected.

`M_MISSING_PARAM`
A required parameter was missing from the request.

`M_INVALID_PARAM`
A parameter that was specified has the wrong value. For example, the
server expected an integer and instead received a string.

`M_TOO_LARGE`
The request or entity was too large.

`M_EXCLUSIVE`
The resource being requested is reserved by an application service, or
the application service making the request has not created the resource.

`M_RESOURCE_LIMIT_EXCEEDED`
The request cannot be completed because the homeserver has reached a
resource limit imposed on it. For example, a homeserver held in a shared
hosting environment may reach a resource limit if it starts using too
much memory or disk space. The error MUST have an `admin_contact` field
to provide the user receiving the error a place to reach out to.
Typically, this error will appear on routes which attempt to modify
state (e.g.: sending messages, account data, etc) and not routes which
only read state (e.g.: `/sync`, get account data, etc).

`M_CANNOT_LEAVE_SERVER_NOTICE_ROOM`
The user is unable to reject an invite to join the server notices room.
See the [Server Notices](#server-notices) module for more information.
Copy link
Member

Choose a reason for hiding this comment

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

It's far from obvious to me that all of these error codes belong outside the C-S API spec. Some of them do seem generic, but anything talking about access tokens is probably irrelevant outside the C-S API.

I'll also note that MSC3823 (which is about to land, via #2014) explicitly specifies a new error code which can only be returned by the C-S API.

I'm happy for the schema definition to move, but I think we should be more conservative before we specify that any S-S API can return M_USER_LOCKED etc.

Copy link
Author

Choose a reason for hiding this comment

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

That seems reasonable. I can go through this list and try to determine which ones are C-S-specific, and then move them to the C-S docs (like what I did with the identity service error codes).

I think that at least stuff like M_UNKNOWN should be shared.

Copy link
Author

Choose a reason for hiding this comment

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

New version has all the error codes that are only used in C2S in the C2S docs. As mentioned in the commit message, I was only able to determine this by reading the synapse source code rather than by looking at the existing spec text, because most endpoints are missing error codes. Let me know if you think the resulting partition is reasonable.

This was added in 9198182, but it was
already present earlier in the list, with a slightly different
description.

Signed-off-by: Olivia Lee <[email protected]>
I asked about this on matrix, and tulir confirmed[1] that the error
schema was intended to be shared between all APIs. I would like the
shared schema to be explicit in the spec text so that ruma can move to
using the same error type for client-server and server-server endpoints.
This would simplify error handling in server implementations.

Error codes that are used in more than one API were moved to the
appendices, while error codes specific to only one API were left there.
Since the docs on which error codes are actually used by which endpoints
aren't very complemete, I determined this by looking at the synapse
source code.

[1]: https://matrix.to/#/#matrix-spec-discussion:neko.dev/$Y2yTCeR_AeW6g_4jViFbx4gTE_AwF0RN7yrHJ25F5Q8

Signed-off-by: Olivia Lee <[email protected]>
@olivia-fl
Copy link
Author

Rebased on main, but found a duplicated error code introduced in #1944. I've removed that in af6e8a2, before the change to move the error codes to the appendix. I also decided to move the rate limiting section to the appendix, because it is referenced by the common M_LIMIT_EXCEEDED error, and because we have "Rate-Limited: [Yes/No]" on non-C2S endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants