Refactor: replace os.IsNotExist() with errors.Is(err, fs.ErrNotExist()

`os.IsNotExist()` is from before `errors.Is()` existed. The latter is the
recommended approach, as it also recognised wrapped errors.

No functional changes, except for recognising more cases of "does not
exist" errors as such.
This commit is contained in:
Sybren A. Stüvel 2022-06-28 10:18:44 +02:00
parent 2045a087ca
commit fb89658530
14 changed files with 42 additions and 22 deletions

View File

@ -7,6 +7,7 @@ import (
"errors" "errors"
"flag" "flag"
"fmt" "fmt"
"io/fs"
"math/rand" "math/rand"
"net" "net"
"net/http" "net/http"
@ -77,7 +78,7 @@ func main() {
// Load configuration. // Load configuration.
configService := config.NewService() configService := config.NewService()
err := configService.Load() err := configService.Load()
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
log.Error().Err(err).Msg("loading configuration") log.Error().Err(err).Msg("loading configuration")
} }

View File

@ -6,6 +6,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"io/fs"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
@ -193,7 +194,7 @@ func loadConf(filename string, overrides ...func(c *Conf)) (Conf, error) {
yamlFile, err := os.ReadFile(filename) yamlFile, err := os.ReadFile(filename)
if err != nil { if err != nil {
var evt *zerolog.Event var evt *zerolog.Event
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
evt = log.Debug() evt = log.Debug()
} else { } else {
evt = log.Warn().Err(err) evt = log.Warn().Err(err)

View File

@ -4,6 +4,7 @@ package job_compilers
import ( import (
"embed" "embed"
"errors"
"fmt" "fmt"
"io" "io"
"io/fs" "io/fs"
@ -64,13 +65,13 @@ func getAvailableFilesystems() []fs.FS {
func loadFileFromAnyFS(path string) ([]byte, error) { func loadFileFromAnyFS(path string) ([]byte, error) {
filesystems := getAvailableFilesystems() filesystems := getAvailableFilesystems()
for _, fs := range filesystems { for _, filesystem := range filesystems {
file, err := fs.Open(path) file, err := filesystem.Open(path)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
continue continue
} }
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to open file %s on filesystem %s: %w", path, fs, err) return nil, fmt.Errorf("failed to open file %s on filesystem %s: %w", path, filesystem, err)
} }
return io.ReadAll(file) return io.ReadAll(file)
} }
@ -137,7 +138,7 @@ func findOnDiskScriptsNextTo(exename string) (string, bool) {
logger.Trace().Msg("job compiler: finding on-disk scripts") logger.Trace().Msg("job compiler: finding on-disk scripts")
stat, err := os.Stat(scriptsDir) stat, err := os.Stat(scriptsDir)
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return scriptsDir, false return scriptsDir, false
} }
if err != nil { if err != nil {

View File

@ -3,6 +3,8 @@ package task_logs
// SPDX-License-Identifier: GPL-3.0-or-later // SPDX-License-Identifier: GPL-3.0-or-later
import ( import (
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"sort" "sort"
@ -44,7 +46,7 @@ func rotateLogFile(logger zerolog.Logger, logpath string) error {
// Don't do anything if the file doesn't exist yet. // Don't do anything if the file doesn't exist yet.
_, err := os.Stat(logpath) _, err := os.Stat(logpath)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
logger.Debug().Msg("log file does not exist, no need to rotate") logger.Debug().Msg("log file does not exist, no need to rotate")
return nil return nil
} }

View File

@ -3,6 +3,8 @@ package task_logs
// SPDX-License-Identifier: GPL-3.0-or-later // SPDX-License-Identifier: GPL-3.0-or-later
import ( import (
"errors"
"io/fs"
"io/ioutil" "io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
@ -112,7 +114,7 @@ func TestMultipleFilesWithHoles(t *testing.T) {
func fileExists(filename string) bool { func fileExists(filename string) bool {
_, err := os.Stat(filename) _, err := os.Stat(filename)
return !os.IsNotExist(err) return !errors.Is(err, fs.ErrNotExist)
} }
func fileTouch(filename string) { func fileTouch(filename string) {

View File

@ -3,7 +3,9 @@ package task_logs
// SPDX-License-Identifier: GPL-3.0-or-later // SPDX-License-Identifier: GPL-3.0-or-later
import ( import (
"errors"
"fmt" "fmt"
"io/fs"
"io/ioutil" "io/ioutil"
"os" "os"
"path/filepath" "path/filepath"
@ -78,7 +80,7 @@ func TestLogRotation(t *testing.T) {
assert.Equal(t, "Ovo je priča\n", string(contents)) assert.Equal(t, "Ovo je priča\n", string(contents))
_, err = os.Stat(filename) _, err = os.Stat(filename)
assert.True(t, os.IsNotExist(err)) assert.True(t, errors.Is(err, fs.ErrNotExist))
} }
func TestLogTail(t *testing.T) { func TestLogTail(t *testing.T) {

View File

@ -7,8 +7,10 @@ package worker
import ( import (
"bufio" "bufio"
"context" "context"
"errors"
"fmt" "fmt"
"io" "io"
"io/fs"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
@ -253,7 +255,7 @@ func createIndexFile(inputGlob string, frameRate float64) ([]string, func(), err
cleanup := func() { cleanup := func() {
err := os.Remove(indexFilename) err := os.Remove(indexFilename)
if err != nil && !os.IsNotExist(err) { if err != nil && !errors.Is(err, fs.ErrNotExist) {
log.Warn(). log.Warn().
Err(err). Err(err).
Str("filename", indexFilename). Str("filename", indexFilename).

View File

@ -6,7 +6,9 @@ package worker
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"regexp" "regexp"
@ -99,7 +101,7 @@ func (ce *CommandExecutor) moveAndLog(ctx context.Context, taskID, cmdName, src,
func fileExists(filename string) bool { func fileExists(filename string) bool {
_, err := os.Stat(filename) _, err := os.Stat(filename)
return !os.IsNotExist(err) return !errors.Is(err, fs.ErrNotExist)
} }
// timestampedPath returns the path with its modification time appended to the name. // timestampedPath returns the path with its modification time appended to the name.

View File

@ -71,7 +71,7 @@ func (fcw *FileConfigWrangler) WorkerConfig() (WorkerConfig, error) {
err := fcw.loadConfig(configFilename, &wc) err := fcw.loadConfig(configFilename, &wc)
if err != nil { if err != nil {
if !os.IsNotExist(err) { if !errors.Is(err, fs.ErrNotExist) {
return wc, err return wc, err
} }

View File

@ -28,7 +28,7 @@ func FindBlender() (string, error) {
blenderPath := filepath.Join(dir, "blender.exe") blenderPath := filepath.Join(dir, "blender.exe")
_, err = os.Stat(blenderPath) _, err = os.Stat(blenderPath)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return "", fmt.Errorf("blender-launcher found at %s but not its blender.exe", exe) return "", fmt.Errorf("blender-launcher found at %s but not its blender.exe", exe)
} }
return "", fmt.Errorf("investigating %s: %w", blenderPath, err) return "", fmt.Errorf("investigating %s: %w", blenderPath, err)

View File

@ -25,6 +25,7 @@ package checkout
import ( import (
"errors" "errors"
"fmt" "fmt"
"io/fs"
"math/rand" "math/rand"
"os" "os"
"path/filepath" "path/filepath"
@ -124,7 +125,7 @@ func (m *Manager) PrepareCheckout(checkoutPath string) (ResolvedCheckoutInfo, er
Str("checkoutPath", checkoutPath). Str("checkoutPath", checkoutPath).
Logger() Logger()
if stat, err := os.Stat(checkoutPaths.absolutePath); !os.IsNotExist(err) { if stat, err := os.Stat(checkoutPaths.absolutePath); !errors.Is(err, fs.ErrNotExist) {
if err == nil { if err == nil {
// No error stat'ing this path, indicating it's an existing checkout. // No error stat'ing this path, indicating it's an existing checkout.
lastErr = ErrCheckoutAlreadyExists lastErr = ErrCheckoutAlreadyExists
@ -202,7 +203,7 @@ func (m *Manager) SymlinkToCheckout(blobPath, checkoutPath, symlinkRelativePath
if err == nil { if err == nil {
return err return err
} }
if !os.IsNotExist(err) { if !errors.Is(err, fs.ErrNotExist) {
logger.Error().Err(err).Msg("shaman: unable to create symlink") logger.Error().Err(err).Msg("shaman: unable to create symlink")
return err return err
} }

View File

@ -23,6 +23,8 @@
package shaman package shaman
import ( import (
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"time" "time"
@ -199,7 +201,7 @@ func (s *Server) gcFilterLinkedFiles(checkoutPath string, oldFiles mtimeMap, log
} }
linkTarget, err := filepath.EvalSymlinks(path) linkTarget, err := filepath.EvalSymlinks(path)
if err != nil { if err != nil {
if os.IsNotExist(err) { if errors.Is(err, fs.ErrNotExist) {
return nil return nil
} }
@ -229,7 +231,7 @@ func (s *Server) gcDeleteOldFiles(doDryRun bool, oldFiles mtimeMap, logger zerol
pathLogger := logger.With().Str("path", path).Logger() pathLogger := logger.With().Str("path", path).Logger()
if stat, err := os.Stat(path); err != nil { if stat, err := os.Stat(path); err != nil {
if !os.IsNotExist(err) { if !errors.Is(err, fs.ErrNotExist) {
pathLogger.Warn().Err(err).Msg("unable to stat to-be-deleted file") pathLogger.Warn().Err(err).Msg("unable to stat to-be-deleted file")
} }
} else if stat.ModTime().After(lastSeenModTime) { } else if stat.ModTime().After(lastSeenModTime) {

View File

@ -23,6 +23,8 @@
package shaman package shaman
import ( import (
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
@ -171,7 +173,7 @@ func TestGCComponents(t *testing.T) {
assert.FileExists(t, absPaths["6001.blob"], "file should exist after GC") assert.FileExists(t, absPaths["6001.blob"], "file should exist after GC")
assert.FileExists(t, absPaths["781.blob"], "file should exist after GC") assert.FileExists(t, absPaths["781.blob"], "file should exist after GC")
_, err = os.Stat(absPaths["7488.blob"]) _, err = os.Stat(absPaths["7488.blob"])
assert.True(t, os.IsNotExist(err), "file %s should NOT exist after GC", absPaths["7488.blob"]) assert.True(t, errors.Is(err, fs.ErrNotExist), "file %s should NOT exist after GC", absPaths["7488.blob"])
} }
// Test of the high-level GCStorage() function. // Test of the high-level GCStorage() function.
@ -215,9 +217,9 @@ func TestGarbageCollect(t *testing.T) {
assert.FileExists(t, absPaths["7488.blob"], "file should exist after dry-run GC") assert.FileExists(t, absPaths["7488.blob"], "file should exist after dry-run GC")
server.GCStorage(false) server.GCStorage(false)
_, err = os.Stat(absPaths["6001.blob"]) _, err = os.Stat(absPaths["6001.blob"])
assert.True(t, os.IsNotExist(err), "file %s should NOT exist after GC", absPaths["6001.blob"]) assert.True(t, errors.Is(err, fs.ErrNotExist), "file %s should NOT exist after GC", absPaths["6001.blob"])
_, err = os.Stat(absPaths["7488.blob"]) _, err = os.Stat(absPaths["7488.blob"])
assert.True(t, os.IsNotExist(err), "file %s should NOT exist after GC", absPaths["7488.blob"]) assert.True(t, errors.Is(err, fs.ErrNotExist), "file %s should NOT exist after GC", absPaths["7488.blob"])
// Used files should still exist. // Used files should still exist.
assert.FileExists(t, absPaths["781.blob"]) assert.FileExists(t, absPaths["781.blob"])

View File

@ -23,6 +23,8 @@
package filestore package filestore
import ( import (
"errors"
"io/fs"
"os" "os"
"path/filepath" "path/filepath"
"strconv" "strconv"
@ -154,7 +156,7 @@ func (s *Store) MoveToStored(checksum string, filesize int64, uploadedFilePath s
func (s *Store) removeFile(filePath string) error { func (s *Store) removeFile(filePath string) error {
err := os.Remove(filePath) err := os.Remove(filePath)
if err != nil { if err != nil {
if !os.IsNotExist(err) { if !errors.Is(err, fs.ErrNotExist) {
log.Debug().Err(err).Msg("shaman: unable to delete file; ignoring") log.Debug().Err(err).Msg("shaman: unable to delete file; ignoring")
} }
} }