Skip to content

Commit

Permalink
Use %w to wrap upstream errors
Browse files Browse the repository at this point in the history
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.

Fixes magefile#503
  • Loading branch information
rkennedy committed May 1, 2024
1 parent 2385abb commit e6a2d7b
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 33 deletions.
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 "", fmt.Errorf("error running \"%s %s\": %w\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, fmt.Errorf("can't get current working directory: %w", 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, fmt.Errorf("error parsing environment variables: %w", 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, fmt.Errorf("failed to parse go source files: %w", 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, fmt.Errorf("listing mage files: %w", 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, fmt.Errorf("listing non-mage files: %w", 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 fmt.Errorf("error creating generated mainfile: %w", 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 fmt.Errorf("can't execute mainfile template: %w", err)
}
if err := f.Close(); err != nil {
return fmt.Errorf("error closing generated mainfile: %v", err)
return fmt.Errorf("error closing generated mainfile: %w", 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 fmt.Errorf("error setting old modtime on generated mainfile: %w", 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 "", fmt.Errorf("can't open input file for hashing: %w", 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 "", fmt.Errorf("can't write data to hash: %w", 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 fmt.Errorf("could not create mage template: %w", err)
}
defer f.Close()

if err := initOutput.Execute(f, nil); err != nil {
return fmt.Errorf("can't execute magefile template: %v", err)
return fmt.Errorf("can't execute magefile template: %w", err)
}

return nil
Expand Down
13 changes: 9 additions & 4 deletions mage/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"debug/macho"
"debug/pe"
"encoding/hex"
"errors"
"flag"
"fmt"
"go/build"
Expand Down Expand Up @@ -1270,7 +1271,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 fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
filename, strings.Join(args, " "), err, stdout, stderr)
}
return nil
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1357,7 +1362,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 fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
filename, strings.Join(args, " "), err, stdout, stderr)
}
return nil
Expand Down Expand Up @@ -1511,7 +1516,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 fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
filename, target, err, stdout, stderr)
}
pid := cmd.Process.Pid
Expand All @@ -1523,7 +1528,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 fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
filename, target, err, stdout, stderr)
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 fmt.Errorf("can't determine GOBIN: %w", err)
}
if bin == "" {
gopath, err := sh.Output(gocmd, "env", "GOPATH")
if err != nil {
return fmt.Errorf("can't determine GOPATH: %v", err)
return fmt.Errorf("can't determine GOPATH: %w", 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 fmt.Errorf("failed to create %q: %w", bin, err)
}
path := filepath.Join(bin, name)

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: %w", 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, fmt.Errorf("failed to parse directory: %w", err)
}

switch len(pkgs) {
Expand Down
2 changes: 1 addition & 1 deletion sh/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, fmt.Errorf(`failed to run "%s %s: %w"`, 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
12 changes: 12 additions & 0 deletions sh/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package sh

import (
"bytes"
"errors"
"io/fs"
"os"
"testing"
)
Expand Down Expand Up @@ -70,3 +72,13 @@ func TestAutoExpand(t *testing.T) {
}

}

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, fs.ErrNotExist) {
t.Fatalf("Expected error to be like ErrNotExist, got %#v", err)
}
}
10 changes: 5 additions & 5 deletions sh/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,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 fmt.Errorf(`failed to remove %s: %w`, 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 fmt.Errorf(`can't copy %s: %w`, src, err)
}
defer from.Close()
finfo, err := from.Stat()
if err != nil {
return fmt.Errorf(`can't stat %s: %v`, src, err)
return fmt.Errorf(`can't stat %s: %w`, 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 fmt.Errorf(`can't copy to %s: %w`, 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 fmt.Errorf(`error copying %s to %s: %w`, src, dst, err)
}
return nil
}
8 changes: 4 additions & 4 deletions sh/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,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 fmt.Errorf("can't stat %s: %w", file1, err)
}
s2, err := os.Stat(file2)
if err != nil {
return fmt.Errorf("can't stat %s: %v", file2, err)
return fmt.Errorf("can't stat %s: %w", 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())
Expand All @@ -31,11 +31,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 fmt.Errorf("can't read %s: %w", file1, err)
}
f2bytes, err := ioutil.ReadFile(file2)
if err != nil {
return fmt.Errorf("can't read %s: %v", file2, err)
return fmt.Errorf("can't read %s: %w", file2, err)
}
if !bytes.Equal(f1bytes, f2bytes) {
return fmt.Errorf("files %s and %s have different contents", file1, file2)
Expand Down

0 comments on commit e6a2d7b

Please sign in to comment.