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

rpcsrv: align HTTP error code with C# implementation #3665

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

AliceInHunterland
Copy link
Contributor

Close #3586

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.07%. Comparing base (66fbcb2) to head (0afd9ac).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3665      +/-   ##
==========================================
- Coverage   83.08%   83.07%   -0.02%     
==========================================
  Files         334      334              
  Lines       46590    46573      -17     
==========================================
- Hits        38710    38690      -20     
- Misses       6310     6313       +3     
  Partials     1570     1570              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM at this stage, waiting for @AliceInHunterland to finalize reference implementation check against other possible status codes.

@AliceInHunterland
Copy link
Contributor Author

I found only one place in C# where the HTTP status code is overwritten internal bool CheckAuth(HttpContext context) (https://github.com/neo-project/neo/blob/233d0b03501a1af5e6a140721a67bb37de84ecc9/src/Plugins/RpcServer/RpcServer.cs#L67-L76 ) to 401, others application-level errors will be with http.StatusOK. So now we have 400, 405 and 500 in getHTTPCodeForError, but none is explicitly set in C#. Please @AnnaShaleva correct me if I'm wrong.

@AnnaShaleva
Copy link
Member

AnnaShaleva commented Nov 12, 2024

I found only one place in C# where the HTTP status code is overwritten internal bool CheckAuth

This method is not applicable to our RPC server due to the different wallet model, hence we don't need to port the status code of this method.

but none is explicitly set in C#. Please @AnnaShaleva correct me if I'm wrong.

Yep, that's true. Here's a simple check, I sent several requests to C# node:

  1. 405 code (unknown method, neorpc.MethodNotFoundCode). The resulting HTTP code returned by C# server is 200 OK:
anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ curl -v -d '{ "jsonrpc": "2.0", "id": 1, "method": "unknown-method", "params": [] }' http://seed1t5.neo.org:20332 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 34.133.235.69:20332...
* Connected to seed1t5.neo.org (34.133.235.69) port 20332 (#0)
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0> POST / HTTP/1.1
> Host: seed1t5.neo.org:20332
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 71
> Content-Type: application/x-www-form-urlencoded
> 
} [71 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Tue, 12 Nov 2024 13:45:51 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
< 
{ [461 bytes data]
100   520    0   449  100    71    729    115 --:--:-- --:--:-- --:--:--   844
* Connection #0 to host seed1t5.neo.org left intact
{
   "error" : {
      "code" : -32601,
      "data" : "   at Neo.Plugins.Result.True_Or(Boolean result, RpcError err)\n   at Neo.Plugins.RpcServer.ProcessRequestAsync(HttpContext context, JObject request)",
      "message" : "Method not found - The method 'unknown-method' doesn't exists. -    at Neo.Plugins.Result.True_Or(Boolean result, RpcError err)\n   at Neo.Plugins.RpcServer.ProcessRequestAsync(HttpContext context, JObject request)"
   },
   "id" : 1,
   "jsonrpc" : "2.0"
}
  1. 400 code (malformed request, neorpc.BadRequestCode). The resulting HTTP code returned by C# server is 200 OK:
anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ curl -v -d '{ "jsonrpc": "2.0", "id": 1, "method": "getapplicationlog", "params": ["] }' http://seed1t5.neo.org:20332 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 34.133.235.69:20332...
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0* Connected to seed1t5.neo.org (34.133.235.69) port 20332 (#0)
> POST / HTTP/1.1
> Host: seed1t5.neo.org:20332
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 75
> Content-Type: application/x-www-form-urlencoded
> 
} [75 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Tue, 12 Nov 2024 13:50:12 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
< 
{ [86 bytes data]
100   150    0    75  100    75     84     84 --:--:-- --:--:-- --:--:--   169
* Connection #0 to host seed1t5.neo.org left intact
{
   "error" : {
      "code" : -32700,
      "message" : "Bad request"
   },
   "id" : null,
   "jsonrpc" : "2.0"
}
  1. 500 code (internal server error, neorpc.InternalServerErrorCode). It's difficult to reproduce this error on real network for C# server, but the resulting code is expected to be the same, 200 OK.

Hence, we don't really need these additional HTTP codes on the NeoGo server side.

We had 400, 405 and 500 in getHTTPCodeForError, but none is explicitly
set in C#.

1. **405 code** (unknown method, `neorpc.MethodNotFoundCode`). The
resulting HTTP code returned by C# server is 200 OK:
```
anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ curl -v -d '{
"jsonrpc": "2.0", "id": 1, "method": "unknown-method", "params": [] }'
http://seed1t5.neo.org:20332 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time
  Current
                                 Dload  Upload   Total   Spent    Left
                                 Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--
       0*   Trying 34.133.235.69:20332...
* Connected to seed1t5.neo.org (34.133.235.69) port 20332 (#0)
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--
       0> POST / HTTP/1.1
> Host: seed1t5.neo.org:20332
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 71
> Content-Type: application/x-www-form-urlencoded
>
} [71 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Tue, 12 Nov 2024 13:45:51 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
<
{ [461 bytes data]
100   520    0   449  100    71    729    115 --:--:-- --:--:-- --:--:--
   844
* Connection #0 to host seed1t5.neo.org left intact
{
   "error" : {
      "code" : -32601,
      "data" : "   at Neo.Plugins.Result.True_Or(Boolean result,
      RpcError err)\n   at Neo.Plugins.RpcServer.ProcessRequestAsync
      (HttpContext context, JObject request)",
      "message" : "Method not found - The method 'unknown-method'
      doesn't exists. -    at Neo.Plugins.Result.True_Or(Boolean result,
       RpcError err)\n   at Neo.Plugins.RpcServer.ProcessRequestAsync
       (HttpContext context, JObject request)"
   },
   "id" : 1,
   "jsonrpc" : "2.0"
}
```
2. **400 code** (malformed request, `neorpc.BadRequestCode`). The
resulting HTTP code returned by C# server is 200 OK:
```
anna@kiwi:~/Documents/GitProjects/bane-labs/go-ethereum$ curl -v -d '{
"jsonrpc": "2.0", "id": 1, "method": "getapplicationlog", "params": ["]
}' http://seed1t5.neo.org:20332 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time
  Current
                                 Dload  Upload   Total   Spent    Left
                                 Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--
       0*   Trying 34.133.235.69:20332...
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--
       0* Connected to seed1t5.neo.org (34.133.235.69) port 20332 (#0)
> POST / HTTP/1.1
> Host: seed1t5.neo.org:20332
> User-Agent: curl/7.81.0
> Accept: */*
> Content-Length: 75
> Content-Type: application/x-www-form-urlencoded
>
} [75 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: application/json
< Date: Tue, 12 Nov 2024 13:50:12 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
<
{ [86 bytes data]
100   150    0    75  100    75     84     84 --:--:-- --:--:-- --:--:--
   169
* Connection #0 to host seed1t5.neo.org left intact
{
   "error" : {
      "code" : -32700,
      "message" : "Bad request"
   },
   "id" : null,
   "jsonrpc" : "2.0"
}
```
3. **500 code** (internal server error, `neorpc
.InternalServerErrorCode`). It's difficult to reproduce this error on
real network for C# server, but the resulting code is expected to be the
 same, 200 OK.

Close #3586

Signed-off-by: Ekaterina Pavlova <[email protected]>
@AnnaShaleva AnnaShaleva merged commit dda2caf into master Nov 12, 2024
33 of 34 checks passed
@AnnaShaleva AnnaShaleva deleted the http-code-status-ok branch November 12, 2024 14:19
@AnnaShaleva AnnaShaleva changed the title rpcsrv: align default error code with C# implementation rpcsrv: align HTTP error code with C# implementation Nov 12, 2024
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

Successfully merging this pull request may close these issues.

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