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

HTTP status codes are used to describe JSON RPC data #893

Open
AdamSLevy opened this issue Oct 6, 2019 · 2 comments
Open

HTTP status codes are used to describe JSON RPC data #893

AdamSLevy opened this issue Oct 6, 2019 · 2 comments

Comments

@AdamSLevy
Copy link
Contributor

AdamSLevy commented Oct 6, 2019

The factomd JSON RPC 2.0 API uses HTTP status codes to indicate a JSON RPC Error Response. This make it difficult for generic JSON RPC 2.0 clients to properly handle Error Responses, since the HTTP status code can no longer be relied upon to sort out whether a JSON RPC server was actually reached successfully.

The JSON RPC 2.0 specification is not specific to HTTP, so it has nothing to say about HTTP status codes. If this were a REST API then the response code would be well defined, but it is not here. HTTP is simply a transport layer to the JSON RPC 2.0 data. When you consider that JSON RPC 2.0 specifies batch requests and responses as well, this further muddies the waters of using something like an HTTP status code to describe the JSON RPC Response.

I believe all JSON RPC 2.0 Responses, regardless of whether they have a "result" or "error" field defined, should be returned with HTTP Status 200/OK to allow clients to easily determine that they actually reached a JSON RPC server. This way a 200/OK status code means that it should be safe to unmarshal the HTTP Response body as a JSON RPC response, and an unmarshaling error at this point indicates a serious integrity or implementation error with the server. Further information about the JSON RPC error is then well defined by the spec.

In the case of the current factomd API implementation, the client must still attempt to unmarshal the body when an HTTP status code is 400, which means that the client code needs to be made bespoke to this JSON RPC API server. Moreover these status codes are not defined anywhere in any spec, which means that they must be guessed at.

@WhoSoup
Copy link
Member

WhoSoup commented Oct 16, 2019

I agree with this change and am submitting two PRs to address this.

@AdamSLevy in addition to the status 400 for jsonrpc, the api returns a http status 401 error when the www-authentication is wrong. in that case, it does not return a json-rpc error, so i think it is consistent with your request, but I wanted to mention it in case you think special handling is necessary for that. (it is theoretically possible to return a json rpc authentication error)

@AdamSLevy
Copy link
Contributor Author

AdamSLevy commented Oct 18, 2019

@WhoSoup I agree with what you wrote. JSON RPC has nothing to saw about authentication, and the authentication in this case is defined by HTTP Basic Auth, so such status codes are appropriate.

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

No branches or pull requests

2 participants