Return appropriate http codes #793
Conversation
Signed-off-by: Maksim Dimitrov <dimitrov.maksim@gmail.com>
Signed-off-by: Maksim Dimitrov <dimitrov.maksim@gmail.com>
Codecov ReportBase: 72.58% // Head: 72.58% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #793 +/- ##
=======================================
Coverage 72.58% 72.58%
=======================================
Files 16 16
Lines 1262 1262
Branches 233 233
=======================================
Hits 916 916
Misses 284 284
Partials 62 62 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: Maksim Dimitrov <dimitrov.maksim@gmail.com>
|
Kudos, SonarCloud Quality Gate passed!
|
dimitrovmaksim
requested review from
natanasow,
georgi-l95,
Ivo-Yankov and
lukelee-sl
and removed request for
georgi-l95
dimitrovmaksim
changed the title
| @@ -99,18 +100,21 @@ export default class KoaJsonRpc { | |||
| ctx.request.method !== 'POST' | |||
| ) { | |||
| ctx.body = jsonResp(body.id || null, new InvalidRequest(), undefined); | |||
| ctx.status = 400; | |||
There was a problem hiding this comment.
does it make sense to add the status as an optional(?) forth parameter to the jsonResp method?
There was a problem hiding this comment.
I'm not sure I understand
There was a problem hiding this comment.
Ah, I see my suggestion does not make sense. My mistake.
But I do have another question. What happens when calling jsonResp and it throws an exception?
There was a problem hiding this comment.
Hm, good question. It seems like it's not being handled by us anywhere. I'm assuming it will return an internal error response but it won't be in json-rpc format. cc: @georgi-l95
There was a problem hiding this comment.
Made jsonResp() to always throw an error and got this:
curl localhost:7546 -X POST -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","method":"nonExistingMethod","params": [],"id":1}'
Internal Server Error
Description:
Currently we return 200 status code in most cases. This PR updates the relay to return the appropriate codes:
InternalError => 500
InvalidParams (JsonRpc request params) => 400
MethodNotFound or UnsupportedMethod => 400
RateLimit => 429
JsonRpcError => 500 for InternalError, 400 for the rest
Related issue(s):
Fixes #751
Notes for reviewer:
Checklist