-
Notifications
You must be signed in to change notification settings - Fork 228
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
Turn non-200 HTTP responses into an error instead of trying to parse the body as a JSON-RPC response #1361
Conversation
7b3ae01
to
bf9eead
Compare
Conditional compilation needs to be fixed for wasm, otherwise looks fine. |
I will do this after we merge #1362 |
…the body as a JSON-RPC response
009bd89
to
064e4de
Compare
When the `reqwest` feature is disabled, this now falls back to `NonZeroU16`, even though that variant is never actually created. A better solution would be to feature-guard the error variant itself, but thats not supported by `flex-error` at the moment.
OK. I'm not a big fan of public enums with conditionally defined members, anyway, so the fallback approach is better. If the enum was private, that would be more disciplined, but still it causes a lot of fragility in match expressions. |
I've tested this from Hermes against a public Injective RPC node which was throttling requests, and it works as expected. We get back the HTTP error instead of trying parse the empty response body. |
In fact, why do we build RPC stuff for wasm? |
Because one can define a |
Closes: #1359
This PR checks for the status code to be 200 OK, if not it returns an error with the actual status code and the body of the response, but does not attempt to parse the response body as a JSON-RPC payload.
CometBFT will always set the status code to 200 OK and the content type to JSON for response which were successfully handled (which does mean that the JSON-RPC response itself is successful).
.changelog/