[ETCM-448] json rpc - status code #836
Conversation
biandratti
changed the title
src/main/scala/io/iohk/ethereum/jsonrpc/server/http/JsonRpcHttpServer.scala
Show resolved
Hide resolved
|
Please update the description of the issue :) |
biandratti
changed the title
biandratti
added
BREAKS API
|
@conqeror Please verify it's working with wallet in case of errors (it should) |
|
Yes @kapke, the node and faucet using the same class, both have the same behavior. So, with this, we need to change faucet ui and wallet ui. |
enriquerodbe
left a comment
enriquerodbe
left a comment
There was a problem hiding this comment.
Looks good to me, only comments for readability
| private def handleResponse(f: Task[JsonRpcResponse]): Task[(StatusCode, JsonRpcResponse)] = f map { jsonRpcResponse => | ||
| jsonRpcResponse.error match { | ||
| case Some(JsonRpcError(error, _, _)) | ||
| if List(JsonRpcError.InvalidRequest.code, JsonRpcError.ParseError.code, JsonRpcError.InvalidParams().code) |
There was a problem hiding this comment.
For readability, this List of errors could be extracted to a constant at the class level or companion object
|
|
||
| postRequest ~> Route.seal(mockJsonRpcHttpServer.route) ~> check { | ||
| status shouldEqual StatusCodes.OK | ||
| responseAs[String] shouldEqual |
There was a problem hiding this comment.
What do you think if instead of comparing Strings (which in case of an error forces the developer to compare the whole string to find the difference), we parse the response as Json and assert each attribute individually?
|
@biandratti I meant - if faucet's rpc client handles errors from mantis node properly? IIRC faucet is being run as a separate process communicating with Mantis over RPC. |
|
@kapke @biandratti Regarding mantis wallet - let me know when this gets merged and I will test it. I don't expect that it will require immediate changes, but it should enable us to handle errors more easily (...no more Internal JSON-RPC error everywhere :) |
Good question @kapke |
Description
Today for every error in json rpc we return status code 200.
As per the 2.0 spec, there is no reason for using anything but 200 http status code whenever we are doing JSON-RPC over HTTP.
Also, returning 400 for valid JSON-RPC requests doesn't help when integrating with JSON-RPC clients.
Proposed Solution
Any valid JSON-RPC request should generate a JSON-RPC response, the HTTP status code should be 200. No matter if the response is a success or error response.
Calling a method that hasn't been enabled or a method that doesn't exist are valid JSON-RPC requests, therefore the expected response is a JSON-RPC error and the status code 200.
Any invalid JSON-RPC request (malformed json, etc) should generate a 400 status code.
So in this way we have status code 400 for json rpc error: [-32600, -32602, -32700]. In other cases we have status code 200.
Important Changes Introduced
Possibly requires change of integration with faucet ui and wallet.