mgmt
mgmt copied to clipboard
Use fmt.Errorf and replace errwrap.Wrapf
Go 1.13 adds %w
to fmt.Errorf which is a first-party replacement for the mgmt errwrap package.
https://blog.golang.org/go1.13-errors
The argument ordering differs slightly, but porting it should be entirely doable.
I think the simplest form of this change would be as follows
diff --git a/util/errwrap/errwrap.go b/util/errwrap/errwrap.go
index 3bd64e3..6c1ac2f 100644
--- a/util/errwrap/errwrap.go
+++ b/util/errwrap/errwrap.go
@@ -19,14 +19,15 @@
package errwrap
import (
+ "fmt"
+
"github.com/hashicorp/go-multierror"
- "github.com/pkg/errors"
)
// Wrapf adds a new error onto an existing chain of errors. If the new error to
// be added is nil, then the old error is returned unchanged.
func Wrapf(err error, format string, args ...interface{}) error {
- return errors.Wrapf(err, format, args...)
+ return fmt.Errorf("%s: %w", fmt.Sprintf(format, args), err)
}
// Append can be used to safely append an error onto an existing one. If you
Although it could certainly be taken one further to inline the usage of the Wrapf function and then remove it completely as it's redundant
What's the current reason of existence for errors.Wrapf? Was it only because of golang didn't support it until now?
If those errors.Wrapf were only to add into error chain, I believe it would indeed be better for readability to use fmt.Errorf %w. The intent is (by far) more clear for me that way.
On Wed, Mar 3, 2021 at 7:20 AM Jean-Philippe Evrard [email protected] wrote:
What's the current reason of existence for errors.Wrapf? Was it only because of golang didn't support it until now?
Correct.
If those errors.Wrapf were only to add into error chain, I believe it would indeed be better for readability to use fmt.Errorf %w. The intent is (by far) more clear for me that way.
I think we should probably switch over, however the problem is that
we can do return errwrap.Wrapf(err, "whatever")
and if err is nil,
the whole thing is nil.
Let's write a bunch of if err != nil
then!
I don't mind having a look at this. @frebib , can I use your help on this? I hope to reduce code/tech debt by removing the whole errwrap in a decent refactor.
Sure thing @evrardjp, anything you need. I did try implementing this in the naive way and it backfired, presumably because the nils