BM-1577: standardize anyhow error formatting

[austinabell]

Debugging a few errors that are much more tedious because context is lost. Thought all of these were migrated over but seems many still have missing context or using debug (which the latter is hard to grep for errors and verbose printing backtrace)

[Wollac]

The recommended idiomatic approach for thiserror is to not include any {0} etc in the message when #[from] is used, and instead use :# or chaining methods when the final error is logged or printed.
Otherwise, when the error message is unrolled automatically, this will result in very strange and long output. An example of this would be a main that returns an Anyhow::Error.

[austinabell]

This is specifically not what we want, because in that case the error would be multiline and hard to grep for and find with alerts. Perhaps I'm missing what you're saying, and I get that we may not be following idiomatic patterns exactly, but this seems ideal for what we actually want.

Playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=79f43464fdc64fbaa098fa32f2a85b97

[Wollac]

The recommended and idiomatic way here would be

    #[error("Transaction confirmation error")]
    TxnConfirmationError(#[source] anyhow::Error),

This then creates an error chain that can be followed to print the correct error message or to downcast the error, when an underlying type is needed.

One shortcoming of Rust in this regard is that println!("{:#}", ClientError::TxnConfirmationError(...)) does not always print the error chain in one line. Therefore, when the final outer error is printed, it must be wrapped in an anyhow (usually automatic since an anyhow::Result is usually returned) or a Display implementation must be used that follows the chain must be used.

Using #[error("Transaction confirmation error: {0:#}")] will lead to incorrect output when something follows the error chain to print the message. This can happen with anyhow or other things, such as logging frameworks.
See this example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=2ae4ba562c19bd0ed1c7224920c9e43a

[Wollac]

    #[error("Transaction confirmation error: {0:#}")]
    TxnConfirmationError(anyhow::Error),

Without 'from' or 'source' would also work, as this error never creates a chain. However, you would lose features that rely on the canonical std::error::Error chaining.

[Wollac]

By the way, I am absolutely fine with the proposed changes if they are the easiest way to obtain usable log messages at this stage. I just wanted to point out that this is not recommended and could backfire.

[austinabell]

Yes, just for expedience rather than switching all error printing to be {:#} and removing the #[from] errors printing semantics. This PR is the most minimal change. If approved, I will open up an issue to switch it to be handled correctly in the future, where the :# formatting is not applied on the error type but on all the logs.

The worst case here is that an error context is printed twice if the error is printed with :# which is much less bad than error context not being included in the log for the time being (imo).