From 07e5c383613292bbc5f52e8f6a13cb7b095d1ff2 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 8 Oct 2022 12:37:18 +0200 Subject: [PATCH] errors: Drop Cause in favor of Go 1.13 error handling The only use cases in the code were in errors.IsFatal, backend/b2, which needs a workaround, and backend.ParseLayout. The last of these requires all backends to implement error unwrapping in IsNotExist. All backends except gs already did that. --- cmd/restic/integration_test.go | 12 ++++++------ internal/backend/b2/b2.go | 11 +++++++++-- internal/backend/gs/gs.go | 9 ++------- internal/backend/layout.go | 2 +- internal/errors/errors.go | 30 +++++------------------------- internal/errors/fatal.go | 25 ++++++++----------------- internal/restic/backend.go | 3 +++ 7 files changed, 34 insertions(+), 58 deletions(-) diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 307937cc8..70d554632 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -1197,7 +1197,7 @@ func TestRestoreFilter(t *testing.T) { if ok, _ := filter.Match(pat, filepath.Base(testFile.name)); !ok { rtest.OK(t, err) } else { - rtest.Assert(t, os.IsNotExist(errors.Cause(err)), + rtest.Assert(t, os.IsNotExist(err), "expected %v to not exist in restore step %v, but it exists, err %v", testFile.name, i+1, err) } } @@ -1283,15 +1283,15 @@ func TestRestoreLatest(t *testing.T) { testRunRestoreLatest(t, env.gopts, filepath.Join(env.base, "restore1"), []string{filepath.Dir(p1)}, nil) rtest.OK(t, testFileSize(p1rAbs, int64(102))) - if _, err := os.Stat(p2rAbs); os.IsNotExist(errors.Cause(err)) { - rtest.Assert(t, os.IsNotExist(errors.Cause(err)), + if _, err := os.Stat(p2rAbs); os.IsNotExist(err) { + rtest.Assert(t, os.IsNotExist(err), "expected %v to not exist in restore, but it exists, err %v", p2rAbs, err) } testRunRestoreLatest(t, env.gopts, filepath.Join(env.base, "restore2"), []string{filepath.Dir(p2)}, nil) rtest.OK(t, testFileSize(p2rAbs, int64(103))) - if _, err := os.Stat(p1rAbs); os.IsNotExist(errors.Cause(err)) { - rtest.Assert(t, os.IsNotExist(errors.Cause(err)), + if _, err := os.Stat(p1rAbs); os.IsNotExist(err) { + rtest.Assert(t, os.IsNotExist(err), "expected %v to not exist in restore, but it exists, err %v", p1rAbs, err) } } @@ -1861,7 +1861,7 @@ func TestHardLink(t *testing.T) { datafile := filepath.Join("testdata", "test.hl.tar.gz") fd, err := os.Open(datafile) - if os.IsNotExist(errors.Cause(err)) { + if os.IsNotExist(err) { t.Skipf("unable to find data file %q, skipping", datafile) return } diff --git a/internal/backend/b2/b2.go b/internal/backend/b2/b2.go index f81d954bf..4a6c79abb 100644 --- a/internal/backend/b2/b2.go +++ b/internal/backend/b2/b2.go @@ -184,7 +184,14 @@ func (be *b2Backend) HasAtomicReplace() bool { // IsNotExist returns true if the error is caused by a non-existing file. func (be *b2Backend) IsNotExist(err error) bool { - return b2.IsNotExist(errors.Cause(err)) + // blazer/b2 does not export its error types and values, + // so we can't use errors.{As,Is}. + for ; err != nil; err = errors.Unwrap(err) { + if b2.IsNotExist(err) { + return true + } + } + return false } // Load runs fn with a reader that yields the contents of the file at h at the @@ -386,7 +393,7 @@ func (be *b2Backend) Delete(ctx context.Context) error { } } err := be.Remove(ctx, restic.Handle{Type: restic.ConfigFile}) - if err != nil && b2.IsNotExist(errors.Cause(err)) { + if err != nil && be.IsNotExist(err) { err = nil } diff --git a/internal/backend/gs/gs.go b/internal/backend/gs/gs.go index 3c68b55e7..8ffc96af8 100644 --- a/internal/backend/gs/gs.go +++ b/internal/backend/gs/gs.go @@ -174,13 +174,8 @@ func (be *Backend) IsNotExist(err error) bool { return true } - if er, ok := err.(*googleapi.Error); ok { - if er.Code == 404 { - return true - } - } - - return false + var gerr *googleapi.Error + return errors.As(err, &gerr) && gerr.Code == 404 } // Join combines path components with slashes. diff --git a/internal/backend/layout.go b/internal/backend/layout.go index ebd54e4af..421e85e8e 100644 --- a/internal/backend/layout.go +++ b/internal/backend/layout.go @@ -71,7 +71,7 @@ var backendFilename = regexp.MustCompile(fmt.Sprintf("^[a-fA-F0-9]{%d}$", backen func hasBackendFile(ctx context.Context, fs Filesystem, dir string) (bool, error) { entries, err := fs.ReadDir(ctx, dir) - if err != nil && fs.IsNotExist(errors.Cause(err)) { + if err != nil && fs.IsNotExist(err) { return false, nil } diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 021da72c5..4fd94cbf2 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -1,9 +1,8 @@ package errors import ( - "net/url" + stderrors "errors" - "github.com/cenkalti/backoff/v4" "github.com/pkg/errors" ) @@ -29,29 +28,10 @@ var WithMessage = errors.WithMessage var WithStack = errors.WithStack -// Cause returns the cause of an error. It will also unwrap certain errors, -// e.g. *url.Error returned by the net/http client. -func Cause(err error) error { - type Causer interface { - Cause() error - } - - for { - switch e := err.(type) { - case Causer: // github.com/pkg/errors - err = e.Cause() - case *backoff.PermanentError: - err = e.Err - case *url.Error: - err = e.Err - default: - return err - } - } -} - // Go 1.13-style error handling. -func As(err error, tgt interface{}) bool { return errors.As(err, tgt) } +func As(err error, tgt interface{}) bool { return stderrors.As(err, tgt) } -func Is(x, y error) bool { return errors.Is(x, y) } +func Is(x, y error) bool { return stderrors.Is(x, y) } + +func Unwrap(err error) error { return stderrors.Unwrap(err) } diff --git a/internal/errors/fatal.go b/internal/errors/fatal.go index 5fb615cf1..9370a68d7 100644 --- a/internal/errors/fatal.go +++ b/internal/errors/fatal.go @@ -1,6 +1,9 @@ package errors -import "fmt" +import ( + "errors" + "fmt" +) // fatalError is an error that should be printed to the user, then the program // should exit with an error code. @@ -10,31 +13,19 @@ func (e fatalError) Error() string { return string(e) } -func (e fatalError) Fatal() bool { - return true -} - -// Fataler is an error which should be printed to the user directly. -// Afterwards, the program should exit with an error. -type Fataler interface { - Fatal() bool -} - // IsFatal returns true if err is a fatal message that should be printed to the // user. Then, the program should exit. func IsFatal(err error) bool { - // unwrap "Wrap" method - err = Cause(err) - e, ok := err.(Fataler) - return ok && e.Fatal() + var fatal fatalError + return errors.As(err, &fatal) } -// Fatal returns a wrapped error which implements the Fataler interface. +// Fatal returns an error that is marked fatal. func Fatal(s string) error { return Wrap(fatalError(s), "Fatal") } -// Fatalf returns an error which implements the Fataler interface. +// Fatalf returns an error that is marked fatal. func Fatalf(s string, data ...interface{}) error { return Wrap(fatalError(fmt.Sprintf(s, data...)), "Fatal") } diff --git a/internal/restic/backend.go b/internal/restic/backend.go index 6ec10e685..d8b280a3a 100644 --- a/internal/restic/backend.go +++ b/internal/restic/backend.go @@ -64,6 +64,9 @@ type Backend interface { // IsNotExist returns true if the error was caused by a non-existing file // in the backend. + // + // The argument may be a wrapped error. The implementation is responsible + // for unwrapping it. IsNotExist(err error) bool // Delete removes all data in the backend.