Skip to content

Commit

Permalink
Merge pull request #18722 from mmorel-35/api/errorlint
Browse files Browse the repository at this point in the history
fix: enable errorlint in api, client and pkg
  • Loading branch information
ahrtr authored Oct 20, 2024
2 parents 44c3918 + b281a3c commit 55de68d
Show file tree
Hide file tree
Showing 22 changed files with 69 additions and 52 deletions.
12 changes: 7 additions & 5 deletions api/v3rpc/rpctypes/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package rpctypes

import (
"errors"
"testing"

"google.golang.org/grpc/codes"
Expand All @@ -24,19 +25,20 @@ import (
func TestConvert(t *testing.T) {
e1 := status.Error(codes.InvalidArgument, "etcdserver: key is not provided")
e2 := ErrGRPCEmptyKey
e3 := ErrEmptyKey
var e3 EtcdError
errors.As(ErrEmptyKey, &e3)

if e1.Error() != e2.Error() {
t.Fatalf("expected %q == %q", e1.Error(), e2.Error())
}
if ev1, ok := status.FromError(e1); ok && ev1.Code() != e3.(EtcdError).Code() {
t.Fatalf("expected them to be equal, got %v / %v", ev1.Code(), e3.(EtcdError).Code())
if ev1, ok := status.FromError(e1); ok && ev1.Code() != e3.Code() {
t.Fatalf("expected them to be equal, got %v / %v", ev1.Code(), e3.Code())
}

if e1.Error() == e3.Error() {
t.Fatalf("expected %q != %q", e1.Error(), e3.Error())
}
if ev2, ok := status.FromError(e2); ok && ev2.Code() != e3.(EtcdError).Code() {
t.Fatalf("expected them to be equal, got %v / %v", ev2.Code(), e3.(EtcdError).Code())
if ev2, ok := status.FromError(e2); ok && ev2.Code() != e3.Code() {
t.Fatalf("expected them to be equal, got %v / %v", ev2.Code(), e3.Code())
}
}
7 changes: 4 additions & 3 deletions client/pkg/fileutil/lock_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package fileutil

import (
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -58,13 +59,13 @@ func TryLockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) {
func ofdTryLockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) {
f, err := os.OpenFile(path, flag, perm)
if err != nil {
return nil, fmt.Errorf("ofdTryLockFile failed to open %q (%v)", path, err)
return nil, fmt.Errorf("ofdTryLockFile failed to open %q (%w)", path, err)
}

flock := wrlck
if err = syscall.FcntlFlock(f.Fd(), unix.F_OFD_SETLK, &flock); err != nil {
f.Close()
if err == syscall.EWOULDBLOCK {
if errors.Is(err, syscall.EWOULDBLOCK) {
err = ErrLocked
}
return nil, err
Expand All @@ -79,7 +80,7 @@ func LockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) {
func ofdLockFile(path string, flag int, perm os.FileMode) (*LockedFile, error) {
f, err := os.OpenFile(path, flag, perm)
if err != nil {
return nil, fmt.Errorf("ofdLockFile failed to open %q (%v)", path, err)
return nil, fmt.Errorf("ofdLockFile failed to open %q (%w)", path, err)
}

flock := wrlck
Expand Down
9 changes: 5 additions & 4 deletions client/pkg/fileutil/preallocate_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package fileutil

import (
"errors"
"os"
"syscall"
)
Expand All @@ -25,10 +26,10 @@ func preallocExtend(f *os.File, sizeInBytes int64) error {
// use mode = 0 to change size
err := syscall.Fallocate(int(f.Fd()), 0, 0, sizeInBytes)
if err != nil {
errno, ok := err.(syscall.Errno)
var errno syscall.Errno
// not supported; fallback
// fallocate EINTRs frequently in some environments; fallback
if ok && (errno == syscall.ENOTSUP || errno == syscall.EINTR) {
if errors.As(err, &errno) && (errno == syscall.ENOTSUP || errno == syscall.EINTR) {
return preallocExtendTrunc(f, sizeInBytes)
}
}
Expand All @@ -39,9 +40,9 @@ func preallocFixed(f *os.File, sizeInBytes int64) error {
// use mode = 1 to keep size; see FALLOC_FL_KEEP_SIZE
err := syscall.Fallocate(int(f.Fd()), 1, 0, sizeInBytes)
if err != nil {
errno, ok := err.(syscall.Errno)
var errno syscall.Errno
// treat not supported as nil error
if ok && errno == syscall.ENOTSUP {
if errors.As(err, &errno) && errno == syscall.ENOTSUP {
return nil
}
}
Expand Down
4 changes: 2 additions & 2 deletions client/pkg/srv/srv.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func GetCluster(serviceScheme, service, name, dns string, apurls types.URLs) ([]

err := updateNodeMap(service, serviceScheme)
if err != nil {
return nil, fmt.Errorf("error querying DNS SRV records for _%s %s", service, err)
return nil, fmt.Errorf("error querying DNS SRV records for _%s %w", service, err)
}
return stringParts, nil
}
Expand Down Expand Up @@ -123,7 +123,7 @@ func GetClient(service, domain string, serviceName string) (*SRVClients, error)
errHTTP := updateURLs(GetSRVService(service, serviceName, "http"), "http")

if errHTTPS != nil && errHTTP != nil {
return nil, fmt.Errorf("dns lookup errors: %s and %s", errHTTPS, errHTTP)
return nil, fmt.Errorf("dns lookup errors: %w and %w", errHTTPS, errHTTP)
}

endpoints := make([]string, len(urls))
Expand Down
6 changes: 4 additions & 2 deletions client/pkg/transport/timeout_dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package transport

import (
"errors"
"net"
"testing"
"time"
Expand Down Expand Up @@ -57,7 +58,8 @@ func TestReadWriteTimeoutDialer(t *testing.T) {
t.Fatal("wait timeout")
}

if operr, ok := err.(*net.OpError); !ok || operr.Op != "write" || !operr.Timeout() {
var operr *net.OpError
if !errors.As(err, &operr) || operr.Op != "write" || !operr.Timeout() {
t.Errorf("err = %v, want write i/o timeout error", err)
}

Expand All @@ -77,7 +79,7 @@ func TestReadWriteTimeoutDialer(t *testing.T) {
t.Fatal("wait timeout")
}

if operr, ok := err.(*net.OpError); !ok || operr.Op != "read" || !operr.Timeout() {
if !errors.As(err, &operr) || operr.Op != "read" || !operr.Timeout() {
t.Errorf("err = %v, want read i/o timeout error", err)
}
}
Expand Down
6 changes: 4 additions & 2 deletions client/pkg/transport/timeout_listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package transport

import (
"errors"
"net"
"testing"
"time"
Expand Down Expand Up @@ -82,7 +83,8 @@ func TestWriteReadTimeoutListener(t *testing.T) {
t.Fatal("wait timeout")
}

if operr, ok := err.(*net.OpError); !ok || operr.Op != "write" || !operr.Timeout() {
var operr *net.OpError
if !errors.As(err, &operr) || operr.Op != "write" || !operr.Timeout() {
t.Errorf("err = %v, want write i/o timeout error", err)
}
writerStopCh <- struct{}{}
Expand All @@ -109,7 +111,7 @@ func TestWriteReadTimeoutListener(t *testing.T) {
t.Fatal("wait timeout")
}

if operr, ok := err.(*net.OpError); !ok || operr.Op != "read" || !operr.Timeout() {
if !errors.As(err, &operr) || operr.Op != "read" || !operr.Timeout() {
t.Errorf("err = %v, want read i/o timeout error", err)
}
readerStopCh <- struct{}{}
Expand Down
9 changes: 5 additions & 4 deletions client/v3/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func (c *Client) getToken(ctx context.Context) error {

resp, err := c.Auth.Authenticate(ctx, c.Username, c.Password)
if err != nil {
if err == rpctypes.ErrAuthNotEnabled {
if errors.Is(err, rpctypes.ErrAuthNotEnabled) {
c.authTokenBundle.UpdateAuthToken("")
return nil
}
Expand Down Expand Up @@ -605,7 +605,8 @@ func ContextError(ctx context.Context, err error) error {
return nil
}
err = rpctypes.Error(err)
if _, ok := err.(rpctypes.EtcdError); ok {
var serverErr rpctypes.EtcdError
if errors.As(err, &serverErr) {
return err
}
if ev, ok := status.FromError(err); ok {
Expand All @@ -627,7 +628,7 @@ func canceledByCaller(stopCtx context.Context, err error) bool {
return false
}

return err == context.Canceled || err == context.DeadlineExceeded
return errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
}

// IsConnCanceled returns true, if error is from a closed gRPC connection.
Expand All @@ -645,7 +646,7 @@ func IsConnCanceled(err error) bool {
}

// >= gRPC v1.10.x
if err == context.Canceled {
if errors.Is(err, context.Canceled) {
return true
}

Expand Down
4 changes: 2 additions & 2 deletions client/v3/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,8 +430,8 @@ func TestClientRejectOldCluster(t *testing.T) {
},
}

if err := c.checkVersion(); err != tt.expectedError {
t.Errorf("heckVersion err:%v", err)
if err := c.checkVersion(); !errors.Is(err, tt.expectedError) {
t.Errorf("checkVersion err:%v", err)
}
})
}
Expand Down
5 changes: 3 additions & 2 deletions client/v3/experimental/recipes/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package recipe

import (
"context"
"errors"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -51,7 +52,7 @@ func newUniqueKV(kv v3.KV, prefix string, val string) (*RemoteKV, error) {
if err == nil {
return &RemoteKV{kv, newKey, rev, val}, nil
}
if err != ErrKeyExists {
if !errors.Is(err, ErrKeyExists) {
return nil, err
}
}
Expand Down Expand Up @@ -155,7 +156,7 @@ func newUniqueEphemeralKV(s *concurrency.Session, prefix, val string) (ek *Ephem
for {
newKey := fmt.Sprintf("%s/%v", prefix, time.Now().UnixNano())
ek, err = newEphemeralKV(s, newKey, val)
if err == nil || err != ErrKeyExists {
if err == nil || !errors.Is(err, ErrKeyExists) {
break
}
}
Expand Down
3 changes: 2 additions & 1 deletion client/v3/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package clientv3

import (
"context"
"errors"
"sync"
"time"

Expand Down Expand Up @@ -464,7 +465,7 @@ func (l *lessor) recvKeepAliveLoop() (gerr error) {
return err
}

if ContextError(l.stopCtx, err) == rpctypes.ErrNoLeader {
if errors.Is(ContextError(l.stopCtx, err), rpctypes.ErrNoLeader) {
l.closeRequireLeader()
}
break
Expand Down
4 changes: 3 additions & 1 deletion client/v3/leasing/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package leasing

import (
"context"
"errors"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -282,7 +283,8 @@ func (lkv *leasingKV) acquire(ctx context.Context, key string, op v3.Op) (*v3.Tx
return resp, nil
}
// retry if transient error
if _, ok := err.(rpctypes.EtcdError); ok {
var serverErr rpctypes.EtcdError
if errors.As(err, &serverErr) {
return nil, err
}
if ev, ok := status.FromError(err); ok && ev.Code() != codes.Unavailable {
Expand Down
6 changes: 3 additions & 3 deletions client/v3/maintenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func NewMaintenance(c *Client) Maintenance {
dial: func(endpoint string) (pb.MaintenanceClient, func(), error) {
conn, err := c.Dial(endpoint)
if err != nil {
return nil, nil, fmt.Errorf("failed to dial endpoint %s with maintenance client: %v", endpoint, err)
return nil, nil, fmt.Errorf("failed to dial endpoint %s with maintenance client: %w", endpoint, err)
}

cancel := func() { conn.Close() }
Expand Down Expand Up @@ -297,8 +297,8 @@ func (m *maintenance) Snapshot(ctx context.Context) (io.ReadCloser, error) {
}

func (m *maintenance) logAndCloseWithError(err error, pw *io.PipeWriter) {
switch err {
case io.EOF:
switch {
case errors.Is(err, io.EOF):
m.lg.Info("completed snapshot read; closing")
default:
m.lg.Warn("failed to receive from snapshot stream; closing", zap.Error(err))
Expand Down
8 changes: 4 additions & 4 deletions client/v3/mock/mockserver/mockserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ func startMockServersUnix(count int) (ms *MockServers, err error) {
for i := 0; i < count; i++ {
f, err := os.CreateTemp(dir, "etcd-unix-so-")
if err != nil {
return nil, fmt.Errorf("failed to allocate temp file for unix socket: %v", err)
return nil, fmt.Errorf("failed to allocate temp file for unix socket: %w", err)
}
fn := f.Name()
err = os.Remove(fn)
if err != nil {
return nil, fmt.Errorf("failed to remove temp file before creating unix socket: %v", err)
return nil, fmt.Errorf("failed to remove temp file before creating unix socket: %w", err)
}
addrs = append(addrs, fn)
}
Expand All @@ -110,7 +110,7 @@ func startMockServers(network string, addrs []string) (ms *MockServers, err erro
for idx, addr := range addrs {
ln, err := net.Listen(network, addr)
if err != nil {
return nil, fmt.Errorf("failed to listen %v", err)
return nil, fmt.Errorf("failed to listen %w", err)
}
ms.Servers[idx] = &MockServer{ln: ln, Network: network, Address: ln.Addr().String()}
ms.StartAt(idx)
Expand All @@ -126,7 +126,7 @@ func (ms *MockServers) StartAt(idx int) (err error) {
if ms.Servers[idx].ln == nil {
ms.Servers[idx].ln, err = net.Listen(ms.Servers[idx].Network, ms.Servers[idx].Address)
if err != nil {
return fmt.Errorf("failed to listen %v", err)
return fmt.Errorf("failed to listen %w", err)
}
}

Expand Down
4 changes: 3 additions & 1 deletion client/v3/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package clientv3

import (
"context"
"errors"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -52,7 +53,8 @@ func (rp retryPolicy) String() string {
// handle itself even with retries.
func isSafeRetryImmutableRPC(err error) bool {
eErr := rpctypes.Error(err)
if serverErr, ok := eErr.(rpctypes.EtcdError); ok && serverErr.Code() != codes.Unavailable {
var serverErr rpctypes.EtcdError
if errors.As(eErr, &serverErr) && serverErr.Code() != codes.Unavailable {
// interrupted by non-transient server-side or gRPC-side error
// client cannot handle itself (e.g. rpctypes.ErrCompacted)
return false
Expand Down
6 changes: 3 additions & 3 deletions client/v3/retry_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,10 @@ func isContextError(err error) bool {
}

func contextErrToGRPCErr(err error) error {
switch err {
case context.DeadlineExceeded:
switch {
case errors.Is(err, context.DeadlineExceeded):
return status.Errorf(codes.DeadlineExceeded, err.Error())
case context.Canceled:
case errors.Is(err, context.Canceled):
return status.Errorf(codes.Canceled, err.Error())
default:
return status.Errorf(codes.Unknown, err.Error())
Expand Down
4 changes: 2 additions & 2 deletions client/v3/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, d
var f *os.File
f, err = os.OpenFile(partpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileutil.PrivateFileMode)
if err != nil {
return "", fmt.Errorf("could not open %s (%v)", partpath, err)
return "", fmt.Errorf("could not open %s (%w)", partpath, err)
}
lg.Info("created temporary db file", zap.String("path", partpath))

Expand Down Expand Up @@ -95,7 +95,7 @@ func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, d
)

if err = os.Rename(partpath, dbPath); err != nil {
return resp.Version, fmt.Errorf("could not rename %s to %s (%v)", partpath, dbPath, err)
return resp.Version, fmt.Errorf("could not rename %s to %s (%w)", partpath, dbPath, err)
}
lg.Info("saved", zap.String("path", dbPath))
return resp.Version, nil
Expand Down
Loading

0 comments on commit 55de68d

Please sign in to comment.