Skip to content

Commit dbd760f

Browse files
committed
Use %w to wrap upstream 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. Fixes #503
1 parent 2385abb commit dbd760f

File tree

10 files changed

+50
-33
lines changed

10 files changed

+50
-33
lines changed

internal/run.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func OutputDebug(cmd string, args ...string) (string, error) {
5252
if err := c.Run(); err != nil {
5353
errMsg := strings.TrimSpace(errbuf.String())
5454
debug.Print("error running '", cmd, strings.Join(args, " "), "': ", err, ": ", errMsg)
55-
return "", fmt.Errorf("error running \"%s %s\": %s\n%s", cmd, strings.Join(args, " "), err, errMsg)
55+
return "", fmt.Errorf("error running \"%s %s\": %w\n%s", cmd, strings.Join(args, " "), err, errMsg)
5656
}
5757
return strings.TrimSpace(buf.String()), nil
5858
}

mage/main.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -473,14 +473,14 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
473473
if !filepath.IsAbs(magePath) {
474474
cwd, err := os.Getwd()
475475
if err != nil {
476-
return nil, fmt.Errorf("can't get current working directory: %v", err)
476+
return nil, fmt.Errorf("can't get current working directory: %w", err)
477477
}
478478
magePath = filepath.Join(cwd, magePath)
479479
}
480480

481481
env, err := internal.SplitEnv(envStr)
482482
if err != nil {
483-
return nil, fmt.Errorf("error parsing environment variables: %v", err)
483+
return nil, fmt.Errorf("error parsing environment variables: %w", err)
484484
}
485485

486486
bctx := build.Default
@@ -502,7 +502,7 @@ func listGoFiles(magePath, goCmd, tag string, envStr []string) ([]string, error)
502502

503503
// Allow multiple packages in the same directory
504504
if _, ok := err.(*build.MultiplePackageError); !ok {
505-
return nil, fmt.Errorf("failed to parse go source files: %v", err)
505+
return nil, fmt.Errorf("failed to parse go source files: %w", err)
506506
}
507507
}
508508

@@ -530,7 +530,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
530530
debug.Println("getting all files including those with mage tag in", magePath)
531531
mageFiles, err := listGoFiles(magePath, goCmd, "mage", env)
532532
if err != nil {
533-
return nil, fmt.Errorf("listing mage files: %v", err)
533+
return nil, fmt.Errorf("listing mage files: %w", err)
534534
}
535535

536536
if isMagefilesDirectory {
@@ -546,7 +546,7 @@ func Magefiles(magePath, goos, goarch, goCmd string, stderr io.Writer, isMagefil
546546
debug.Println("getting all files without mage tag in", magePath)
547547
nonMageFiles, err := listGoFiles(magePath, goCmd, "", env)
548548
if err != nil {
549-
return nil, fmt.Errorf("listing non-mage files: %v", err)
549+
return nil, fmt.Errorf("listing non-mage files: %w", err)
550550
}
551551

552552
// convert non-Mage list to a map of files to exclude.
@@ -612,7 +612,7 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
612612

613613
f, err := os.Create(path)
614614
if err != nil {
615-
return fmt.Errorf("error creating generated mainfile: %v", err)
615+
return fmt.Errorf("error creating generated mainfile: %w", err)
616616
}
617617
defer f.Close()
618618
data := mainfileTemplateData{
@@ -629,16 +629,16 @@ func GenerateMainfile(binaryName, path string, info *parse.PkgInfo) error {
629629

630630
debug.Println("writing new file at", path)
631631
if err := mainfileTemplate.Execute(f, data); err != nil {
632-
return fmt.Errorf("can't execute mainfile template: %v", err)
632+
return fmt.Errorf("can't execute mainfile template: %w", err)
633633
}
634634
if err := f.Close(); err != nil {
635-
return fmt.Errorf("error closing generated mainfile: %v", err)
635+
return fmt.Errorf("error closing generated mainfile: %w", err)
636636
}
637637
// we set an old modtime on the generated mainfile so that the go tool
638638
// won't think it has changed more recently than the compiled binary.
639639
longAgo := time.Now().Add(-time.Hour * 24 * 365 * 10)
640640
if err := os.Chtimes(path, longAgo, longAgo); err != nil {
641-
return fmt.Errorf("error setting old modtime on generated mainfile: %v", err)
641+
return fmt.Errorf("error setting old modtime on generated mainfile: %w", err)
642642
}
643643
return nil
644644
}
@@ -675,13 +675,13 @@ func ExeName(goCmd, cacheDir string, files []string) (string, error) {
675675
func hashFile(fn string) (string, error) {
676676
f, err := os.Open(fn)
677677
if err != nil {
678-
return "", fmt.Errorf("can't open input file for hashing: %#v", err)
678+
return "", fmt.Errorf("can't open input file for hashing: %w", err)
679679
}
680680
defer f.Close()
681681

682682
h := sha1.New()
683683
if _, err := io.Copy(h, f); err != nil {
684-
return "", fmt.Errorf("can't write data to hash: %v", err)
684+
return "", fmt.Errorf("can't write data to hash: %w", err)
685685
}
686686
return fmt.Sprintf("%x", h.Sum(nil)), nil
687687
}
@@ -690,12 +690,12 @@ func generateInit(dir string) error {
690690
debug.Println("generating default magefile in", dir)
691691
f, err := os.Create(filepath.Join(dir, initFile))
692692
if err != nil {
693-
return fmt.Errorf("could not create mage template: %v", err)
693+
return fmt.Errorf("could not create mage template: %w", err)
694694
}
695695
defer f.Close()
696696

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

701701
return nil

mage/main_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"debug/macho"
77
"debug/pe"
88
"encoding/hex"
9+
"errors"
910
"flag"
1011
"fmt"
1112
"go/build"
@@ -1270,7 +1271,7 @@ func TestCompiledFlags(t *testing.T) {
12701271
cmd.Stderr = stderr
12711272
cmd.Stdout = stdout
12721273
if err := cmd.Run(); err != nil {
1273-
return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
1274+
return fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
12741275
filename, strings.Join(args, " "), err, stdout, stderr)
12751276
}
12761277
return nil
@@ -1315,6 +1316,10 @@ func TestCompiledFlags(t *testing.T) {
13151316
if err == nil {
13161317
t.Fatalf("expected an error because of timeout")
13171318
}
1319+
var exitError *exec.ExitError
1320+
if !errors.As(err, &exitError) {
1321+
t.Errorf("Expected wrapped ExitError from process; got %#v", err)
1322+
}
13181323
got = stderr.String()
13191324
want = "context deadline exceeded"
13201325
if strings.Contains(got, want) == false {
@@ -1357,7 +1362,7 @@ func TestCompiledEnvironmentVars(t *testing.T) {
13571362
cmd.Stderr = stderr
13581363
cmd.Stdout = stdout
13591364
if err := cmd.Run(); err != nil {
1360-
return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
1365+
return fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
13611366
filename, strings.Join(args, " "), err, stdout, stderr)
13621367
}
13631368
return nil
@@ -1511,7 +1516,7 @@ func TestSignals(t *testing.T) {
15111516
cmd.Stderr = stderr
15121517
cmd.Stdout = stdout
15131518
if err := cmd.Start(); err != nil {
1514-
return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
1519+
return fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
15151520
filename, target, err, stdout, stderr)
15161521
}
15171522
pid := cmd.Process.Pid
@@ -1523,7 +1528,7 @@ func TestSignals(t *testing.T) {
15231528
}
15241529
}()
15251530
if err := cmd.Wait(); err != nil {
1526-
return fmt.Errorf("running '%s %s' failed with: %v\nstdout: %s\nstderr: %s",
1531+
return fmt.Errorf("running '%s %s' failed with: %w\nstdout: %s\nstderr: %s",
15271532
filename, target, err, stdout, stderr)
15281533
}
15291534
return nil

magefile.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,20 @@ func Install() error {
4343
// in GOPATH environment string
4444
bin, err := sh.Output(gocmd, "env", "GOBIN")
4545
if err != nil {
46-
return fmt.Errorf("can't determine GOBIN: %v", err)
46+
return fmt.Errorf("can't determine GOBIN: %w", err)
4747
}
4848
if bin == "" {
4949
gopath, err := sh.Output(gocmd, "env", "GOPATH")
5050
if err != nil {
51-
return fmt.Errorf("can't determine GOPATH: %v", err)
51+
return fmt.Errorf("can't determine GOPATH: %w", err)
5252
}
5353
paths := strings.Split(gopath, string([]rune{os.PathListSeparator}))
5454
bin = filepath.Join(paths[0], "bin")
5555
}
5656
// specifically don't mkdirall, if you have an invalid gopath in the first
5757
// place, that's not on us to fix.
5858
if err := os.Mkdir(bin, 0700); err != nil && !os.IsExist(err) {
59-
return fmt.Errorf("failed to create %q: %v", bin, err)
59+
return fmt.Errorf("failed to create %q: %w", bin, err)
6060
}
6161
path := filepath.Join(bin, name)
6262

mg/fn.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func F(target interface{}, args ...interface{}) Fn {
3737
}
3838
id, err := json.Marshal(args)
3939
if err != nil {
40-
panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %s", err))
40+
panic(fmt.Errorf("can't convert args into a mage-compatible id for mg.Deps: %w", err))
4141
}
4242
return fn{
4343
name: funcName(target),

parse/parse.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ func getPackage(path string, files []string, fset *token.FileSet) (*ast.Package,
735735

736736
pkgs, err := parser.ParseDir(fset, path, filter, parser.ParseComments)
737737
if err != nil {
738-
return nil, fmt.Errorf("failed to parse directory: %v", err)
738+
return nil, fmt.Errorf("failed to parse directory: %w", err)
739739
}
740740

741741
switch len(pkgs) {

sh/cmd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func Exec(env map[string]string, stdout, stderr io.Writer, cmd string, args ...s
120120
if ran {
121121
return ran, mg.Fatalf(code, `running "%s %s" failed with exit code %d`, cmd, strings.Join(args, " "), code)
122122
}
123-
return ran, fmt.Errorf(`failed to run "%s %s: %v"`, cmd, strings.Join(args, " "), err)
123+
return ran, fmt.Errorf(`failed to run "%s %s: %w"`, cmd, strings.Join(args, " "), err)
124124
}
125125

126126
func run(env map[string]string, stdout, stderr io.Writer, cmd string, args ...string) (ran bool, code int, err error) {

sh/cmd_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package sh
22

33
import (
44
"bytes"
5+
"errors"
6+
"io/fs"
57
"os"
68
"testing"
79
)
@@ -70,3 +72,13 @@ func TestAutoExpand(t *testing.T) {
7072
}
7173

7274
}
75+
76+
func TestWrappedError(t *testing.T) {
77+
_, err := Exec(nil, nil, nil, os.Args[0]+"-doesnotexist", "-printArgs", "foo")
78+
if err == nil {
79+
t.Fatalf("Expected to fail running %s", os.Args[0]+"-doesnotexist")
80+
}
81+
if !errors.Is(err, fs.ErrNotExist) {
82+
t.Fatalf("Expected error to be like ErrNotExist, got %#v", err)
83+
}
84+
}

sh/helpers.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,28 @@ func Rm(path string) error {
1313
if err == nil || os.IsNotExist(err) {
1414
return nil
1515
}
16-
return fmt.Errorf(`failed to remove %s: %v`, path, err)
16+
return fmt.Errorf(`failed to remove %s: %w`, path, err)
1717
}
1818

1919
// Copy robustly copies the source file to the destination, overwriting the destination if necessary.
2020
func Copy(dst string, src string) error {
2121
from, err := os.Open(src)
2222
if err != nil {
23-
return fmt.Errorf(`can't copy %s: %v`, src, err)
23+
return fmt.Errorf(`can't copy %s: %w`, src, err)
2424
}
2525
defer from.Close()
2626
finfo, err := from.Stat()
2727
if err != nil {
28-
return fmt.Errorf(`can't stat %s: %v`, src, err)
28+
return fmt.Errorf(`can't stat %s: %w`, src, err)
2929
}
3030
to, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, finfo.Mode())
3131
if err != nil {
32-
return fmt.Errorf(`can't copy to %s: %v`, dst, err)
32+
return fmt.Errorf(`can't copy to %s: %w`, dst, err)
3333
}
3434
defer to.Close()
3535
_, err = io.Copy(to, from)
3636
if err != nil {
37-
return fmt.Errorf(`error copying %s to %s: %v`, src, dst, err)
37+
return fmt.Errorf(`error copying %s to %s: %w`, src, dst, err)
3838
}
3939
return nil
4040
}

sh/helpers_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ import (
1717
func compareFiles(file1 string, file2 string) error {
1818
s1, err := os.Stat(file1)
1919
if err != nil {
20-
return fmt.Errorf("can't stat %s: %v", file1, err)
20+
return fmt.Errorf("can't stat %s: %w", file1, err)
2121
}
2222
s2, err := os.Stat(file2)
2323
if err != nil {
24-
return fmt.Errorf("can't stat %s: %v", file2, err)
24+
return fmt.Errorf("can't stat %s: %w", file2, err)
2525
}
2626
if s1.Size() != s2.Size() {
2727
return fmt.Errorf("files %s and %s have different sizes: %d vs %d", file1, file2, s1.Size(), s2.Size())
@@ -31,11 +31,11 @@ func compareFiles(file1 string, file2 string) error {
3131
}
3232
f1bytes, err := ioutil.ReadFile(file1)
3333
if err != nil {
34-
return fmt.Errorf("can't read %s: %v", file1, err)
34+
return fmt.Errorf("can't read %s: %w", file1, err)
3535
}
3636
f2bytes, err := ioutil.ReadFile(file2)
3737
if err != nil {
38-
return fmt.Errorf("can't read %s: %v", file2, err)
38+
return fmt.Errorf("can't read %s: %w", file2, err)
3939
}
4040
if !bytes.Equal(f1bytes, f2bytes) {
4141
return fmt.Errorf("files %s and %s have different contents", file1, file2)

0 commit comments

Comments
 (0)