From 740bdadbcb16b37924b420d757373dca92bfa64d Mon Sep 17 00:00:00 2001 From: Pascal Bourdier Date: Tue, 16 Sep 2025 14:54:46 +0200 Subject: [PATCH] fix: use errors pkg for comparison follow this best practice: https://go.dev/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features --- docker/client.go | 6 ++-- docker/container.go | 45 ++++++++++++++++++++---------- docker/event.go | 4 +-- docker/exec.go | 9 ++++-- docker/image.go | 12 +++++--- docker/network.go | 12 +++++--- docker/pkg/archive/archive.go | 15 +++++----- docker/pkg/ioutils/bytespipe.go | 2 +- docker/pkg/mount/mount.go | 3 +- docker/pkg/system/exitcode.go | 4 ++- docker/pkg/system/process_unix.go | 3 +- docker/pkg/system/rm.go | 6 ++-- docker/plugin.go | 10 +++++-- docker/types/filters/parse_test.go | 2 +- docker/volume.go | 6 ++-- 15 files changed, 88 insertions(+), 51 deletions(-) diff --git a/docker/client.go b/docker/client.go index f84b1850..91693355 100644 --- a/docker/client.go +++ b/docker/client.go @@ -345,9 +345,6 @@ func NewVersionedTLSClientFromBytes(endpoint string, certPEMBlock, keyPEMBlock, } tr := defaultTransport() tr.TLSClientConfig = tlsConfig - if err != nil { - return nil, err - } c := &Client{ HTTPClient: &http.Client{Transport: tr}, TLSConfig: tlsConfig, @@ -1073,7 +1070,8 @@ func parseEndpoint(endpoint string, tls bool) (*url.URL, error) { case "http", "https", "tcp": _, port, err := net.SplitHostPort(u.Host) if err != nil { - if e, ok := err.(*net.AddrError); ok { + var e *net.AddrError + if errors.As(err, &e) { if e.Err == "missing port in address" { return u, nil } diff --git a/docker/container.go b/docker/container.go index 0d7d3fe7..ab3c2841 100644 --- a/docker/container.go +++ b/docker/container.go @@ -559,7 +559,8 @@ func (c *Client) inspectContainer(id string, opts doOptions) (*Container, error) path := "/containers/" + id + "/json" resp, err := c.do("GET", path, opts) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchContainer{ID: id} } return nil, err @@ -579,7 +580,8 @@ func (c *Client) ContainerChanges(id string) ([]Change, error) { path := "/containers/" + id + "/changes" resp, err := c.do("GET", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchContainer{ID: id} } return nil, err @@ -629,7 +631,8 @@ func (c *Client) CreateContainer(opts CreateContainerOptions) (*Container, error }, ) - if e, ok := err.(*Error); ok { + var e *Error + if errors.As(err, &e) { if e.Status == http.StatusNotFound { return nil, ErrNoSuchImage } @@ -836,7 +839,8 @@ func (c *Client) startContainer(id string, hostConfig *HostConfig, opts doOption } resp, err := c.do("POST", path, opts) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: id, Err: err} } return err @@ -869,7 +873,8 @@ func (c *Client) stopContainer(id string, timeout uint, opts doOptions) error { path := fmt.Sprintf("/containers/%s/stop?t=%d", id, timeout) resp, err := c.do("POST", path, opts) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: id} } return err @@ -889,7 +894,8 @@ func (c *Client) RestartContainer(id string, timeout uint) error { path := fmt.Sprintf("/containers/%s/restart?t=%d", id, timeout) resp, err := c.do("POST", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: id} } return err @@ -905,7 +911,8 @@ func (c *Client) PauseContainer(id string) error { path := fmt.Sprintf("/containers/%s/pause", id) resp, err := c.do("POST", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: id} } return err @@ -921,7 +928,8 @@ func (c *Client) UnpauseContainer(id string) error { path := fmt.Sprintf("/containers/%s/unpause", id) resp, err := c.do("POST", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: id} } return err @@ -951,7 +959,8 @@ func (c *Client) TopContainer(id string, psArgs string) (TopResult, error) { path := fmt.Sprintf("/containers/%s/top%s", id, args) resp, err := c.do("GET", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return result, &NoSuchContainer{ID: id} } return result, err @@ -1131,7 +1140,8 @@ func (c *Client) Stats(opts StatsOptions) (retErr error) { reqSent: reqSent, }) if err != nil { - dockerError, ok := err.(*Error) + var dockerError *Error + ok := errors.As(err, &dockerError) if ok { if dockerError.Status == http.StatusNotFound { err = &NoSuchContainer{ID: opts.ID} @@ -1192,7 +1202,8 @@ func (c *Client) KillContainer(opts KillContainerOptions) error { path := "/containers/" + opts.ID + "/kill" + "?" + queryString(opts) resp, err := c.do("POST", path, doOptions{context: opts.Context}) if err != nil { - e, ok := err.(*Error) + var e *Error + ok := errors.As(err, &e) if !ok { return err } @@ -1233,7 +1244,8 @@ func (c *Client) RemoveContainer(opts RemoveContainerOptions) error { path := "/containers/" + opts.ID + "?" + queryString(opts) resp, err := c.do("DELETE", path, doOptions{context: opts.Context}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: opts.ID} } return err @@ -1321,7 +1333,8 @@ func (c *Client) CopyFromContainer(opts CopyFromContainerOptions) error { context: opts.Context, }) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchContainer{ID: opts.Container} } return err @@ -1351,7 +1364,8 @@ func (c *Client) WaitContainerWithContext(id string, ctx context.Context) (int, func (c *Client) waitContainer(id string, opts doOptions) (int, error) { resp, err := c.do("POST", "/containers/"+id+"/wait", opts) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return 0, &NoSuchContainer{ID: id} } return 0, err @@ -1388,7 +1402,8 @@ func (c *Client) CommitContainer(opts CommitContainerOptions) (*Image, error) { context: opts.Context, }) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchContainer{ID: opts.Container} } return nil, err diff --git a/docker/event.go b/docker/event.go index 982ca1b2..8ba00c9d 100644 --- a/docker/event.go +++ b/docker/event.go @@ -236,7 +236,7 @@ func (eventState *eventMonitoringState) monitorEvents(c *Client) { eventState.updateLastSeen(ev) eventState.sendEvent(ev) case err = <-eventState.errC: - if err == ErrNoListeners { + if errors.Is(err, ErrNoListeners) { eventState.disableEventMonitoring() return } else if err != nil { @@ -349,7 +349,7 @@ func (c *Client) eventHijack(startTime int64, eventChan chan *APIEvents, errChan for { var event APIEvents if err = decoder.Decode(&event); err != nil { - if err == io.EOF || err == io.ErrUnexpectedEOF { + if err == io.EOF || errors.Is(io.ErrUnexpectedEOF, err) { c.eventMonitor.RLock() if c.eventMonitor.enabled && c.eventMonitor.C == eventChan { // Signal that we're exiting. diff --git a/docker/exec.go b/docker/exec.go index 1efdfd21..fbdba7d7 100644 --- a/docker/exec.go +++ b/docker/exec.go @@ -51,7 +51,8 @@ func (c *Client) CreateExec(opts CreateExecOptions) (*Exec, error) { path := fmt.Sprintf("/containers/%s/exec", opts.Container) resp, err := c.do("POST", path, doOptions{data: opts, context: opts.Context}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchContainer{ID: opts.Container} } return nil, err @@ -120,7 +121,8 @@ func (c *Client) StartExecNonBlocking(id string, opts StartExecOptions) (CloseWa if opts.Detach { resp, err := c.do("POST", path, doOptions{data: opts, context: opts.Context}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchExec{ID: id} } return nil, err @@ -193,7 +195,8 @@ func (c *Client) InspectExec(id string) (*ExecInspect, error) { path := fmt.Sprintf("/exec/%s/json", id) resp, err := c.do("GET", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchExec{ID: id} } return nil, err diff --git a/docker/image.go b/docker/image.go index ee7e5f53..943f265f 100644 --- a/docker/image.go +++ b/docker/image.go @@ -141,7 +141,8 @@ type ImageHistory struct { func (c *Client) ImageHistory(name string) ([]ImageHistory, error) { resp, err := c.do("GET", "/images/"+name+"/history", doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, ErrNoSuchImage } return nil, err @@ -160,7 +161,8 @@ func (c *Client) ImageHistory(name string) ([]ImageHistory, error) { func (c *Client) RemoveImage(name string) error { resp, err := c.do("DELETE", "/images/"+name, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return ErrNoSuchImage } return err @@ -187,7 +189,8 @@ func (c *Client) RemoveImageExtended(name string, opts RemoveImageOptions) error uri := fmt.Sprintf("/images/%s?%s", name, queryString(&opts)) resp, err := c.do("DELETE", uri, doOptions{context: opts.Context}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return ErrNoSuchImage } return err @@ -202,7 +205,8 @@ func (c *Client) RemoveImageExtended(name string, opts RemoveImageOptions) error func (c *Client) InspectImage(name string) (*Image, error) { resp, err := c.do("GET", "/images/"+name+"/json", doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, ErrNoSuchImage } return nil, err diff --git a/docker/network.go b/docker/network.go index 1d7c1009..76ee152e 100644 --- a/docker/network.go +++ b/docker/network.go @@ -94,7 +94,8 @@ func (c *Client) NetworkInfo(id string) (*Network, error) { path := "/networks/" + id resp, err := c.do("GET", path, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchNetwork{ID: id} } return nil, err @@ -184,7 +185,8 @@ func (c *Client) CreateNetwork(opts CreateNetworkOptions) (*Network, error) { func (c *Client) RemoveNetwork(id string) error { resp, err := c.do("DELETE", "/networks/"+id, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchNetwork{ID: id} } return err @@ -246,7 +248,8 @@ func (c *Client) ConnectNetwork(id string, opts NetworkConnectionOptions) error context: opts.Context, }) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchNetworkOrContainer{NetworkID: id, ContainerID: opts.Container} } return err @@ -262,7 +265,8 @@ func (c *Client) ConnectNetwork(id string, opts NetworkConnectionOptions) error func (c *Client) DisconnectNetwork(id string, opts NetworkConnectionOptions) error { resp, err := c.do("POST", "/networks/"+id+"/disconnect", doOptions{data: opts}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchNetworkOrContainer{NetworkID: id, ContainerID: opts.Container} } return err diff --git a/docker/pkg/archive/archive.go b/docker/pkg/archive/archive.go index dcfbcdbb..914d8ae5 100644 --- a/docker/pkg/archive/archive.go +++ b/docker/pkg/archive/archive.go @@ -10,6 +10,7 @@ import ( "compress/bzip2" "compress/gzip" "context" + "errors" "fmt" "io" "os" @@ -662,15 +663,15 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L } } - var errors []string + var errs []string for key, value := range hdr.Xattrs { if err := system.Lsetxattr(path, key, []byte(value), 0); err != nil { - if err == syscall.ENOTSUP { + if errors.Is(err, syscall.ENOTSUP) { // We ignore errors here because not all graphdrivers support // xattrs *cough* old versions of AUFS *cough*. However only // ENOTSUP should be emitted in that case, otherwise we still // bail. - errors = append(errors, err.Error()) + errs = append(errs, err.Error()) continue } return err @@ -678,9 +679,9 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L } - if len(errors) > 0 { + if len(errs) > 0 { logrus.WithFields(logrus.Fields{ - "errors": errors, + "errors": errs, }).Warn("ignored xattrs in archive: underlying filesystem doesn't support them") } @@ -709,7 +710,7 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L } } else { ts := []syscall.Timespec{timeToTimespec(aTime), timeToTimespec(hdr.ModTime)} - if err := system.LUtimesNano(path, ts); err != nil && err != system.ErrNotSupportedPlatform { + if err := system.LUtimesNano(path, ts); err != nil && !errors.Is(err, system.ErrNotSupportedPlatform) { return err } } @@ -885,7 +886,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) if err := ta.addTarFile(filePath, relFilePath); err != nil { logrus.Errorf("Can't add file %s to tar: %s", filePath, err) // if pipe is broken, stop writing tar stream to it - if err == io.ErrClosedPipe { + if errors.Is(err, io.ErrClosedPipe) { return err } } diff --git a/docker/pkg/ioutils/bytespipe.go b/docker/pkg/ioutils/bytespipe.go index e2ba15e7..5bf6fc1e 100644 --- a/docker/pkg/ioutils/bytespipe.go +++ b/docker/pkg/ioutils/bytespipe.go @@ -73,7 +73,7 @@ loop0: bp.bufLen += n // errBufferFull is an error we expect to get if the buffer is full - if err != nil && err != errBufferFull { + if err != nil && !errors.Is(err, errBufferFull) { bp.wait.Broadcast() bp.mu.Unlock() return written, err diff --git a/docker/pkg/mount/mount.go b/docker/pkg/mount/mount.go index 977c0db0..9c887c40 100644 --- a/docker/pkg/mount/mount.go +++ b/docker/pkg/mount/mount.go @@ -4,6 +4,7 @@ package mount // import "github.com/ory/dockertest/v3/docker/pkg/mount" import ( + "errors" "sort" "strings" @@ -93,7 +94,7 @@ func RecursiveUnmount(target string) error { // We've purposefully used `syscall.EINVAL` here instead of `unix.EINVAL` to avoid platform branching // Since `EINVAL` is defined for both Windows and Linux in the `syscall` package (and other platforms), // this is nicer than defining a custom value that we can refer to in each platform file. - if err == syscall.EINVAL { + if errors.Is(err, syscall.EINVAL) { continue } if i == len(mounts)-1 { diff --git a/docker/pkg/system/exitcode.go b/docker/pkg/system/exitcode.go index f1bc3c5f..0333cee7 100644 --- a/docker/pkg/system/exitcode.go +++ b/docker/pkg/system/exitcode.go @@ -4,6 +4,7 @@ package system // import "github.com/ory/dockertest/v3/docker/pkg/system" import ( + "errors" "fmt" "os/exec" "syscall" @@ -13,7 +14,8 @@ import ( // exec.ExitError, returns 0 and an error otherwise. func GetExitCode(err error) (int, error) { exitCode := 0 - if exiterr, ok := err.(*exec.ExitError); ok { + var exiterr *exec.ExitError + if errors.As(err, &exiterr) { if procExit, ok := exiterr.Sys().(syscall.WaitStatus); ok { return procExit.ExitStatus(), nil } diff --git a/docker/pkg/system/process_unix.go b/docker/pkg/system/process_unix.go index 406eb842..04ad819e 100644 --- a/docker/pkg/system/process_unix.go +++ b/docker/pkg/system/process_unix.go @@ -7,6 +7,7 @@ package system // import "github.com/ory/dockertest/v3/docker/pkg/system" import ( + "errors" "syscall" "golang.org/x/sys/unix" @@ -15,7 +16,7 @@ import ( // IsProcessAlive returns true if process with a given pid is running. func IsProcessAlive(pid int) bool { err := unix.Kill(pid, syscall.Signal(0)) - if err == nil || err == unix.EPERM { + if err == nil || errors.Is(err, unix.EPERM) { return true } diff --git a/docker/pkg/system/rm.go b/docker/pkg/system/rm.go index 08bcaf7f..6efc2a22 100644 --- a/docker/pkg/system/rm.go +++ b/docker/pkg/system/rm.go @@ -4,6 +4,7 @@ package system // import "github.com/ory/dockertest/v3/docker/pkg/system" import ( + "errors" "fmt" "os" "syscall" @@ -40,7 +41,8 @@ func EnsureRemoveAll(dir string) error { return err } - pe, ok := err.(*os.PathError) + var pe *os.PathError + ok := errors.As(err, &pe) if !ok { return err } @@ -62,7 +64,7 @@ func EnsureRemoveAll(dir string) error { continue } - if pe.Err != syscall.EBUSY { + if !errors.Is(pe.Err, syscall.EBUSY) { return err } diff --git a/docker/plugin.go b/docker/plugin.go index 9a4b160b..188a3445 100644 --- a/docker/plugin.go +++ b/docker/plugin.go @@ -10,6 +10,7 @@ package docker import ( "context" "encoding/json" + "errors" "io" "net/http" ) @@ -226,7 +227,8 @@ func (c *Client) InspectPlugins(name string, ctx context.Context) (*PluginDetail } defer resp.Body.Close() if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchPlugin{ID: name} } return nil, err @@ -261,7 +263,8 @@ func (c *Client) RemovePlugin(opts RemovePluginOptions) (*PluginDetail, error) { } defer resp.Body.Close() if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, &NoSuchPlugin{ID: opts.Name} } return nil, err @@ -396,7 +399,8 @@ func (c *Client) ConfigurePlugin(opts ConfigurePluginOptions) error { context: opts.Context, }) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return &NoSuchPlugin{ID: opts.Name} } return err diff --git a/docker/types/filters/parse_test.go b/docker/types/filters/parse_test.go index 67a12432..65a75e14 100644 --- a/docker/types/filters/parse_test.go +++ b/docker/types/filters/parse_test.go @@ -40,7 +40,7 @@ func TestParseArgsEdgeCase(t *testing.T) { if args.Len() != 0 { t.Fatalf("Expected an empty Args (map), got %v", args) } - if _, err := ParseFlag("anything", args); err == nil || err != ErrBadFormat { + if _, err := ParseFlag("anything", args); err == nil || !errors.Is(err, ErrBadFormat) { t.Fatalf("Expected ErrBadFormat, got %v", err) } } diff --git a/docker/volume.go b/docker/volume.go index f5fae9f8..155b99dc 100644 --- a/docker/volume.go +++ b/docker/volume.go @@ -107,7 +107,8 @@ func (c *Client) CreateVolume(opts CreateVolumeOptions) (*Volume, error) { func (c *Client) InspectVolume(name string) (*Volume, error) { resp, err := c.do("GET", "/volumes/"+name, doOptions{}) if err != nil { - if e, ok := err.(*Error); ok && e.Status == http.StatusNotFound { + var e *Error + if errors.As(err, &e) && e.Status == http.StatusNotFound { return nil, ErrNoSuchVolume } return nil, err @@ -145,7 +146,8 @@ func (c *Client) RemoveVolumeWithOptions(opts RemoveVolumeOptions) error { path := "/volumes/" + opts.Name resp, err := c.do("DELETE", path+"?"+queryString(opts), doOptions{context: opts.Context}) if err != nil { - if e, ok := err.(*Error); ok { + var e *Error + if errors.As(err, &e) { if e.Status == http.StatusNotFound { return ErrNoSuchVolume }