-
Notifications
You must be signed in to change notification settings - Fork 803
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
Introduce support for wrapped errors in the codebase #6041
base: master
Are you sure you want to change the base?
Introduce support for wrapped errors in the codebase #6041
Conversation
bd63d06
to
89451a9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
There is a lot of places where I think we shoud prefer to use assert.ErrorAs
insted of assert.True(t, errors.As...
. Commented a few places, but not all
fe3bb34
to
c9a782d
Compare
35a729e
to
91d2e19
Compare
91d2e19
to
47f55f5
Compare
What changed?
I've switched all direct error handling to usage of either errors.Is or errors.As.
I've split the PR into commits so it can be reviewed commit by commit.
Most of the changes are converting expressions to an identical one so no logical changes are expected:
to
There were a couple of more complicated cases, but all could be resolved.
Why?
This is a mandatory step to improve existing error handling that does not provide a clear path of where the error is coming from. In all style guides, error wrapping is recommended, but the codebase should be ready to handle wrapped errors instead of exact type checks.
How did you test it?
Unit/integration tests.
Potential risks
Potentially, some errors could be misclassified.
Release notes
Documentation Changes