Add example to each endpoint when the capability is not available - #2212
Add example to each endpoint when the capability is not available#2212hughns wants to merge 6 commits into
Conversation
hughns
changed the title
| value: { | ||
| "submit_url": "https://example.org/path/to/submitToken" | ||
| } | ||
| "400": |
There was a problem hiding this comment.
Where do these status codes choices come from?
There was a problem hiding this comment.
The original MSC3283 did not specify the expected response code and referred to the Synapse implementation. So, I took these values from the Synapse implementation.
Another approach to the clarification would be to specify a range. e.g. 400-499
Co-authored-by: Johannes Marbach <n0-0ne+github@mailbox.org>
| application/json: | ||
| schema: | ||
| $ref: definitions/auth_response.yaml | ||
| "403": |
There was a problem hiding this comment.
This code came from approach described at https://github.com/matrix-org/matrix-spec/pull/2212/files#r2378179370
| "errcode": "M_INVALID_PARAM", | ||
| "error": "Invalid profile key.", | ||
| } | ||
| capability_disabled: |
There was a problem hiding this comment.
Again, this code was based on the Synapse implementation.
| application/json: | ||
| schema: | ||
| $ref: definitions/auth_response.yaml | ||
| "404": |
There was a problem hiding this comment.
I'm re-reading MSC3882 now (which I happened to author). The suggested 404 here is based on Synapse implementation.
An alternative would be:
- 404 M_UNRECOGNIZED = the server does not implement the optional login token feature
- 400 (or 403) M_FORBIDDEN = the user is not allowed to do this (i.e. capability would return false)
|
@anoadragon453 I've added some more notes inline about where the response code came from. If you are thinking that the definition of these error codes actually warrants an MSC then that seems reasonable and I can write the MSC. If that is the case then I would simplify this PR down by removing the definition of the error codes so that all that remained was the notes for each endpoint saying that the is a corresponding capability to check. |
hughns
changed the title
hughns
changed the title
|
I've reduced the scope of this and separated out the changes which purely add a note that capability negotiation exists for an endpoint (into #2223). |
anoadragon453
left a comment
anoadragon453
left a comment
There was a problem hiding this comment.
I see that we're not only adding examples to the spec, but also real response codes that clients are expected to follow. This requires an MSC.
MSC3283 and MSC3882 only adds new entries to the capabilities endpoint. They don't specify that the relevant endpoints clients will be using should change their error response behaviour based on whether the capability is true or false.
Could you write up a quick MSC detailing each of these new error responses (status code and error codes)? And do you have a sense of whether introducing a new M_CAPABILITY_NOT_ENABLED error code instead of using the existing ones would break lots of functionality?
I'd feel better about telling clients to expect 400 / M_CAPABILITY_NOT_ENABLED instead of 400 / M_FORBIDDEN (especially as Forbidden is typically associated with 403 status codes).
I've started this in MSC4369.
Not off the top of my head, but I will consider in the MSC. |
3 participants
Pull Request Checklist
Signed-off-by: Hugh Nimmo-Smith hughns@element.io
Preview: https://pr2212--matrix-spec-previews.netlify.app