chore(sequencer): migrate from anyhow::Result to eyre::Result#1387
Merged
ethanoroshiba merged 19 commits intomainfrom Sep 13, 2024
Merged
chore(sequencer): migrate from anyhow::Result to eyre::Result#1387ethanoroshiba merged 19 commits intomainfrom
anyhow::Result to eyre::Result#1387ethanoroshiba merged 19 commits intomainfrom
Conversation
SuperFluffy
suggested changes
Aug 22, 2024
Contributor
There was a problem hiding this comment.
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
Fraser999
reviewed
Sep 11, 2024
Fraser999
approved these changes
Sep 11, 2024
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
approved these changes
Sep 13, 2024
Contributor
SuperFluffy
left a comment
There was a problem hiding this comment.
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.
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
reviewed
Sep 13, 2024
SuperFluffy
reviewed
Sep 13, 2024
steezeburger
added a commit
that referenced
this pull request
Sep 23, 2024
* 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)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
## 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
sgranfield4403-3
added a commit
to sgranfield4403-3/astria
that referenced
this pull request
Oct 2, 2025
## Summary Added `astria_eyre::install()` call to `main.rs`. ## Background astriaorg/astria#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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrate all instances of
anyhow::Resulttoeyre::Result.Background
Sequencer was using
anyhow::Result, which provides an unhelpfulDisplayimpl 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::Resulttoeyre::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
eyreandanyhowsource chains are maintained when converting between one another.Related Issues
closes #1386