-
Notifications
You must be signed in to change notification settings - Fork 286
Better error handling #466
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
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.
Really nice work there! I like the idea of context. They will help a lot with improving error handling/reporting
@o2sh Would you want my review now or when the PR is no longer a draft? |
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.
Left some suggestions. All of them are style preference, they shouldn't change the behavior.
One thing I noticed was that you used with_context(|| "foo")
a lot. Is there a reason to use that over context("foo")
?
Apparently, it results in better performance in an error-free scenario. |
Co-authored-by: Patrick Haller <[email protected]>
Co-authored-by: Patrick Haller <[email protected]>
Co-authored-by: Patrick Haller <[email protected]>
The crate error-chain used by onefetch for error handling seems to no longer be maintained.
Anyhow seems to be a good candidate for structuring errors 🤔.
At the same time, this might be an opportunity to improve and homogenize the way we handle/report errors.