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

List known client-server error codes; Clarify priority of error codes vs http status code #1637

Merged
merged 4 commits into from Aug 31, 2018

Conversation

4 participants
@turt2live
Member

turt2live commented Aug 31, 2018

Rendered: see 'docs' status check


Document known client-server error codes
Covers part of #603 (updating all the endpoints is being done as a separate PR/commit).

Reference: https://github.com/matrix-org/synapse/blob/74854a97191191b08101821753c2672efc2a65fd/synapse/api/errors.py#L30-L61

Clarify how the client should treat errors
This is based on observation and rough interpretation and may need additional review from people.

Fixes #1188

turt2live added some commits Aug 31, 2018

Clarify how the client should treat errors
This is based on observation and rough interpretation and may need additional review from people.

Fixes #1188

@turt2live turt2live requested a review from matrix-org/spec-core-team Aug 31, 2018

@turt2live turt2live added this to In review (just the PRs) in August 2018 r0 via automation Aug 31, 2018

Some standard error codes are below:
Errors are generally best expressed by their error code rather than the HTTP
status code returned. When encountering the error code ``M_UNKNOWN``, clients
should prefer the HTTP status code as a more reliable reference for what the

This comment has been minimized.

@erikjohnston

erikjohnston Aug 31, 2018

Member

Do you mean the error code is more reliable than the HTTP status code?

This comment has been minimized.

@turt2live

turt2live Aug 31, 2018

Member

yes - suggestions for wording welcome

This comment has been minimized.

@erikjohnston

erikjohnston Aug 31, 2018

Member

Well, "clients should prefer the error code as a more reliable reference..." would be better

This comment has been minimized.

@turt2live

turt2live Aug 31, 2018

Member

wait, having re-read this sentence: the flip for M_UNKNOWN is intentional. Because we return M_UNKNOWN in strange places without a better error code, the client should probably rely on the HTTP status code which gives a bit more context.

but the request gave a 400 Bad Request status code, the client should treat
the error as if the resource was not found. However, if the client were to
receive an error code of ``M_UNKNOWN`` with a 400 Bad Request, the client
should assume that the request being made was invalid.

This comment has been minimized.

@erikjohnston

erikjohnston Aug 31, 2018

Member

Do we actually ever do 400 with M_NOT_FOUND? If we don't then I think this is a confusing example, I'm not even sure its worth having this section really.

This comment has been minimized.

@turt2live

turt2live Aug 31, 2018

Member

Some amount of my memory says we do something similar in some cases. It's more to reinforce the above where the error code > http status

@turt2live turt2live requested review from erikjohnston and matrix-org/spec-core-team and removed request for erikjohnston Aug 31, 2018

@uhoreg

I have no context as to whether the documented error codes are used and correct, but the wording seems fine.

@richvdh

looks sane to me

@turt2live turt2live merged commit bb28356 into matrix-org:master Aug 31, 2018

7 checks passed

ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

August 2018 r0 automation moved this from In review (just the PRs) to Done (this list will be incomplete) Aug 31, 2018

@turt2live turt2live deleted the turt2live:travis/c2s/clarify-errors branch Aug 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment