Merge pull request #4089 from greatroar/errors

Clean up error handling further

cmd/restic/cmd_check.go

@@ -268,7 +268,7 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args
 		if checker.IsOrphanedPack(err) {
 			orphanedPacks++
 			Verbosef("%v\n", err)
-		} else if _, ok := err.(*checker.ErrLegacyLayout); ok {
+		} else if err == checker.ErrLegacyLayout {
 			Verbosef("repository still uses the S3 legacy layout\nPlease run `restic migrate s3legacy` to correct this.\n")
 		} else {
 			errorsFound = true

internal/archiver/archiver.go

@@ -183,7 +183,7 @@ func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo)
 	}
 	// overwrite name to match that within the snapshot
 	node.Name = path.Base(snPath)
-	return node, errors.Wrap(err, "NodeFromFileInfo")
+	return node, errors.WithStack(err)
 }
 
 // loadSubtree tries to load the subtree referenced by node. In case of an error, nil is returned.
@@ -352,7 +352,7 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 		debug.Log("lstat() for %v returned error: %v", target, err)
 		err = arch.error(abstarget, err)
 		if err != nil {
-			return FutureNode{}, false, errors.Wrap(err, "Lstat")
+			return FutureNode{}, false, errors.WithStack(err)
 		}
 		return FutureNode{}, true, nil
 	}
@@ -404,7 +404,7 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 			debug.Log("Openfile() for %v returned error: %v", target, err)
 			err = arch.error(abstarget, err)
 			if err != nil {
-				return FutureNode{}, false, errors.Wrap(err, "Lstat")
+				return FutureNode{}, false, errors.WithStack(err)
 			}
 			return FutureNode{}, true, nil
 		}
@@ -415,7 +415,7 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 			_ = file.Close()
 			err = arch.error(abstarget, err)
 			if err != nil {
-				return FutureNode{}, false, errors.Wrap(err, "Lstat")
+				return FutureNode{}, false, errors.WithStack(err)
 			}
 			return FutureNode{}, true, nil
 		}
@@ -524,7 +524,7 @@ func join(elem ...string) string {
 func (arch *Archiver) statDir(dir string) (os.FileInfo, error) {
 	fi, err := arch.FS.Stat(dir)
 	if err != nil {
-		return nil, errors.Wrap(err, "Lstat")
+		return nil, errors.WithStack(err)
 	}
 
 	tpe := fi.Mode() & (os.ModeType | os.ModeCharDevice)
@@ -626,7 +626,7 @@ func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree,
 func readdirnames(filesystem fs.FS, dir string, flags int) ([]string, error) {
 	f, err := filesystem.OpenFile(dir, fs.O_RDONLY|flags, 0)
 	if err != nil {
-		return nil, errors.Wrap(err, "Open")
+		return nil, errors.WithStack(err)
 	}
 
 	entries, err := f.Readdirnames(-1)

internal/backend/rest/config.go

@@ -34,9 +34,8 @@ func ParseConfig(s string) (interface{}, error) {
 	s = prepareURL(s)
 
 	u, err := url.Parse(s)
-
 	if err != nil {
-		return nil, errors.Wrap(err, "url.Parse")
+		return nil, errors.WithStack(err)
 	}
 
 	cfg := NewConfig()

internal/backend/s3/config.go

@@ -52,7 +52,7 @@ func ParseConfig(s string) (interface{}, error) {
 		// bucket name and prefix
 		url, err := url.Parse(s[3:])
 		if err != nil {
-			return nil, errors.Wrap(err, "url.Parse")
+			return nil, errors.WithStack(err)
 		}
 
 		if url.Path == "" {

internal/backend/sftp/config.go

@@ -42,7 +42,7 @@ func ParseConfig(s string) (interface{}, error) {
 		// parse the "sftp://user@host/path" url format
 		url, err := url.Parse(s)
 		if err != nil {
-			return nil, errors.Wrap(err, "url.Parse")
+			return nil, errors.WithStack(err)
 		}
 		if url.User != nil {
 			user = url.User.Username()

internal/checker/checker.go

@@ -59,11 +59,7 @@ func New(repo restic.Repository, trackUnused bool) *Checker {
 }
 
 // ErrLegacyLayout is returned when the repository uses the S3 legacy layout.
-type ErrLegacyLayout struct{}
+var ErrLegacyLayout = errors.New("repository uses S3 legacy layout")
-
-func (e *ErrLegacyLayout) Error() string {
-	return "repository uses S3 legacy layout"
-}
 
 // ErrDuplicatePacks is returned when a pack is found in more than one index.
 type ErrDuplicatePacks struct {
@@ -231,7 +227,7 @@ func (c *Checker) Packs(ctx context.Context, errChan chan<- error) {
 	defer close(errChan)
 
 	if isS3Legacy(c.repo.Backend()) {
-		errChan <- &ErrLegacyLayout{}
+		errChan <- ErrLegacyLayout
 	}
 
 	debug.Log("checking for %d packs", len(c.packs))

internal/fs/fs_reader.go

@@ -1,6 +1,7 @@
 package fs
 
 import (
+	"fmt"
 	"io"
 	"os"
 	"path"
@@ -47,7 +48,7 @@ func (fs *Reader) Open(name string) (f File, err error) {
 		})
 
 		if f == nil {
-			return nil, syscall.EIO
+			return nil, pathError("open", name, syscall.EIO)
 		}
 
 		return f, nil
@@ -58,7 +59,7 @@ func (fs *Reader) Open(name string) (f File, err error) {
 		return f, nil
 	}
 
-	return nil, syscall.ENOENT
+	return nil, pathError("open", name, syscall.ENOENT)
 }
 
 func (fs *Reader) fi() os.FileInfo {
@@ -74,10 +75,11 @@ func (fs *Reader) fi() os.FileInfo {
 // or Create instead.  It opens the named file with specified flag
 // (O_RDONLY etc.) and perm, (0666 etc.) if applicable.  If successful,
 // methods on the returned File can be used for I/O.
-// If there is an error, it will be of type *PathError.
+// If there is an error, it will be of type *os.PathError.
 func (fs *Reader) OpenFile(name string, flag int, perm os.FileMode) (f File, err error) {
 	if flag & ^(O_RDONLY|O_NOFOLLOW) != 0 {
-		return nil, errors.Errorf("invalid combination of flags 0x%x", flag)
+		return nil, pathError("open", name,
+			fmt.Errorf("invalid combination of flags 0x%x", flag))
 	}
 
 	fs.open.Do(func() {
@@ -85,14 +87,14 @@ func (fs *Reader) OpenFile(name string, flag int, perm os.FileMode) (f File, err
 	})
 
 	if f == nil {
-		return nil, syscall.EIO
+		return nil, pathError("open", name, syscall.EIO)
 	}
 
 	return f, nil
 }
 
 // Stat returns a FileInfo describing the named file. If there is an error, it
-// will be of type *PathError.
+// will be of type *os.PathError.
 func (fs *Reader) Stat(name string) (os.FileInfo, error) {
 	return fs.Lstat(name)
 }
@@ -100,7 +102,7 @@ func (fs *Reader) Stat(name string) (os.FileInfo, error) {
 // Lstat returns the FileInfo structure describing the named file.
 // If the file is a symbolic link, the returned FileInfo
 // describes the symbolic link.  Lstat makes no attempt to follow the link.
-// If there is an error, it will be of type *PathError.
+// If there is an error, it will be of type *os.PathError.
 func (fs *Reader) Lstat(name string) (os.FileInfo, error) {
 	getDirInfo := func(name string) os.FileInfo {
 		fi := fakeFileInfo{
@@ -130,7 +132,7 @@ func (fs *Reader) Lstat(name string) (os.FileInfo, error) {
 		dir = fs.Dir(dir)
 	}
 
-	return nil, os.ErrNotExist
+	return nil, pathError("lstat", name, os.ErrNotExist)
 }
 
 // Join joins any number of path elements into a single path, adding a
@@ -207,11 +209,7 @@ func (r *readerFile) Read(p []byte) (int, error) {
 
 	// return an error if we did not read any data
 	if err == io.EOF && !r.AllowEmptyFile && !r.bytesRead {
-		return n, &os.PathError{
+		return n, pathError("read", r.fakeFile.name, ErrFileEmpty)
-			Path: r.fakeFile.name,
-			Op:   "read",
-			Err:  ErrFileEmpty,
-		}
 	}
 
 	return n, err
@@ -239,19 +237,19 @@ func (f fakeFile) Fd() uintptr {
 }
 
 func (f fakeFile) Readdirnames(n int) ([]string, error) {
-	return nil, os.ErrInvalid
+	return nil, pathError("readdirnames", f.name, os.ErrInvalid)
 }
 
 func (f fakeFile) Readdir(n int) ([]os.FileInfo, error) {
-	return nil, os.ErrInvalid
+	return nil, pathError("readdir", f.name, os.ErrInvalid)
 }
 
 func (f fakeFile) Seek(int64, int) (int64, error) {
-	return 0, os.ErrInvalid
+	return 0, pathError("seek", f.name, os.ErrInvalid)
 }
 
 func (f fakeFile) Read(p []byte) (int, error) {
-	return 0, os.ErrInvalid
+	return 0, pathError("read", f.name, os.ErrInvalid)
 }
 
 func (f fakeFile) Close() error {
@@ -274,7 +272,7 @@ type fakeDir struct {
 
 func (d fakeDir) Readdirnames(n int) ([]string, error) {
 	if n > 0 {
-		return nil, errors.New("not implemented")
+		return nil, pathError("readdirnames", d.name, errors.New("not implemented"))
 	}
 	names := make([]string, 0, len(d.entries))
 	for _, entry := range d.entries {
@@ -286,7 +284,7 @@ func (d fakeDir) Readdirnames(n int) ([]string, error) {
 
 func (d fakeDir) Readdir(n int) ([]os.FileInfo, error) {
 	if n > 0 {
-		return nil, errors.New("not implemented")
+		return nil, pathError("readdir", d.name, errors.New("not implemented"))
 	}
 	return d.entries, nil
 }
@@ -322,3 +320,7 @@ func (fi fakeFileInfo) IsDir() bool {
 func (fi fakeFileInfo) Sys() interface{} {
 	return nil
 }
+
+func pathError(op, name string, err error) *os.PathError {
+	return &os.PathError{Op: op, Path: name, Err: err}
+}

internal/fs/fs_reader_test.go

@@ -2,6 +2,7 @@ package fs
 
 import (
 	"bytes"
+	"errors"
 	"io"
 	"os"
 	"path"
@@ -288,7 +289,7 @@ func TestFSReader(t *testing.T) {
 			name: "dir/Lstat-error-not-exist",
 			f: func(t *testing.T, fs FS) {
 				_, err := fs.Lstat("other")
-				if err != os.ErrNotExist {
+				if !errors.Is(err, os.ErrNotExist) {
 					t.Fatal(err)
 				}
 			},

internal/repository/packer_manager.go

@@ -110,7 +110,7 @@ func (r *packerManager) newPacker() (packer *Packer, err error) {
 	debug.Log("create new pack")
 	tmpfile, err := fs.TempFile("", "restic-temp-pack-")
 	if err != nil {
-		return nil, errors.Wrap(err, "fs.TempFile")
+		return nil, errors.WithStack(err)
 	}
 
 	bufWr := bufio.NewWriter(tmpfile)
@@ -183,7 +183,7 @@ func (r *Repository) savePacker(ctx context.Context, t restic.BlobType, p *Packe
 	if runtime.GOOS != "windows" {
 		err = fs.RemoveIfExists(p.tmpfile.Name())
 		if err != nil {
-			return errors.Wrap(err, "Remove")
+			return errors.WithStack(err)
 		}
 	}
 

internal/restic/node.go

@@ -191,14 +191,14 @@ func (node Node) restoreMetadata(path string) error {
 			debug.Log("not running as root, ignoring lchown permission error for %v: %v",
 				path, err)
 		} else {
-			firsterr = errors.Wrap(err, "Lchown")
+			firsterr = errors.WithStack(err)
 		}
 	}
 
 	if node.Type != "symlink" {
 		if err := fs.Chmod(path, node.Mode); err != nil {
 			if firsterr != nil {
-				firsterr = errors.Wrap(err, "Chmod")
+				firsterr = errors.WithStack(err)
 			}
 		}
 	}
@@ -250,7 +250,7 @@ func (node Node) RestoreTimestamps(path string) error {
 func (node Node) createDirAt(path string) error {
 	err := fs.Mkdir(path, node.Mode)
 	if err != nil && !os.IsExist(err) {
-		return errors.Wrap(err, "Mkdir")
+		return errors.WithStack(err)
 	}
 
 	return nil
@@ -259,7 +259,7 @@ func (node Node) createDirAt(path string) error {
 func (node Node) createFileAt(ctx context.Context, path string, repo Repository) error {
 	f, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
 	if err != nil {
-		return errors.Wrap(err, "OpenFile")
+		return errors.WithStack(err)
 	}
 
 	err = node.writeNodeContent(ctx, repo, f)
@@ -270,7 +270,7 @@ func (node Node) createFileAt(ctx context.Context, path string, repo Repository)
 	}
 
 	if closeErr != nil {
-		return errors.Wrap(closeErr, "Close")
+		return errors.WithStack(closeErr)
 	}
 
 	return nil
@@ -286,7 +286,7 @@ func (node Node) writeNodeContent(ctx context.Context, repo Repository, f *os.Fi
 
 		_, err = f.Write(buf)
 		if err != nil {
-			return errors.Wrap(err, "Write")
+			return errors.WithStack(err)
 		}
 	}
 
@@ -300,7 +300,7 @@ func (node Node) createSymlinkAt(path string) error {
 	}
 
 	if err := fs.Symlink(node.LinkTarget, path); err != nil {
-		return errors.Wrap(err, "Symlink")
+		return errors.WithStack(err)
 	}
 
 	return nil
@@ -591,7 +591,7 @@ func (node *Node) fillExtra(path string, fi os.FileInfo) error {
 		node.LinkTarget, err = fs.Readlink(path)
 		node.Links = uint64(stat.nlink())
 		if err != nil {
-			return errors.Wrap(err, "Readlink")
+			return errors.WithStack(err)
 		}
 	case "dev":
 		node.Device = uint64(stat.rdev())

internal/restic/node_linux.go

@@ -13,7 +13,7 @@ import (
 func (node Node) restoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error {
 	dir, err := fs.Open(filepath.Dir(path))
 	if err != nil {
-		return errors.Wrap(err, "Open")
+		return errors.WithStack(err)
 	}
 
 	times := []unix.Timespec{

internal/restorer/restorer.go

@@ -184,7 +184,7 @@ func (res *Restorer) restoreHardlinkAt(node *restic.Node, target, path, location
 	}
 	err := fs.Link(target, path)
 	if err != nil {
-		return errors.Wrap(err, "CreateHardlink")
+		return errors.WithStack(err)
 	}
 	// TODO investigate if hardlinks have separate metadata on any supported system
 	return res.restoreNodeMetadataTo(node, path, location)