rsksmart / rskj

RSKj is a Java implementation of the Rootstock protocol.
https://rootstock.io
GNU Lesser General Public License v3.0
670 stars 269 forks source link

Incorrect HTTP status code when calling a non-existent JSON-RPC method? #876

Open alcuadrado opened 5 years ago

alcuadrado commented 5 years ago

If a non-existent JSON-RPC method is called RskJ responds with a 404 HTTP status.

While there's no mention of status codes in the JSON-RPC 2.0 specification, all the major Ethereum clients respond with a 200 in this case. I believe they always return 200.

This behavior is not necessarily wrong but leads to small discrepancies between RSK and Ethereum when handling JSON-RPC errors.

RskJ reproduction

I'm running RskJ using the official fat jar distribution: java -cp rskj-core-0.6.2-ORCHID-all.jar co.rsk.Start --regtest.

Sending a request like this gets me a 404:

curl -i localhost:4444 -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"non_existent","params":[],"id":1}'
HTTP/1.1 404 Not Found

{"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"method not found"}}

Expected result

An example with Geth being run as docker run -p 8545:8545 ethereum/client-go --dev --rpc --rpcapi "db,eth,net,web3,personal,shh" --rpccorsdomain "*" --rpcaddr 0.0.0.0

Sending a request like this gets me a 200:

curl -i localhost:8545 -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"non_existent","params":[],"id":1}'
HTTP/1.1 200 OK
Content-Type: application/json
Vary: Origin
Date: Sun, 09 Jun 2019 00:22:59 GMT
Content-Length: 117

{"jsonrpc":"2.0","id":1,"error":{"code":-32601,"message":"the method non_existent does not exist/is not available"}}
colltoaction commented 5 years ago

Thanks for your detailed report, we'll take a look! I would be curious of what kind of problem you ran into with this behavior, and if there's a workaround you can share if anyone bumps into the same problem.

Side note: it seems weird that not finding a method doesn't return a 404 Not Found in Geth.

alcuadrado commented 5 years ago

Thanks for your detailed report, we'll take a look! I would be curious of what kind of problem you ran into with this behavior, and if there's a workaround you can share if anyone bumps into the same problem.

I found this when running this test against RskJ. Some libraries return different errors when a method is not found when connected RskJ than when connected to other nodes.

In my case, that wasn't a big issue, and I just disabled the test for now. The situation may be different for users with more complex error handling logic.

Side note: it seems weird that not finding a method doesn't return a 404 Not Found in Geth.

I tried to get a 404 from Geth and Parity by sending the jsonrpc payload to a non-existent path and they still returned a 200. Ganache returns a 404 in that case.

colltoaction commented 5 years ago

Related: https://github.com/rsksmart/rskj/issues/408

colltoaction commented 5 years ago

The jsonrpc 2.0 specification doesn't mention the HTTP transport, so it seems to be left to implementors: https://www.jsonrpc.org/specification#error_object.

Historically, jsonrpc 1.0 returned 404: https://www.jsonrpc.org/historical/json-rpc-over-http.html#errors.

The following specification for 2.0 suggests returning status 200 for both responses and errors: https://www.simple-is-better.org/json-rpc/transport_http.html.

jsonrpc4j, the library we use, decided to follow the 1.0 specification: https://github.com/briandilley/jsonrpc4j/pull/73/files#diff-8c04bc130c55cd72d7d47e4799d80b78R186.