-
Notifications
You must be signed in to change notification settings - Fork 46
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
RPC error message is being swallowed #47
Comments
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. |
I am going to work on this |
That's nonsense:
|
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 |
You are right. |
I created an example on how to solve this. Using the bitcoin_rpc_errorr crate I created and my fork of 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 |
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? |
76dc3ae doc: add note for MSRV to README (Vladimir Fomene) 48ee09f test: propagate errors to users (Vladimir Fomene) a1fadb0 chore: remove ampersand on pinned dependencies line (Vladimir Fomene) fef4a77 Expose error message for Async client (Vladimir Fomene) 8d974ff Expose error message in addition to status in Blocking client (remix7531) Pull request description: Quick fix for #47 Top commit has no ACKs. Tree-SHA512: eeea5bebcb1d835015ef735e489eb85280e3cf36fa7e576dbe2d85970fffa5ec11c4065279ac5ba841dbb2f7cb29d64f8b12c6fa8eb2a1a8586b45bdbb2587e5
@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. |
Sounds great, thank you! |
@vladimirfomene I finally remembered to update to 269360f and it works great. Thanks! |
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
whilst the client gives us
which is much less useful.
The text was updated successfully, but these errors were encountered: