Skip to content

StdResult and StdError as anyhow alias #1162

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

Merged
merged 10 commits into from
Aug 17, 2023
Merged

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Aug 17, 2023

Content

This heart of this PR is to change what StdResult and StdError are aliasing to, replacing std types with the one from anyhow:

 /// Generic error type
-pub type StdError = Box<dyn Error + Sync + Send>;
+pub type StdError = anyhow::Error;
 
 /// Generic result type
-pub type StdResult<T> = std::result::Result<T, StdError>;
+pub type StdResult<T> = anyhow::Result<T, StdError>;

Most of the other changes are related to adapting the code to anyhow results.

This PR also:

  • Changes the return types of the main function of our three binaries (aggregator, signer, client) to the "new" StdResult.
    This is an improvement since when main function yield an anyhow error, it is printed out nicely (including the backtrace, see this rust playground).
  • It unify the usages of boxed errors, replacing most usage of local error/result types to a StdError or StdResult.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Comments

The unused dependency to lazy_static from mithril-common was removed. It was only used to access the .deref() method something that the standard rust library already provide.

Issue(s)

Relates to #798

@Alenar Alenar requested a review from dlachaume August 17, 2023 13:14
@github-actions
Copy link

github-actions bot commented Aug 17, 2023

Test Results

    3 files  ±0    17 suites  ±0   6m 35s ⏱️ +6s
666 tests ±0  666 ✔️ ±0  0 💤 ±0  0 ±0 
722 runs  ±0  722 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit bbf4ba9. ± Comparison against base commit c07f297.

♻️ This comment has been updated with latest results.

@Alenar Alenar force-pushed the djo/798/stdresult-as-anyhow-alias branch 4 times, most recently from b84f429 to 0daf3df Compare August 17, 2023 14:35
Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Alenar Alenar force-pushed the djo/798/stdresult-as-anyhow-alias branch from 0daf3df to bbf4ba9 Compare August 17, 2023 15:04
@Alenar Alenar temporarily deployed to testing-preview August 17, 2023 15:20 — with GitHub Actions Inactive
@Alenar Alenar merged commit 894bf6b into main Aug 17, 2023
@Alenar Alenar deleted the djo/798/stdresult-as-anyhow-alias branch August 17, 2023 15:32
@dlachaume dlachaume mentioned this pull request Sep 18, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants