diff --git a/internal/errors.go b/internal/errors.go new file mode 100644 index 00000000..8e2a2283 --- /dev/null +++ b/internal/errors.go @@ -0,0 +1,40 @@ +package internal + +import "fmt" + +// wrappedError is an error that supports the Go 1.13+ mechanism of wrapping +// errors. It implements Unwrap to return the underlying error, but it still +// returns the string version of whatever "main" error it represents. +type wrappedError struct { + underlyingError error + stringError error +} + +// WrapErrorf returns an error that implements the Go 1.13 Unwrap interface. +// The Error function returns the message calculated from formatting the +// arguments, just like [fmt.Errorf], and the Unwrap function returns the +// "underlying" error. Use this wherever one might otherwise use the "%w" +// format string in [fmt.Errorf]. +// +// err := doSomething() +// if err != nil { +// return WrapErrorf(err, "Could not do something: %v", err) +// } +// +// Although the "%w" format string will be recognized in versions of Go that +// support it, any error wrapped by it will not be included in this error; only +// underlying is considered wrapped by this error. +func WrapErrorf(underlying error, format string, args ...interface{}) error { + return &wrappedError{ + underlyingError: underlying, + stringError: fmt.Errorf(format, args...), + } +} + +func (e *wrappedError) Error() string { + return e.stringError.Error() +} + +func (e *wrappedError) Unwrap() error { + return e.underlyingError +} diff --git a/internal/errors113_test.go b/internal/errors113_test.go new file mode 100644 index 00000000..8a0c51f3 --- /dev/null +++ b/internal/errors113_test.go @@ -0,0 +1,23 @@ +// The concept of "wrapping" errors was only introduced in Go 1.13, so we only +// want to test that our errors behave like wrapped errors on Go versions that +// support `errors.Is`. +//go:build go1.13 +// +build go1.13 + +package internal + +import ( + "errors" + "testing" +) + +// TestErrorUnwrap checks that [errors.Is] can detect the underlying error in a +// wrappedError. +func TestErrorUnwrap(t *testing.T) { + underlyingError := errors.New("underlying error") + actual := WrapErrorf(underlyingError, "main message") + + if !errors.Is(actual, underlyingError) { + t.Fatalf("Expected outer error %#v to include %#v", actual, underlyingError) + } +} diff --git a/internal/errors_test.go b/internal/errors_test.go new file mode 100644 index 00000000..1df01912 --- /dev/null +++ b/internal/errors_test.go @@ -0,0 +1,29 @@ +package internal + +import ( + "errors" + "testing" +) + +// TestBasicWrappedError confirms that a wrappedError returns its string +// message, not that of its "underlying" error. +func TestBasicWrappedError(t *testing.T) { + message := "main message" + underlyingError := errors.New("underlying error") + actual := WrapErrorf(underlyingError, message) + + if actual.Error() != message { + t.Fatalf("Expected outer error to have Error() = %q but got %q", message, actual.Error()) + } +} + +// TestWrapFormat checks that the arguments get formatted, just like +// [fmt.Sprintf]. +func TestWrapFormat(t *testing.T) { + underlyingError := errors.New("underlying error") + actual := WrapErrorf(underlyingError, "%s %s", "main", "message") + + if actual.Error() != "main message" { + t.Fatalf("Expected outer error to have formatted message, but got %q", actual.Error()) + } +} diff --git a/internal/run.go b/internal/run.go index 79b4f049..cad3685c 100644 --- a/internal/run.go +++ b/internal/run.go @@ -52,7 +52,7 @@ func OutputDebug(cmd string, args ...string) (string, error) { if err := c.Run(); err != nil { errMsg := strings.TrimSpace(errbuf.String()) debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errMsg) - return "", fmt.Errorf("error running \"%s %s\": %s\n%s", cmd, strings.Join(args, " "), err, errMsg) + return "", WrapErrorf(err, "error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg) } return strings.TrimSpace(buf.String()), nil } diff --git a/mage/main.go b/mage/main.go index 0062bd35..758c3c31 100644 --- a/mage/main.go +++ b/mage/main.go @@ -473,14 +473,14 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) if !filepath.IsAbs(magePath) { cwd, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("can't get current working directory: %v", err) + return nil, internal.WrapErrorf(err, "can't get current working directory: %v", err) } magePath = filepath.Join(cwd, magePath) } env, err := internal.SplitEnv(envStr) if err != nil { - return nil, fmt.Errorf("error parsing environment variables: %v", err) + return nil, internal.WrapErrorf(err, "error parsing environment variables: %v", err) } bctx := build.Default @@ -502,7 +502,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error) // Allow multiple packages in the same directory if _, ok := err.(*build.MultiplePackageError); !ok { - return nil, fmt.Errorf("failed to parse go source files: %v", err) + return nil, internal.WrapErrorf(err, "failed to parse go source files: %v", err) } } @@ -530,7 +530,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil debug.Println("getting all files including those with mage tag in", magePath) mageFiles, err := listGoFiles(magePath, goCmd, "mage", env) if err != nil { - return nil, fmt.Errorf("listing mage files: %v", err) + return nil, internal.WrapErrorf(err, "listing mage files: %v", err) } if isMagefilesDirectory { @@ -546,7 +546,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil debug.Println("getting all files without mage tag in", magePath) nonMageFiles, err := listGoFiles(magePath, goCmd, "", env) if err != nil { - return nil, fmt.Errorf("listing non-mage files: %v", err) + return nil, internal.WrapErrorf(err, "listing non-mage files: %v", err) } // convert non-Mage list to a map of files to exclude. @@ -612,7 +612,7 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error { f, err := os.Create(path) if err != nil { - return fmt.Errorf("error creating generated mainfile: %v", err) + return internal.WrapErrorf(err, "error creating generated mainfile: %v", err) } defer f.Close() data := mainfileTemplateData{ @@ -629,16 +629,16 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error { debug.Println("writing new file at", path) if err := mainfileTemplate.Execute(f, data); err != nil { - return fmt.Errorf("can't execute mainfile template: %v", err) + return internal.WrapErrorf(err, "can't execute mainfile template: %v", err) } if err := f.Close(); err != nil { - return fmt.Errorf("error closing generated mainfile: %v", err) + return internal.WrapErrorf(err, "error closing generated mainfile: %v", err) } // we set an old modtime on the generated mainfile so that the go tool // won't think it has changed more recently than the compiled binary. longAgo := time.Now().Add(-time.Hour * 24 * 365 * 10) if err := os.Chtimes(path, longAgo, longAgo); err != nil { - return fmt.Errorf("error setting old modtime on generated mainfile: %v", err) + return internal.WrapErrorf(err, "error setting old modtime on generated mainfile: %v", err) } return nil } @@ -675,13 +675,13 @@ func ExeName(goCmd, cacheDir string, files []string) (string, error) { func hashFile(fn string) (string, error) { f, err := os.Open(fn) if err != nil { - return "", fmt.Errorf("can't open input file for hashing: %#v", err) + return "", internal.WrapErrorf(err, "can't open input file for hashing: %v", err) } defer f.Close() h := sha1.New() if _, err := io.Copy(h, f); err != nil { - return "", fmt.Errorf("can't write data to hash: %v", err) + return "", internal.WrapErrorf(err, "can't write data to hash: %v", err) } return fmt.Sprintf("%x", h.Sum(nil)), nil } @@ -690,12 +690,12 @@ func generateInit(dir string) error { debug.Println("generating default magefile in", dir) f, err := os.Create(filepath.Join(dir, initFile)) if err != nil { - return fmt.Errorf("could not create mage template: %v", err) + return internal.WrapErrorf(err, "could not create mage template: %v", err) } defer f.Close() if err := initOutput.Execute(f, nil); err != nil { - return fmt.Errorf("can't execute magefile template: %v", err) + return internal.WrapErrorf(err, "can't execute magefile template: %v", err) } return nil diff --git a/mage/main_test.go b/mage/main_test.go index 07e598e5..d4ddd9de 100644 --- a/mage/main_test.go +++ b/mage/main_test.go @@ -1270,7 +1270,7 @@ func TestCompiledFlags(t *testing.T) { cmd.Stderr = stderr cmd.Stdout = stdout if err := cmd.Run(); err != nil { - return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", + return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", filename, strings.Join(args, " "), err, stdout, stderr) } return nil @@ -1357,7 +1357,7 @@ func TestCompiledEnvironmentVars(t *testing.T) { cmd.Stderr = stderr cmd.Stdout = stdout if err := cmd.Run(); err != nil { - return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", + return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", filename, strings.Join(args, " "), err, stdout, stderr) } return nil @@ -1511,7 +1511,7 @@ func TestSignals(t *testing.T) { cmd.Stderr = stderr cmd.Stdout = stdout if err := cmd.Start(); err != nil { - return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", + return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", filename, target, err, stdout, stderr) } pid := cmd.Process.Pid @@ -1523,7 +1523,7 @@ func TestSignals(t *testing.T) { } }() if err := cmd.Wait(); err != nil { - return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", + return internal.WrapErrorf(err, "running '%s %s' failed with: %v\nstdout: %s\nstderr: %s", filename, target, err, stdout, stderr) } return nil diff --git a/magefile.go b/magefile.go index c08ffa37..34708dfc 100644 --- a/magefile.go +++ b/magefile.go @@ -17,6 +17,7 @@ import ( "strings" "time" + "github.com/magefile/mage/internal" "github.com/magefile/mage/mg" "github.com/magefile/mage/sh" ) @@ -43,12 +44,12 @@ func Install() error { // in GOPATH environment string bin, err := sh.Output(gocmd, "env", "GOBIN") if err != nil { - return fmt.Errorf("can't determine GOBIN: %v", err) + return internal.WrapErrorf(err, "can't determine GOBIN: %v", err) } if bin == "" { gopath, err := sh.Output(gocmd, "env", "GOPATH") if err != nil { - return fmt.Errorf("can't determine GOPATH: %v", err) + return internal.WrapErrorf(err, "can't determine GOPATH: %v", err) } paths := strings.Split(gopath, string([]rune{os.PathListSeparator})) bin = filepath.Join(paths[0], "bin") @@ -56,7 +57,7 @@ func Install() error { // specifically don't mkdirall, if you have an invalid gopath in the first // place, that's not on us to fix. if err := os.Mkdir(bin, 0700); err != nil && !os.IsExist(err) { - return fmt.Errorf("failed to create %q: %v", bin, err) + return internal.WrapErrorf(err, "failed to create %q: %v", bin, err) } path := filepath.Join(bin, name) @@ -72,7 +73,7 @@ var releaseTag = regexp.MustCompile(`^v1\.[0-9]+\.[0-9]+$`) // Generates a new release. Expects a version tag in v1.x.x format. func Release(tag string) (err error) { if _, err := exec.LookPath("goreleaser"); err != nil { - return fmt.Errorf("can't find goreleaser: %w", err) + return internal.WrapErrorf(err, "can't find goreleaser: %v", err) } if !releaseTag.MatchString(tag) { return errors.New("TAG environment variable must be in semver v1.x.x format, but was " + tag) diff --git a/mg/errors_test.go b/mg/errors_test.go index ac5e68f6..21c053e4 100644 --- a/mg/errors_test.go +++ b/mg/errors_test.go @@ -1,6 +1,8 @@ package mg -import "testing" +import ( + "testing" +) func TestFatalExit(t *testing.T) { expected := 99 diff --git a/mg/fn.go b/mg/fn.go index 3856857a..8e303080 100644 --- a/mg/fn.go +++ b/mg/fn.go @@ -37,7 +37,7 @@ func F(target interface{}, args ...interface{}) Fn { } id, err := json.Marshal(args) if err != nil { - panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %s", err)) + panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %v", err)) } return fn{ name: funcName(target), diff --git a/parse/parse.go b/parse/parse.go index e68703bd..323dc0ee 100644 --- a/parse/parse.go +++ b/parse/parse.go @@ -735,7 +735,7 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package, pkgs, err := parser.ParseDir(fset, path, filter, parser.ParseComments) if err != nil { - return nil, fmt.Errorf("failed to parse directory: %v", err) + return nil, internal.WrapErrorf(err, "failed to parse directory: %v", err) } switch len(pkgs) { diff --git a/sh/cmd.go b/sh/cmd.go index 312de65a..adc992ac 100644 --- a/sh/cmd.go +++ b/sh/cmd.go @@ -9,6 +9,7 @@ import ( "os/exec" "strings" + "github.com/magefile/mage/internal" "github.com/magefile/mage/mg" ) @@ -120,7 +121,7 @@ func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...s if ran { return ran, mg.Fatalf(code, `running "%s %s" failed with exit code %d`, cmd, strings.Join(args, " "), code) } - return ran, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err) + return ran, internal.WrapErrorf(err, `failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err) } func run(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, code int, err error) { diff --git a/sh/cmd113_test.go b/sh/cmd113_test.go new file mode 100644 index 00000000..23a201ce --- /dev/null +++ b/sh/cmd113_test.go @@ -0,0 +1,23 @@ +// The concept of "wrapping" errors was only introduced in Go 1.13, so we only +// want to test that our errors behave like wrapped errors on Go versions that +// support `errors.Is`. +//go:build go1.13 +// +build go1.13 + +package sh + +import ( + "errors" + "os" + "testing" +) + +func TestWrappedError(t *testing.T) { + _, err := Exec(nil, nil, nil, os.Args[0]+"-doesnotexist", "-printArgs", "foo") + if err == nil { + t.Fatalf("Expected to fail running %s", os.Args[0]+"-doesnotexist") + } + if !errors.Is(err, os.ErrNotExist) { + t.Fatalf("Expected error to be like ErrNotExist, got %#v", err) + } +} diff --git a/sh/helpers.go b/sh/helpers.go index f5d20a27..ef6bf2ca 100644 --- a/sh/helpers.go +++ b/sh/helpers.go @@ -1,9 +1,10 @@ package sh import ( - "fmt" "io" "os" + + "github.com/magefile/mage/internal" ) // Rm removes the given file or directory even if non-empty. It will not return @@ -13,28 +14,28 @@ func Rm(path string) error { if err == nil || os.IsNotExist(err) { return nil } - return fmt.Errorf(`failed to remove %s: %v`, path, err) + return internal.WrapErrorf(err, `failed to remove %s: %v`, path, err) } // Copy robustly copies the source file to the destination, overwriting the destination if necessary. func Copy(dst string, src string) error { from, err := os.Open(src) if err != nil { - return fmt.Errorf(`can't copy %s: %v`, src, err) + return internal.WrapErrorf(err, `can't copy %s: %v`, src, err) } defer from.Close() finfo, err := from.Stat() if err != nil { - return fmt.Errorf(`can't stat %s: %v`, src, err) + return internal.WrapErrorf(err, `can't stat %s: %v`, src, err) } to, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, finfo.Mode()) if err != nil { - return fmt.Errorf(`can't copy to %s: %v`, dst, err) + return internal.WrapErrorf(err, `can't copy to %s: %v`, dst, err) } defer to.Close() _, err = io.Copy(to, from) if err != nil { - return fmt.Errorf(`error copying %s to %s: %v`, src, dst, err) + return internal.WrapErrorf(err, `error copying %s to %s: %v`, src, dst, err) } return nil } diff --git a/sh/helpers_test.go b/sh/helpers_test.go index 54e78aa1..38ded150 100644 --- a/sh/helpers_test.go +++ b/sh/helpers_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "testing" + "github.com/magefile/mage/internal" "github.com/magefile/mage/sh" ) @@ -17,11 +18,11 @@ import ( func compareFiles(file1 string, file2 string) error { s1, err := os.Stat(file1) if err != nil { - return fmt.Errorf("can't stat %s: %v", file1, err) + return internal.WrapErrorf(err, "can't stat %s: %v", file1, err) } s2, err := os.Stat(file2) if err != nil { - return fmt.Errorf("can't stat %s: %v", file2, err) + return internal.WrapErrorf(err, "can't stat %s: %v", file2, err) } if s1.Size() != s2.Size() { return fmt.Errorf("files %s and %s have different sizes: %d vs %d", file1, file2, s1.Size(), s2.Size()) @@ -31,11 +32,11 @@ func compareFiles(file1 string, file2 string) error { } f1bytes, err := ioutil.ReadFile(file1) if err != nil { - return fmt.Errorf("can't read %s: %v", file1, err) + return internal.WrapErrorf(err, "can't read %s: %v", file1, err) } f2bytes, err := ioutil.ReadFile(file2) if err != nil { - return fmt.Errorf("can't read %s: %v", file2, err) + return internal.WrapErrorf(err, "can't read %s: %v", file2, err) } if !bytes.Equal(f1bytes, f2bytes) { return fmt.Errorf("files %s and %s have different contents", file1, file2)