Skip to content

BM-1577: standardize anyhow error formatting #1110

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

BM-1577: standardize anyhow error formatting #1110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/boundless-market/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ pub enum ClientError {
#[error("Error building RequestBuilder {0}")]
BuilderError(#[from] StandardRequestBuilderBuilderError),
/// General error
#[error("Error {0}")]
Copy link
Contributor

@Wollac Wollac Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@austinabell austinabell Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@Wollac Wollac Sep 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@Wollac Wollac Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    #[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.

Copy link
Contributor

@Wollac Wollac Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@austinabell austinabell Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#[error("Error {0:#}")]
Error(#[from] anyhow::Error),
}

Expand Down
4 changes: 2 additions & 2 deletions crates/boundless-market/src/contracts/boundless_market.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum MarketError {
TxnError(#[from] TxnErr),

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

/// Request not fulfilled.
Expand Down Expand Up @@ -104,7 +104,7 @@ pub enum MarketError {
LogNotEmitted(B256, anyhow::Error),

/// General market error.
#[error("Other error: {0:?}")]
#[error("Other error: {0:#}")]
Error(#[from] anyhow::Error),

/// Timeout reached.
Expand Down
12 changes: 6 additions & 6 deletions crates/boundless-market/src/contracts/bytecode.rs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/boundless-market/src/storage/file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub enum TempFileStorageProviderError {
UrlParse(#[from] url::ParseError),

/// Error type for other errors.
#[error("{0}")]
#[error("{0:#}")]
Other(#[from] anyhow::Error),
}

Expand Down
2 changes: 1 addition & 1 deletion crates/boundless-market/src/storage/pinata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub enum PinataStorageProviderError {
Config(String),

/// Error type for other errors.
#[error("{0}")]
#[error("{0:#}")]
Other(#[from] anyhow::Error),
}

Expand Down
2 changes: 1 addition & 1 deletion crates/boundless-market/src/storage/s3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub enum S3StorageProviderError {
UrlParseError(#[from] ParseError),

/// Error type for other errors.
#[error("{0}")]
#[error("{0:#}")]
Other(#[from] anyhow::Error),
}

Expand Down
4 changes: 2 additions & 2 deletions crates/broker/src/aggregator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ use tokio_util::sync::CancellationToken;

#[derive(Error)]
pub enum AggregatorErr {
#[error("{code} Compression error: {0}", code = self.code())]
#[error("{code} Compression error: {0:#}", code = self.code())]
CompressionErr(crate::provers::ProverError),
#[error("{code} Unexpected error: {0:?}", code = self.code())]
#[error("{code} Unexpected error: {0:#}", code = self.code())]
UnexpectedErr(#[from] anyhow::Error),
}

Expand Down
4 changes: 2 additions & 2 deletions crates/broker/src/chain_monitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ use crate::{

#[derive(Error)]
pub enum ChainMonitorErr {
#[error("{code} RPC error: {0:?}", code = self.code())]
#[error("{code} RPC error: {0:#}", code = self.code())]
RpcErr(anyhow::Error),
#[error("{code} Unexpected error: {0:?}", code = self.code())]
#[error("{code} Unexpected error: {0:#}", code = self.code())]
UnexpectedErr(#[from] anyhow::Error),
}

Expand Down
4 changes: 2 additions & 2 deletions crates/broker/src/order_monitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ pub enum OrderMonitorErr {
#[error("{code} Order already locked", code = self.code())]
AlreadyLocked,

#[error("{code} RPC error: {0:?}", code = self.code())]
#[error("{code} RPC error: {0:#}", code = self.code())]
RpcErr(anyhow::Error),

#[error("{code} Unexpected error: {0:?}", code = self.code())]
#[error("{code} Unexpected error: {0:#}", code = self.code())]
UnexpectedError(#[from] anyhow::Error),
}

Expand Down
2 changes: 1 addition & 1 deletion crates/broker/src/provers/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub enum ProverError {
#[error("{code} Prover internal error: {0}", code = self.code())]
ProverInternalError(String),

#[error("{code} {0:?}", code = self.code())]
#[error("{code} Unexpected prover error: {0:#}", code = self.code())]
UnexpectedError(#[from] anyhow::Error),
}

Expand Down
2 changes: 1 addition & 1 deletion crates/indexer/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub enum ServiceError {
#[error("Event query error: {0}")]
EventQueryError(#[from] alloy::contract::Error),

#[error("Error: {0}")]
#[error("Error: {0:#}")]
Error(#[from] anyhow::Error),

#[error("Maximum retries reached")]
Expand Down
Loading