rust-esplora-client icon indicating copy to clipboard operation
rust-esplora-client copied to clipboard

RPC error message is being swallowed

Open luckysori opened this issue 1 year ago • 10 comments

When calling something like AsyncClient::broadcast, if we get an RPC error back from the electrum server, we immediately convert the response into a generic one based on the status. This means that we lose the context of the error message, which makes debugging harder as one has to check the logs of the electrum server to figure out exactly what went wrong.

For instance, electrum might report

WARN - HttpError(400, "sendrawtransaction RPC error: {\"code\":-26,\"message\":\"non-mandatory-script-verify-flag (Signature must be zero for failed CHECK(MULTI)SIG operation)\"}")

whilst the client gives us

Esplora client error: Reqwest(reqwest::Error { kind: Status(400), url: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(3000), path: "/tx", query: None, fragment: None } })

which is much less useful.

luckysori avatar May 05 '23 18:05 luckysori

I solved this problem for myself once but never managed to upstream it. See: https://github.com/LLFourn/bdk/blob/9e6150717d392538ea8b514de7c6af9c3ea69e1f/src/blockchain/mod.rs#L285

This parsing of RPC error messages could be rolled into its own crate I think.

LLFourn avatar May 08 '23 03:05 LLFourn

I am going to work on this

remix7531 avatar May 09 '23 12:05 remix7531

That's nonsense:

rust-esplora-client is basically a wrapper around the Esplora HTTP API. This is different from an Electrum RPC interface. While an Electrum server provides detailed information about the reasons for an invalid transaction, Esplora merely returns a status code 400, leaving the rust-esplora-client unaware of the specific cause. Only rust-electrum-client is capable of delivering detailed error messages.

remix7531 avatar May 16 '23 18:05 remix7531

Isn't it the case that we can parse the error response returned by the HTTP API instead of solely considering the response status? I thought the response included a message field describing the error.

luckysori avatar May 16 '23 18:05 luckysori

You are right.

remix7531 avatar May 16 '23 19:05 remix7531

I created an example on how to solve this. Using the bitcoin_rpc_errorr crate I created and my fork of rust-esplora-client one can do something link this:

 let builder = esplora_client::Builder::new("https://blockstream.info/testnet/api");
 let client_esplora = builder.build_blocking().unwrap();

 let txid: Txid = Txid::from_str("0a7c8109b0e31bad63be1c1561837b44cb40575d48b03e66a33abd91b085629f").unwrap();
 let tx: Transaction = client_esplora.get_tx_no_opt(&txid).unwrap();

 let result = client_esplora.broadcast(&tx);
 println!("{result:?}");

which prints Err(RPCErrorCode(RPC_VERIFY_ALREADY_IN_CHAIN))

remix7531 avatar Jun 14 '23 15:06 remix7531

Hi @remix75 ! Good progress on the error crate work. I will suggest we go for a quick solution for now. Can we have our esplora errors have a cause field with a string of the error coming from electrum server. Can you implement this in a quick PR?

vladimirfomene avatar Jul 08 '23 16:07 vladimirfomene

@luckysori we have merged a quick fix to this issue where we expose the errors we get from the Esplora backend rather than swallowing them.

vladimirfomene avatar Oct 02 '23 09:10 vladimirfomene

@luckysori we have merge a quick fix to this issue where we expose the errors we get from the Esplora backend rather than swallowing them.

Sounds great, thank you!

luckysori avatar Oct 02 '23 13:10 luckysori

@vladimirfomene I finally remembered to update to 269360f and it works great. Thanks!

luckysori avatar Feb 26 '24 12:02 luckysori