Support wrapped errors

Wrapping errors instead of merely embedding error messages allows
calling code to use `errors.Is` and `errors.As` to more precisely check
the reasons for various errors instead of having to rely only on
substring searches.

We implement our own wrapper error to retain backward compatibility
prior to Go 1.13, where there is no support for the "%w" format string
in `fmt.Errorf`.

Author: rkennedy

internal/run.go

@@ -9,6 +9,8 @@ import (
 	"os/exec"
 	"runtime"
 	"strings"
+
+	"github.com/magefile/mage/mg"
 )
 
 var debug *log.Logger = log.New(ioutil.Discard, "", 0)
@@ -52,7 +54,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 "", mg.WrapError(err, fmt.Errorf("error running \"%s %s\": %v\n%s", cmd, strings.Join(args, " "), err, errMsg))
 	}
 	return strings.TrimSpace(buf.String()), nil
 }

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, mg.WrapError(err, fmt.Errorf("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, mg.WrapError(err, fmt.Errorf("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, mg.WrapError(err, fmt.Errorf("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, mg.WrapError(err, fmt.Errorf("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, mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("can't execute mainfile template: %v", err))
 	}
 	if err := f.Close(); err != nil {
-		return fmt.Errorf("error closing generated mainfile: %v", err)
+		return mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("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 "", mg.WrapError(err, fmt.Errorf("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 "", mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("can't execute magefile template: %v", err))
 	}
 
 	return nil

mage/main_test.go

@@ -6,6 +6,7 @@ import (
 	"debug/macho"
 	"debug/pe"
 	"encoding/hex"
+	"errors"
 	"flag"
 	"fmt"
 	"go/build"
@@ -1270,8 +1271,8 @@ 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",
-				filename, strings.Join(args, " "), err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
 	}
@@ -1315,6 +1316,10 @@ func TestCompiledFlags(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected an error because of timeout")
 	}
+	var exitError *exec.ExitError
+	if !errors.As(err, &exitError) {
+		t.Errorf("Expected wrapped ExitError from process; got %#v", err)
+	}
 	got = stderr.String()
 	want = "context deadline exceeded"
 	if strings.Contains(got, want) == false {
@@ -1357,8 +1362,8 @@ 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",
-				filename, strings.Join(args, " "), err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, strings.Join(args, " "), err, stdout, stderr))
 		}
 		return nil
 	}
@@ -1511,8 +1516,8 @@ 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",
-				filename, target, err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr))
 		}
 		pid := cmd.Process.Pid
 		go func() {
@@ -1523,8 +1528,8 @@ func TestSignals(t *testing.T) {
 			}
 		}()
 		if err := cmd.Wait(); err != nil {
-			return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
-				filename, target, err, stdout, stderr)
+			return mg.WrapError(err, fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
+				filename, target, err, stdout, stderr))
 		}
 		return nil
 	}

magefile.go

@@ -43,20 +43,20 @@ 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 mg.WrapError(err, fmt.Errorf("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 mg.WrapError(err, fmt.Errorf("can't determine GOPATH: %v", err))
 		}
 		paths := strings.Split(gopath, string([]rune{os.PathListSeparator}))
 		bin = filepath.Join(paths[0], "bin")
 	}
 	// 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 mg.WrapError(err, fmt.Errorf("failed to create %q: %v", bin, err))
 	}
 	path := filepath.Join(bin, name)
 
@@ -72,7 +72,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 mg.WrapError(err, fmt.Errorf("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)

mg/errors.go

@@ -49,3 +49,38 @@ func ExitStatus(err error) int {
 	}
 	return exit.ExitStatus()
 }
+
+// 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
+}
+
+// WrapError returns an error that implements the Go 1.13 Unwrap interface. The
+// Error function returns the value of the "string" error, 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 WrapError(err, fmt.Errorf("Could not do something: %v", err))
+//     }
+// The premise is that the "string" error is not an interesting one to try to
+// inspect with [errors.Is] or [errors.As] because it has no other wrapped
+// errors of its own, and it will usually be of whatever error type
+// `fmt.Errorf` returns.
+func WrapError(underlying, str error) error {
+	return &wrappedError{
+		underlyingError: underlying,
+		stringError:     str,
+	}
+}
+
+func (e *wrappedError) Error() string {
+	return e.stringError.Error()
+}
+
+func (e *wrappedError) Unwrap() error {
+	return e.underlyingError
+}

mg/errors113_test.go

@@ -0,0 +1,24 @@
+// 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 mg
+
+import (
+	"errors"
+	"testing"
+)
+
+// TestErrorUnwrap checks that [errors.Is] can detect the underlying error in a
+// wrappedError.
+func TestErrorUnwrap(t *testing.T) {
+	strError := errors.New("main error")
+	underlyingError := errors.New("underlying error")
+	actual := WrapError(underlyingError, strError)
+
+	if !errors.Is(actual, underlyingError) {
+		t.Fatalf("Expected outer error %#v to include %#v", actual, underlyingError)
+	}
+}

mg/errors_test.go

@@ -1,6 +1,9 @@
 package mg
 
-import "testing"
+import (
+	"errors"
+	"testing"
+)
 
 func TestFatalExit(t *testing.T) {
 	expected := 99
@@ -17,3 +20,15 @@ func TestFatalfExit(t *testing.T) {
 		t.Fatalf("Expected code %v but got %v", expected, code)
 	}
 }
+
+// TestBasicWrappedError confirms that a wrappedError returns the same string
+// as its "str" error (not its "underlying" error).
+func TestBasicWrappedError(t *testing.T) {
+	strError := errors.New("main error")
+	underlyingError := errors.New("underlying error")
+	actual := WrapError(underlyingError, strError)
+
+	if actual.Error() != strError.Error() {
+		t.Fatalf("Expected outer error to have Error() = %q but got %q", strError.Error(), actual.Error())
+	}
+}

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(WrapError(err, fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %v", err)))
 	}
 	return fn{
 		name: funcName(target),

parse/parse.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/magefile/mage/internal"
+	"github.com/magefile/mage/mg"
 )
 
 const importTag = "mage:import"
@@ -735,7 +736,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, mg.WrapError(err, fmt.Errorf("failed to parse directory: %v", err))
 	}
 
 	switch len(pkgs) {

sh/cmd.go

@@ -120,7 +120,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, mg.WrapError(err, fmt.Errorf(`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) {

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)
+	}
+}