-
Notifications
You must be signed in to change notification settings - Fork 87
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
chore(sequencer): migrate from anyhow::Result
to eyre::Result
#1387
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach breaks the error source chain, see proof below. (on a side note: please base the instrumentation fix in ENG-670 on this change from anyhow to eyre, not the other way round)
We need to find a way to turn anyhow:Error
into a Box<dyn std::error::Error>
and then wrap that in eyre
(which sounds easier then it is in practice): https://docs.rs/anyhow/latest/anyhow/struct.Error.html#impl-From%3CError%3E-for-Box%3Cdyn+Error+%2B+Send+%2B+Sync%3E
Proof:
eyre-wrapped anyhow error:
Err(wrapped
Caused by:
level 2
Location:
src/main.rs:8:55)
eyre-wrapped anyhow error: broken source chain
0: wrapped
1: level 2
eyre error:
Err(wrapped
Caused by:
0: level 2
1: level 1
2: level 0
Location:
src/main.rs:6:33)
eyre-only error: functioning source chain
0: wrapped
1: level 2
2: level 1
3: level 0
Code used to generate this:
fn main() {
let anyhow_err = Err::<(), _>(anyhow!("level 0").context("level 1").context("level 2"));
let eyre_err = Err::<(), _>(eyre!("level 0").wrap_err("level 1").wrap_err("level 2"));
let wrapped_anyhow_err = anyhow_err.map_err(|err| eyre!(err).wrap_err("wrapped"));
println!("eyre-wrapped anyhow error:\n{wrapped_anyhow_err:?}\n");
println!("eyre-wrapped anyhow error: broken source chain");
for (i, source) in wrapped_anyhow_err.unwrap_err().chain().enumerate() {
println!("{i}: {source}");
}
let wrapped_eyre_err = eyre_err.wrap_err("wrapped");
println!("eyre error:\n{wrapped_eyre_err:?}");
println!("eyre-only error: functioning source chain");
for (i, source) in wrapped_eyre_err.unwrap_err().chain().enumerate() {
println!("{i}: {source}");
}
2d64bb4
to
623adc8
Compare
623adc8
to
ef46741
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for going through the entire codebase and making these changes! I think we should go ahead and merge this.
I have left a few comments. Please address them before merging.
* main: feat(conductor): implement restart logic (#1463) fix: ignore `RUSTSEC-2024-0370` (#1483) fix, refactor(sequencer): refactor ics20 logic (#1495) fix(ci): use commit SHA instead of PR number preview-env images (#1501) chore(bridge-withdrawer): pass GRPC and CometBFT clients to consumers directly (#1510) fix(sequencer): Fix incorrect error message from BridgeUnlock actions (#1505) fix(bridge-contracts): fix memo transaction hash encoding (#1428) fix: build docker when workflow explicitly includes component (#1498) chore(sequencer): migrate from `anyhow::Result` to `eyre::Result` (#1387) fix(ci): typo for required field in sequencer preview-env (#1500) feat(ci): provide demo/preview environments (#1406)
## Summary Added `astria_eyre::install()` call to `main.rs`. ## Background #1387 switched the sequencer from `anyhow` to `eyre` usage, but did not install the `astria-eyre` error hook. ## Changes Added `astria_eyre::install()` call to `main.rs`. ## Testing Passing all tests ## Related Issues closes #1551
Summary
Migrate all instances of
anyhow::Result
toeyre::Result
.Background
Sequencer was using
anyhow::Result
, which provides an unhelpfulDisplay
impl and contrasts our error handling in the rest of the codebase. This change is to flush out our error handling in the sequencer, except for those parts which necessitate usinganyhow
.Changes
cargo.toml
.anyhow::Result
toeyre::Result
, except for those that touch cnidarium directly.anyhow_to_eyre()
andeyre_to_anyhow()
helper functions for moving between the two without breaking the source chain.Testing
Added unit tests to ensure
eyre
andanyhow
source chains are maintained when converting between one another.Related Issues
closes #1386