HTTP client should not try to parse body of response with non-2XX status code

[xoac]

What went wrong?

Concurrent request to tx_search fails with value: serde parse error

    Finished dev [unoptimized + debuginfo] target(s) in 6.36s
     Running `target/debug/tendermint_tx_search`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: serde parse error

Caused by:
    expected value at line 1 column 1

Steps to reproduce

Cargo.toml

```toml [package] name = "tendermint_tx_search" version = "0.1.0" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] # async tokio = { version = "1.13.1", features = [ "rt-multi-thread", "time", "sync", "macros", ] } # tendermint tendermint-rpc = { version = "0.33", features = ["http-client"] } tendermint = { version = "0.33" } ```

main.rs (not working)

```rs use tendermint_rpc::{client::Client, query::Query, Order}; #[tokio::main] async fn main() { let mut c = tendermint_rpc::client::HttpClient::new("https://juno-rpc.icycro.org").unwrap(); c.set_compat_mode(tendermint_rpc::client::CompatMode::V0_37); let query = Query::default() .and_eq( "coin_spent.spender", "juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x", ) .and_lt("tx.height", 10_187_764); let query2 = Query::default() .and_eq( "coin_received.receiver", "juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x", ) .and_lt("tx.height", 10_187_764); let r_task = c.tx_search(query, false, 1, 12, tendermint_rpc::Order::Descending); let r2_taks = c.tx_search(query2, false, 1, 15, Order::Descending); let (r, r2) = tokio::try_join!(r_task, r2_taks).unwrap(); println!("{:?}", r); } ```

main.rs(working)

```rs use tendermint_rpc::{client::Client, query::Query, Order}; #[tokio::main] async fn main() { let mut c = tendermint_rpc::client::HttpClient::new("https://juno-rpc.icycro.org").unwrap(); c.set_compat_mode(tendermint_rpc::client::CompatMode::V0_37); let query = Query::default() .and_eq( "coin_spent.spender", "juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x", ) .and_lt("tx.height", 10_187_764); let query2 = Query::default() .and_eq( "coin_received.receiver", "juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x", ) .and_lt("tx.height", 10_187_764); let r_task = c.tx_search(query, false, 1, 12, tendermint_rpc::Order::Descending); let r2_taks = c.tx_search(query2, false, 1, 15, Order::Descending); let (r, r2) = (r_task.await.unwrap(), r2_taks.await.unwrap()); println!("{:?}", r); } ```

diff between `not working` and `working` implementation:

```rs // not working main let (r, r2) = tokio::try_join!(r_task, r2_taks).unwrap(); // working main let (r, r2) = (r_task.await.unwrap(), r2_taks.await.unwrap()); ```

Definition of "done"

concurrent calls to HttpClient should working without any side effects

[romac]

If you add tracing-subscriber as a dependency

tracing-subscriber = { version = "0.3.17", features = ["env-filter", "fmt"] }

and install it within main()

#[tokio::main]
async fn main() {
    tracing_subscriber::fmt::init();

    // ... snip ...
}

and then run the executable with

$ RUST_LOG=tendermint_rpc=debug cargo run

then the HTTP client will output the outgoing requests it makes and their corresponding incoming responses:

     Running `/Users/romac/.cargo/target/debug/tendermint_tx_search`
2023-09-26T16:18:05.197402Z DEBUG tendermint_rpc::client::transport::http::sealed: Outgoing request: {
  "jsonrpc": "2.0",
  "id": "e7020abe-49db-42d2-af67-adfe0702ce08",
  "method": "tx_search",
  "params": {
    "query": "coin_spent.spender = 'juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x' AND tx.height < 10187764",
    "prove": false,
    "page": "1",
    "per_page": "12",
    "order_by": "desc"
  }
}
2023-09-26T16:18:05.198328Z DEBUG tendermint_rpc::client::transport::http::sealed: Outgoing request: {
  "jsonrpc": "2.0",
  "id": "1680f0ce-8a79-481c-a44d-f656b029b3e6",
  "method": "tx_search",
  "params": {
    "query": "coin_received.receiver = 'juno185zc74a7w2pfxkv6d06t3kdja65tngjgg6223x' AND tx.height < 10187764",
    "prove": false,
    "page": "1",
    "per_page": "15",
    "order_by": "desc"
  }
}
2023-09-26T16:18:05.335952Z DEBUG tendermint_rpc::client::transport::http::sealed: Incoming response: Too Many Requests
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: serde parse error

On the second to last line, we see that the server responded with a plain text message that Too Many Requests instead of a valid JSON-RPC response.

Therefore, the issue lies with the server which appears to be doing rate limiting/throttling.

To solve this, you will have to use the machinery provided by Tokio to ensure you don't perform too many requests at once, or just not try to perform concurrent requests. There is nothing the HTTP client can do for you by default.

Hope that helps!

[romac]

I did a quick test, and by adding a (fairly hefty) delay the node will reliably answer both queries:

    let r_task = async {
        tokio::time::sleep(tokio::time::Duration::from_millis(750)).await;
        c.tx_search(query, false, 1, 12, tendermint_rpc::Order::Descending)
            .await
    };

[xoac]

Thanks for founding this. I was probably tired because I did RUST_LOG=tendermint_rpc=debug before reporting this but it probably got lost with my other logs.

---

Overall I think this should be handled better by tendermint_rpc::HttpClient. There is possibility to bypass this error (http error) properly https://docs.rs/tendermint-rpc/0.33.2/src/tendermint_rpc/error.rs.html#50-57 (you can return one of this http::Error and hyper::Error).

On stackoverflow I found links how json-rpc should be handled when transport layer is http: https://www.simple-is-better.org/json-rpc/transport_http.html

JSON-RPC 2.0 is sent over HTTP so if HTTP return error then the body shouldn't be parsed but error should be returned. discussion source: https://groups.google.com/g/json-rpc/c/2L_4zYIafGw/m/Oa_tEt-aAgAJ?pli=1

[romac]

That's a good point! Let me re-open the issue then.