*: migrate to Go stdlib error wrapping #556
Merged
Conversation
This file contains 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
tych0
previously approved these changes
Nov 28, 2024
cyphar
force-pushed
the
go-std-error
branch
2 times, most recently
from
167e6ff to
ddf4530
Compare
|
Yeah, it turns out depending on the behaviour of |
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #556 +/- ##
===========================================
+ Coverage 46.49% 72.43% +25.94%
===========================================
Files 60 60
Lines 4906 4985 +79
===========================================
+ Hits 2281 3611 +1330
+ Misses 2358 1000 -1358
- Partials 267 374 +107
|
cyphar
force-pushed
the
go-std-error
branch
5 times, most recently
from
9317136 to
d6122d9
Compare
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
I used to dislike using %q because once it starts nesting it gets quite nasty, but we probably should be properly quoting strings in error messages in case of weird inputs. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
fmt.Errorf will still return an error if the wrapped errors are nil, which is a noticable departure from errors.Wrap. Unfortunately, umoci implicitly depends on this in ways that are non-obvious so a compatibility shim is the safest option for now. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Quite a few of these callers don't need to use fmtcompat, but for simplicity they have all been converted and we can slowly remove fmtcompat.Errorf calls when we verify that they are correct. Some of the obvious "err = nil" logic has been reworked, but there are still some suble usages of it that will take some time to find and fix. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
For the super-simple case of string errors that don't contain %w, we can safely use fmt.Errorf or errors.New. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
For cases like
if err != nil {
return fmtcompat.Errorf("...: %w", err)
}
then we don't need fmtcompat.Errorf because the error has already
explcitly been checked.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These are more complicated checks than the trivial
if err != nil {
return fmt.Errorf("...: %w", err)
}
but are similar and can be ported for the same reasons as the simple
case, except they needed to be hand-checked for correctness.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The remaining users of fmtcompat.Errorf are all something resembling
err := someFunc(...)
return fmtcompat.Errorf("...: %w", err)
which can easily be rewritten to be more standard Go and doing explicit
error checks. This removes the final users of fmtcompat.Errorf.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
fmtcompat.Errorf was only needed as a transitional tool, now that everything uses native fmt.Errorf(%w) we can remove it entirely. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
|
This patchset adds and removes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #388