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

discrepancy in http return codes for RPC server between C# node and Go node on error #3586

Closed
ixje opened this issue Sep 12, 2024 · 2 comments · Fixed by #3665
Closed

discrepancy in http return codes for RPC server between C# node and Go node on error #3586

ixje opened this issue Sep 12, 2024 · 2 comments · Fixed by #3665
Labels
bug Something isn't working I2 Regular impact rpc RPC server and client S4 Routine U4 Nothing urgent
Milestone

Comments

@ixje
Copy link
Contributor

ixje commented Sep 12, 2024

Current Behaviour

curl -i --location 'https://rpc10.n3.nspcc.ru:10331' --header 'Content-Type: text/plain' --data '{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "getapplicationlog",
  "params": ["0xacaa380861ecbd458a924256f3ab798d89263d43d3340014002eccab2542da31"]
}'
**HTTP/2 422**

Expected Behavior

status code 200 like C#

curl -i --location 'http://seed1.neo.org:10332' --header 'Content-Type: text/plain' --data '{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "getapplicationlog",
  "params": ["0xacaa380861ecbd458a924256f3ab798d89263d43d3340014002eccab2542da31"]
}'
**HTTP/1.1 200 OK**

Similarly an invalid invokescript returns 200 on C# and 400 with neo-go.

Possible Solution

change status code to 200

From https://www.rfc-editor.org/rfc/rfc4918#section-11.2

The 422 (Unprocessable Content) status code indicates that the server understands the content type of the request content (hence a 415 (Unsupported Media Type) status code is inappropriate), and the syntax of the request content is correct, but it was unable to process the contained instructions.

I assume the reason came from the bold part, but is that really fair? It actually is able to process the instructions in the request, its just that it couldn't find what was asked for. This feels like throwing a 422 just because you called getnep17balances expecting there to be some but there were none

Your Environment

0.106.3

@ixje ixje added bug Something isn't working U2 Seriously planned labels Sep 12, 2024
@ixje ixje changed the title descrepancy in http return codes for RPC server between C# node and Go node on error discrepancy in http return codes for RPC server between C# node and Go node on error Sep 12, 2024
@AnnaShaleva AnnaShaleva added I2 Regular impact rpc RPC server and client U4 Nothing urgent S4 Routine and removed U2 Seriously planned labels Sep 12, 2024
@AnnaShaleva AnnaShaleva added this to the v0.106.4 milestone Sep 12, 2024
@roman-khimov
Copy link
Member

This behavior goes back to 19a430b. Not that we've never seen it, it was always suspicious, but it worked well and I'm still not sure what can be affected if we're to change it. Also, JSON-RPC itself doesn't specify how to use it over HTTP.

Let's review other codes as well while we're here and try to pick the best practices. This spec is outdated, but still has some ideas for HTTP codes: https://www.jsonrpc.org/historical/json-rpc-over-http.html, also https://stackoverflow.com/questions/76860816/json-rpc-v2-api-http-layer-what-are-the-appropriate-http-layer-status-codes-fo.

@ixje
Copy link
Contributor Author

ixje commented Sep 12, 2024

from the comment on the last post

The gist of both of these is that non-200 codes are for indicating a problem with the HTTP transport, and not for errors that are related to the semantics of the method call.

That seems to be inline with what C# is doing. Most HTTP request libraries also suggest some flow of

  1. do request
  2. check status code of response
  3. process response if status OK

The way neo-go works now means I need to remove the 2nd step (which sometimes is even done implicitly by libraries) or I can't get to the point that processes the actual error from the JSON-RPC error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working I2 Regular impact rpc RPC server and client S4 Routine U4 Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants