From a47834da8ad9980456c6d1192534a3b6021ee61b Mon Sep 17 00:00:00 2001 From: Simone Gotti Date: Wed, 2 Mar 2022 13:23:32 +0100 Subject: [PATCH] *: replace os errors check functions with errors.Is Since we're wrapping also internal errors, if a function returns a wrapped os pkg error (like os.ErrNotExists) and the caller function uses the os error check functions (like os.IsNotExist) it won't work since (like explained in the os pkg comment) it won't unwrap the error. Fix this by using errors.Is checks. --- internal/git-save/save.go | 4 ++-- internal/objectstorage/posix.go | 10 +++++----- internal/objectstorage/posixflat.go | 14 +++++++------- internal/services/executor/api.go | 4 ++-- internal/services/executor/executor.go | 2 +- internal/services/gitserver/main.go | 12 ++++++------ internal/toolbox/unarchive/unarchive.go | 8 ++++---- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/internal/git-save/save.go b/internal/git-save/save.go index 315e098..8cdb2b6 100644 --- a/internal/git-save/save.go +++ b/internal/git-save/save.go @@ -51,10 +51,10 @@ func copyFile(src, dest string) error { func fileExists(path string) (bool, error) { _, err := os.Stat(path) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return false, errors.WithStack(err) } - return !os.IsNotExist(err), nil + return !errors.Is(err, os.ErrNotExist), nil } // GitDir returns the git dir relative to the working dir diff --git a/internal/objectstorage/posix.go b/internal/objectstorage/posix.go index 55f4205..51fe51f 100644 --- a/internal/objectstorage/posix.go +++ b/internal/objectstorage/posix.go @@ -64,7 +64,7 @@ func (s *PosixStorage) Stat(p string) (*ObjectInfo, error) { fi, err := os.Stat(fspath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil, NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return nil, errors.WithStack(err) @@ -80,7 +80,7 @@ func (s *PosixStorage) ReadObject(p string) (ReadSeekCloser, error) { } f, err := os.Open(fspath) - if err != nil && os.IsNotExist(err) { + if err != nil && errors.Is(err, os.ErrNotExist) { return nil, NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return f, errors.WithStack(err) @@ -113,7 +113,7 @@ func (s *PosixStorage) DeleteObject(p string) error { } if err := os.Remove(fspath); err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return errors.WithStack(err) @@ -178,10 +178,10 @@ func (s *PosixStorage) List(prefix, startWith, delimiter string, doneCh <-chan s go func(objectCh chan<- ObjectInfo) { defer close(objectCh) err := filepath.Walk(root, func(ep string, info os.FileInfo, err error) error { - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.WithStack(err) } - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil } p := ep diff --git a/internal/objectstorage/posixflat.go b/internal/objectstorage/posixflat.go index 343d377..8f4693c 100644 --- a/internal/objectstorage/posixflat.go +++ b/internal/objectstorage/posixflat.go @@ -238,7 +238,7 @@ func (s *PosixFlatStorage) Stat(p string) (*ObjectInfo, error) { fi, err := os.Stat(fspath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil, NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return nil, errors.WithStack(err) @@ -254,7 +254,7 @@ func (s *PosixFlatStorage) ReadObject(p string) (ReadSeekCloser, error) { } f, err := os.Open(fspath) - if err != nil && os.IsNotExist(err) { + if err != nil && errors.Is(err, os.ErrNotExist) { return nil, NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return f, errors.WithStack(err) @@ -287,7 +287,7 @@ func (s *PosixFlatStorage) DeleteObject(p string) error { } if err := os.Remove(fspath); err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return NewErrNotExist(errors.Errorf("object %q doesn't exist", p)) } return errors.WithStack(err) @@ -353,10 +353,10 @@ func (s *PosixFlatStorage) List(prefix, startWith, delimiter string, doneCh <-ch var prevp string defer close(objectCh) err := filepath.Walk(root, func(ep string, info os.FileInfo, err error) error { - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.WithStack(err) } - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { return nil } p := ep @@ -389,10 +389,10 @@ func (s *PosixFlatStorage) List(prefix, startWith, delimiter string, doneCh <-ch // just be listed hasFile := true _, err = os.Stat(ep + ".f") - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.WithStack(err) } - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { hasFile = false } if info.IsDir() && !hasFile { diff --git a/internal/services/executor/api.go b/internal/services/executor/api.go index b5c8986..f75a8e6 100644 --- a/internal/services/executor/api.go +++ b/internal/services/executor/api.go @@ -114,7 +114,7 @@ func (h *logsHandler) readTaskLogs(taskID string, setup bool, step int, w http.R func (h *logsHandler) readLogs(taskID string, setup bool, step int, logPath string, w http.ResponseWriter, follow bool) error { f, err := os.Open(logPath) if err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { http.Error(w, "", http.StatusNotFound) } else { http.Error(w, "", http.StatusInternalServerError) @@ -220,7 +220,7 @@ func (h *archivesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.Header().Set("Cache-Control", "no-cache") if err := h.readArchive(taskID, step, w); err != nil { - if os.IsNotExist(err) { + if errors.Is(err, os.ErrNotExist) { http.Error(w, "", http.StatusNotFound) } else { http.Error(w, "", http.StatusInternalServerError) diff --git a/internal/services/executor/executor.go b/internal/services/executor/executor.go index 6334afe..3ecc10d 100644 --- a/internal/services/executor/executor.go +++ b/internal/services/executor/executor.go @@ -1334,7 +1334,7 @@ func (e *Executor) handleTasks(ctx context.Context, c <-chan *types.ExecutorTask func (e *Executor) getExecutorID() (string, error) { id, err := ioutil.ReadFile(e.executorIDPath()) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return "", errors.WithStack(err) } return string(id), nil diff --git a/internal/services/gitserver/main.go b/internal/services/gitserver/main.go index 1b0f500..08842c1 100644 --- a/internal/services/gitserver/main.go +++ b/internal/services/gitserver/main.go @@ -54,10 +54,10 @@ func repoPathIsValid(reposDir, repoPath string) (bool, error) { path := repoPath _, err = os.Stat(path) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return false, errors.WithStack(err) } - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { // if it exists assume it's valid return true, nil } @@ -69,14 +69,14 @@ func repoPathIsValid(reposDir, repoPath string) (bool, error) { } _, err := os.Stat(path) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return false, errors.WithStack(err) } // a parent path cannot end with .git if strings.HasSuffix(path, gitSuffix) { return false, nil } - if !os.IsNotExist(err) { + if !errors.Is(err, os.ErrNotExist) { // if a parent exists return not valid return false, nil } @@ -87,10 +87,10 @@ func repoPathIsValid(reposDir, repoPath string) (bool, error) { func repoExists(repoAbsPath string) (bool, error) { _, err := os.Stat(repoAbsPath) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return false, errors.WithStack(err) } - return !os.IsNotExist(err), nil + return !errors.Is(err, os.ErrNotExist), nil } func repoAbsPath(reposDir, repoPath string) (string, bool, error) { diff --git a/internal/toolbox/unarchive/unarchive.go b/internal/toolbox/unarchive/unarchive.go index b4df8ef..2a0e751 100644 --- a/internal/toolbox/unarchive/unarchive.go +++ b/internal/toolbox/unarchive/unarchive.go @@ -37,7 +37,7 @@ func Unarchive(source io.Reader, destDir string, overwrite, removeDestDir bool) } // don't follow destdir if it's a symlink fi, err := os.Lstat(destDir) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.Wrapf(err, "failed to lstat destination dir") } if fi != nil && !fi.IsDir() { @@ -92,7 +92,7 @@ func untarNext(tr *tar.Reader, destDir string, overwrite bool) error { switch hdr.Typeflag { case tar.TypeDir: fi, err := os.Lstat(destPath) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.WithStack(err) } if fi != nil && !fi.IsDir() { @@ -103,7 +103,7 @@ func untarNext(tr *tar.Reader, destDir string, overwrite bool) error { return mkdir(destPath, hdr.FileInfo().Mode()) case tar.TypeReg, tar.TypeRegA, tar.TypeChar, tar.TypeBlock, tar.TypeFifo: fi, err := os.Lstat(destPath) - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return errors.WithStack(err) } if fi != nil && !fi.Mode().IsRegular() { @@ -135,7 +135,7 @@ func untarNext(tr *tar.Reader, destDir string, overwrite bool) error { func fileExists(name string) bool { _, err := os.Lstat(name) - return !os.IsNotExist(err) + return !errors.Is(err, os.ErrNotExist) } func mkdir(dirPath string, mode os.FileMode) error {