Error handling improvements (#71)

* Remove `emperror.dev/errors`, remove all `errors#Wrap` and `errors#WithStack` calls
* Improve logging in `server/backup.go`
This commit is contained in:
Matthew Penner
2020-11-28 16:57:10 -07:00
committed by GitHub
parent 40c70673cd
commit de51fd1c51
55 changed files with 326 additions and 339 deletions

View File

@@ -2,7 +2,6 @@ package filesystem
import (
"context"
"emperror.dev/errors"
"fmt"
"github.com/karrick/godirwalk"
"github.com/pterodactyl/wings/server/backup"
@@ -27,7 +26,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In
i, err := ignore.CompileIgnoreLines(ignored...)
if err != nil {
return nil, errors.WithStack(err)
return nil, err
}
// Walk through all of the files and directories on a server. This callback only returns
@@ -65,7 +64,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In
},
})
return inc, errors.WithStackIf(err)
return inc, err
}
// Compresses all of the files matching the given paths in the specified directory. This function
@@ -141,7 +140,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er
d := path.Join(cleanedRootDir, fmt.Sprintf("archive-%s.tar.gz", strings.ReplaceAll(time.Now().Format(time.RFC3339), ":", "")))
if err := a.Create(d, context.Background()); err != nil {
return nil, errors.WithStackIf(err)
return nil, err
}
f, err := os.Stat(d)

View File

@@ -4,9 +4,9 @@ import (
"archive/tar"
"archive/zip"
"compress/gzip"
"emperror.dev/errors"
"fmt"
"github.com/mholt/archiver/v3"
"github.com/pkg/errors"
"os"
"path/filepath"
"reflect"
@@ -47,10 +47,10 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b
return false, ErrUnknownArchiveFormat
}
return false, errors.WithStackIf(err)
return false, err
}
return true, errors.WithStackIf(err)
return true, err
}
// Decompress a file in a given directory by using the archiver tool to infer the file
@@ -60,12 +60,12 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b
func (fs *Filesystem) DecompressFile(dir string, file string) error {
source, err := fs.SafePath(filepath.Join(dir, file))
if err != nil {
return errors.WithStackIf(err)
return err
}
// Make sure the file exists basically.
if _, err := os.Stat(source); err != nil {
return errors.WithStackIf(err)
return err
}
// Walk over all of the files spinning up an additional go-routine for each file we've encountered
@@ -93,17 +93,17 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error {
p, err := fs.SafePath(filepath.Join(dir, name))
if err != nil {
return errors.WrapIf(err, "failed to generate a safe path to server file")
return errors.WithMessage(err, "failed to generate a safe path to server file")
}
return errors.WrapIf(fs.Writefile(p, f), "could not extract file from archive")
return errors.WithMessage(fs.Writefile(p, f), "could not extract file from archive")
})
if err != nil {
if strings.HasPrefix(err.Error(), "format ") {
return errors.WithStackIf(ErrUnknownArchiveFormat)
return ErrUnknownArchiveFormat
}
return errors.WithStackIf(err)
return err
}
return nil

View File

@@ -1,7 +1,6 @@
package filesystem
import (
"emperror.dev/errors"
"github.com/apex/log"
"github.com/karrick/godirwalk"
"sync"
@@ -158,7 +157,7 @@ func (fs *Filesystem) updateCachedDiskUsage() (int64, error) {
func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
d, err := fs.SafePath(dir)
if err != nil {
return 0, errors.WithStackIf(err)
return 0, err
}
var size int64
@@ -189,7 +188,7 @@ func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
},
})
return size, errors.WithStackIf(err)
return size, err
}
// Helper function to determine if a server has space available for a file of a given size.
@@ -202,7 +201,7 @@ func (fs *Filesystem) hasSpaceFor(size int64) error {
s, err := fs.DiskUsage(true)
if err != nil {
return errors.WithStackIf(err)
return err
}
if (s + size) > fs.MaxDisk() {

View File

@@ -1,16 +1,16 @@
package filesystem
import (
"emperror.dev/errors"
"fmt"
"github.com/apex/log"
"github.com/pkg/errors"
"os"
"path/filepath"
)
var ErrIsDirectory = errors.Sentinel("filesystem: is a directory")
var ErrNotEnoughDiskSpace = errors.Sentinel("filesystem: not enough disk space")
var ErrUnknownArchiveFormat = errors.Sentinel("filesystem: unknown archive format")
var ErrIsDirectory = errors.New("filesystem: is a directory")
var ErrNotEnoughDiskSpace = errors.New("filesystem: not enough disk space")
var ErrUnknownArchiveFormat = errors.New("filesystem: unknown archive format")
type BadPathResolutionError struct {
path string
@@ -23,6 +23,7 @@ func (b *BadPathResolutionError) Error() string {
if r == "" {
r = "<empty>"
}
return fmt.Sprintf("filesystem: server path [%s] resolves to a location outside the server root: %s", b.path, r)
}
@@ -57,7 +58,7 @@ func (fs *Filesystem) error(err error) *log.Entry {
// for the remainder of the directory. This is assuming an os.FileInfo struct was even returned.
func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error {
if !IsBadPathResolutionError(err) {
return errors.WithStackIf(err)
return err
}
if f != nil && f.IsDir() {

View File

@@ -2,9 +2,9 @@ package filesystem
import (
"bufio"
"emperror.dev/errors"
"github.com/gabriel-vasile/mimetype"
"github.com/karrick/godirwalk"
"github.com/pkg/errors"
"github.com/pterodactyl/wings/config"
"github.com/pterodactyl/wings/system"
"io"
@@ -60,27 +60,27 @@ func (fs *Filesystem) Readfile(p string, w io.Writer) error {
}
if st, err := os.Stat(cleaned); err != nil {
return errors.WithStack(err)
return err
} else if st.IsDir() {
return errors.WithStack(ErrIsDirectory)
return ErrIsDirectory
}
f, err := os.Open(cleaned)
if err != nil {
return errors.WithStack(err)
return err
}
defer f.Close()
_, err = bufio.NewReader(f).WriteTo(w)
return errors.WithStack(err)
return err
}
// Writes a file to the system. If the file does not already exist one will be created.
func (fs *Filesystem) Writefile(p string, r io.Reader) error {
cleaned, err := fs.SafePath(p)
if err != nil {
return errors.WithStackIf(err)
return err
}
var currentSize int64
@@ -88,19 +88,19 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
// to it and an empty file. We'll then write to it later on after this completes.
if stat, err := os.Stat(cleaned); err != nil {
if !os.IsNotExist(err) {
return errors.WithStackIf(err)
return err
}
if err := os.MkdirAll(filepath.Dir(cleaned), 0755); err != nil {
return errors.WithStackIf(err)
return err
}
if err := fs.Chown(filepath.Dir(cleaned)); err != nil {
return errors.WithStackIf(err)
return err
}
} else {
if stat.IsDir() {
return errors.WithStack(ErrIsDirectory)
return ErrIsDirectory
}
currentSize = stat.Size()
@@ -119,7 +119,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
// truncate the existing file.
file, err := o.open(cleaned, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0644)
if err != nil {
return errors.WithStackIf(err)
return err
}
defer file.Close()
@@ -138,7 +138,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
func (fs *Filesystem) CreateDirectory(name string, p string) error {
cleaned, err := fs.SafePath(path.Join(p, name))
if err != nil {
return errors.WithStackIf(err)
return err
}
return os.MkdirAll(cleaned, 0755)
@@ -148,12 +148,12 @@ func (fs *Filesystem) CreateDirectory(name string, p string) error {
func (fs *Filesystem) Rename(from string, to string) error {
cleanedFrom, err := fs.SafePath(from)
if err != nil {
return errors.WithStackIf(err)
return err
}
cleanedTo, err := fs.SafePath(to)
if err != nil {
return errors.WithStackIf(err)
return err
}
// If the target file or directory already exists the rename function will fail, so just
@@ -171,7 +171,7 @@ func (fs *Filesystem) Rename(from string, to string) error {
// we're not at the root directory level.
if d != fs.Path() {
if mkerr := os.MkdirAll(d, 0755); mkerr != nil {
return errors.WrapIf(mkerr, "failed to create directory structure for file rename")
return errors.WithMessage(mkerr, "failed to create directory structure for file rename")
}
}
@@ -185,7 +185,7 @@ func (fs *Filesystem) Rename(from string, to string) error {
func (fs *Filesystem) Chown(path string) error {
cleaned, err := fs.SafePath(path)
if err != nil {
return errors.WithStackIf(err)
return err
}
if fs.isTest {
@@ -197,7 +197,7 @@ func (fs *Filesystem) Chown(path string) error {
// Start by just chowning the initial path that we received.
if err := os.Chown(cleaned, uid, gid); err != nil {
return errors.WithStackIf(err)
return err
}
// If this is not a directory we can now return from the function, there is nothing
@@ -249,7 +249,7 @@ func (fs *Filesystem) findCopySuffix(dir string, name string, extension string)
// does exist, we'll just continue to the next loop and try again.
if _, err := fs.Stat(path.Join(dir, n)); err != nil {
if !errors.Is(err, os.ErrNotExist) {
return "", errors.WithStackIf(err)
return "", err
}
break
@@ -268,12 +268,12 @@ func (fs *Filesystem) findCopySuffix(dir string, name string, extension string)
func (fs *Filesystem) Copy(p string) error {
cleaned, err := fs.SafePath(p)
if err != nil {
return errors.WithStackIf(err)
return err
}
s, err := os.Stat(cleaned)
if err != nil {
return errors.WithStackIf(err)
return err
} else if s.IsDir() || !s.Mode().IsRegular() {
// If this is a directory or not a regular file, just throw a not-exist error
// since anything calling this function should understand what that means.
@@ -300,7 +300,7 @@ func (fs *Filesystem) Copy(p string) error {
source, err := os.Open(cleaned)
if err != nil {
return errors.WithStackIf(err)
return err
}
defer source.Close()

View File

@@ -2,7 +2,6 @@ package filesystem
import (
"context"
"emperror.dev/errors"
"golang.org/x/sync/errgroup"
"os"
"path/filepath"
@@ -26,7 +25,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) {
// is truly pointing to.
ep, err := filepath.EvalSymlinks(r)
if err != nil && !os.IsNotExist(err) {
return "", errors.WithStackIf(err)
return "", err
} else if os.IsNotExist(err) {
// The requested directory doesn't exist, so at this point we need to iterate up the
// path chain until we hit a directory that _does_ exist and can be validated.
@@ -138,5 +137,5 @@ func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) {
}
// Block until all of the routines finish and have returned a value.
return cleaned, errors.WithStackIf(g.Wait())
return cleaned, g.Wait()
}

View File

@@ -2,8 +2,8 @@ package filesystem
import (
"bytes"
"emperror.dev/errors"
. "github.com/franela/goblin"
"github.com/pkg/errors"
"os"
"path/filepath"
"testing"

View File

@@ -1,7 +1,6 @@
package filesystem
import (
"emperror.dev/errors"
"encoding/json"
"github.com/gabriel-vasile/mimetype"
"os"
@@ -51,14 +50,14 @@ func (fs *Filesystem) Stat(p string) (*Stat, error) {
func (fs *Filesystem) unsafeStat(p string) (*Stat, error) {
s, err := os.Stat(p)
if err != nil {
return nil, errors.WithStackIf(err)
return nil, err
}
var m *mimetype.MIME
if !s.IsDir() {
m, err = mimetype.DetectFile(p)
if err != nil {
return nil, errors.WithStackIf(err)
return nil, err
}
}