informalsystems / tendermint-rs

Client libraries for Tendermint/CometBFT in Rust!
Apache License 2.0
588 stars 215 forks source link

Turn non-200 HTTP responses into an error instead of trying to parse the body as a JSON-RPC response #1361

Closed romac closed 9 months ago

romac commented 9 months ago

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).

mzabaluev commented 9 months ago

Conditional compilation needs to be fixed for wasm, otherwise looks fine.

romac commented 9 months ago

I will do this after we merge #1362

romac commented 9 months ago

Not the cleanest solution (see the commit message) but it's pretty much what we do already for source errors, so let's stick with that until we eventually move away from flex-error.

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.

romac commented 9 months ago

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.

mzabaluev commented 9 months ago

Conditional compilation needs to be fixed for wasm, otherwise looks fine.

In fact, why do we build RPC stuff for wasm?

romac commented 9 months ago

In fact, why do we build RPC stuff for wasm?

Because one can define a Client implementation which uses eg. the browser's fetch method and then compile all that to WASM.