nspcc-dev / neo-go

Go Node and SDK for the Neo blockchain
MIT License
123 stars 79 forks source link

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

Closed AliceInHunterland closed 5 days ago

AliceInHunterland commented 1 week ago

Close #3586

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AliceInHunterland commented 5 days ago

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 commented 5 days ago

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"
    }
  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.

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