Use HTTP status codes and consistent errors in RPC

[jcraigk]

Related to #1783

RPC is currently accessible over a POST-only HTTP API. Responses always return HTTP status code 200. Instead, response codes should reflect the outcome of the request. See https://www.restapitutorial.com/httpstatuscodes.html for standard usage.

Failures should be handled by returning an appropriate non-200 status code and having a consistently named error key in the JSON response, such as error, which contains a human-readable explanation of what went wrong.

Examples:

• POSTing an unknown action should result in status 400 with "error": "The action <foo> is not a supported RPC command" in the JSON payload
• Failures of a specific action such as bootstrap should return non-200 status and a detailed error message in error about what went wrong
• General failures on the server should result in status 500 with a generic error message

Additionally, specific successful 2xx status codes can be used such as 201 when creating a wallet or account.

[zhyatt]

@jcraigk Thank you for highlighting this. After discussing with the team we are planning on implementing these type of changes alongside other planned RPC improvements in a future release. We will coordinate with the community ahead of time to make sure we cover the most pressing RPC-related items.

[aspic]

Totally agree on using HTTP status codes to indicate if a request was successful or not.

However, don't think an error should be encoded as JSON. This puts more responsibility of error handling onto client. In this case the client first has to check HTTP status code, if it was non-200 then need to decode JSON and then need to check the error string of that json. What is the actual error state if decoding fails?

An alternative approach is to return an (text/plain) error string on all 400/500s with a key that is explained in documentation. And an appropriate response type (JSON) for complex responses.

[jcraigk]

@aspic From a client perspective in this situation, simplicity trumps performance on errors. Imo, every response from the RPC server should contain exactly two parts: (1) HTTP status code and (2) JSON payload. If non 2xx status code, error key should be present in JSON holding an English human readable error. That describes the entire contract I am suggesting here. Adding a variation on what is returned for certain status codes increases complexity to a degree that will confuse developers. Am I interpreting your reasoning here to be performance based or would there be another reason to avoid parsing JSON on error?

[codesoap]

I agree, that the complexity should be kept as low as possible and I think plain text would be simplest, because clients can then skip the parsing of JSON. I don't like the idea of returning some sort of "error key", which the client would have to translate into a meaningful error message. An HTTP response already contains a response code, which is a well machine readable "error key". The error message/response payload should be a more detailed description of the problem, targeted at humans, IMO.

[jcraigk]

@codesoap Sorry I think we have a miscommunication. See my original post. The error key of the JSON response would contain the meaningful human readable error message. Sending a different type of payload (plaintext vs JSON) depending on the status code is the complexity I am suggesting avoiding. Sending an HTTP status code indicating an error (any non-200 code) along with JSON body is extremely common in modern RESTful APIs. Can you site an example of what you are suggesting in a popular open source project?

[codesoap]

@jcraigk When criticizing the error keys in the payload, I was referring to this suggestion of @aspic, sorry for the confusion; although, maybe, I misunderstood this as well:

An alternative approach is to return an (text/plain) error string on all 400/500s with a key that is explained in documentation.

---

Returning a JSON payload on errors, which have a fixed form, as you suggest (e.g. {"error": "<human readable msg>"}), wouldn't be too bad, but I don't understand why the extra complexity of JSON should be added. If the JSON always contains just one value and the key is always the same, then there is no benefit over a plain text response. There is just one extra step - parsing the JSON - that clients have to perform.

As for examples:

1. The GitHub API returns JSON responses on errors, but I can understand that, since the JSON contains more than one key:

$ curl https://api.github.com/foo
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest"
}

2. As for non-JSON-respones: e.g. Go's http.FileServer and gitea return plain text messages on errors; although, I'll admit that it's not really a REST-API, I couldn't find a better example in a quick search:

$ curl https://gitea.com/foo/bar
Not found.

---

Overall, however, I don't have too strong of an opinion on the matter. I just wanted to put in my two cents on the topic.

Using special "error keys", as @aspic suggested (if I understood him correctly), would bother me a bit more, though, because then I would have to add an extra lookup table to my client.

[jcraigk]

@codesoap I understand and appreciate the basic argument of "isn't plaintext simpler than JSON in error situations" and if I were writing a highly performant application in C I would hope to receive something like that to speed up error handling. However, in the modern server/client REST architecture that has evolved over the last decade or so, servers always return JSON so that parsing is easy and consistent on the client side, regardless of status code.

I agree we should avoid another lookup for an error code, I was using the term "key" to refer to the name of a key in a JSON object. Its content would contain English sentences.

Consider this pseucode of a common pattern I see in Ruby JSON REST clients:

response = HTTP.fetch(url)
json = JSON.parse(response.body)
if response.status != 200
  puts "The error was: #{json[:error]}"
else
  # The happy path, using stuff from json[...]
end

Note line 2 where we parse the response body for JSON before we check the status code. Again, if I were writing highly performant code, I wouldn't do that. But here we're targeting simplicity and programmer happiness (can you tell I'm a Rubyist ;). In Ruby we have the "principle of least surprise," and in 2021 I'd be surprised if JSON parsing failed on a 500 error from a REST-ish API.

Also consider how different JSON parsing libraries will behave. Some will raise error if plaintext is given to it, some will return empty string. So as a programmer, I would have to consider two things: (1) conditions under which the server will or will not return JSON and (2) how my JSON parsing library handles errors. If JSON is always returned, both of those considerations go away.

[aspic]

@jcraigk thanks for the elaborate examples and argumentation! I think the most important thing here is to have an API that is consistent. Either direction here has its pros or cons.

I believe that limiting the ways a thing can fail will make it harder for anyone to make it fail. And in some languages, having to parse the error response is an extra step that might lead to more messy code / possibility of errors, which is my main concern with this.

However also notice that most "big public" APIs do describe errors as a JSON shape so they can't be that bad 😉 I see there's also a standard proposal for this which at least can be of some inspiration: https://datatracker.ietf.org/doc/html/rfc7807

[shryder]

I personally prefer having the response in JSON at all times, I kinda like this proposal: #2262
The way I do it with my personal APIs is something like this and it works well for me:

{
    success: false,
    error: {
        code: "ERROR_CODE",
        message: "Human readable description here"
    }
}

or

{
    success: true,
    data: {
        balance: "1000000000000000"
    }
}