Skip to content
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

Support wrapped errors #504

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions internal/errors.go
Original file line number Diff line number Diff line change
@@ -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
}
23 changes: 23 additions & 0 deletions internal/errors113_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
29 changes: 29 additions & 0 deletions internal/errors_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
}
2 changes: 1 addition & 1 deletion internal/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
26 changes: 13 additions & 13 deletions mage/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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{
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions mage/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 5 additions & 4 deletions magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"strings"
"time"

"github.com/magefile/mage/internal"
"github.com/magefile/mage/mg"
"github.com/magefile/mage/sh"
)
Expand All @@ -43,20 +44,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 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")
}
// 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)

Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion mg/errors_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package mg

import "testing"
import (
"testing"
)

func TestFatalExit(t *testing.T) {
expected := 99
Expand Down
2 changes: 1 addition & 1 deletion mg/fn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion sh/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os/exec"
"strings"

"github.com/magefile/mage/internal"
"github.com/magefile/mage/mg"
)

Expand Down Expand Up @@ -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) {
Expand Down
23 changes: 23 additions & 0 deletions sh/cmd113_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
13 changes: 7 additions & 6 deletions sh/helpers.go
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
}
Loading