From add6d6ff8288b99728bf06a7cebbc43f1c9b2811 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Sun, 20 Oct 2024 11:13:03 +0200 Subject: [PATCH] fix: enable errorlint in server directory (part 1) Signed-off-by: Matthieu MOREL --- server/etcdmain/config.go | 6 +++--- server/etcdmain/etcd.go | 14 ++++++++------ server/etcdserver/api/etcdhttp/peer.go | 9 +++++---- server/etcdserver/api/etcdhttp/utils.go | 10 +++++++--- server/etcdserver/api/membership/cluster.go | 2 +- server/etcdserver/api/membership/errors.go | 4 ++-- server/etcdserver/api/membership/storev2.go | 4 ++-- server/etcdserver/api/v2store/store.go | 2 +- server/etcdserver/api/v2store/store_ttl_test.go | 8 ++++++-- server/etcdserver/api/v3rpc/watch.go | 8 ++++---- server/etcdserver/bootstrap.go | 12 ++++++------ server/etcdserver/corrupt.go | 11 ++++++----- server/etcdserver/server.go | 12 ++++++------ server/lease/leasehttp/http.go | 4 ++-- server/storage/backend.go | 4 ++-- server/storage/mvcc/kvstore_test.go | 4 ++-- server/storage/schema/membership.go | 4 ++-- server/storage/schema/schema.go | 4 ++-- 18 files changed, 67 insertions(+), 55 deletions(-) diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index 60233445ced..79e422fdfca 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -115,9 +115,9 @@ func newConfig() *config { func (cfg *config) parse(arguments []string) error { perr := cfg.cf.flagSet.Parse(arguments) - switch perr { - case nil: - case flag.ErrHelp: + switch { + case errors.Is(perr, nil): + case errors.Is(perr, flag.ErrHelp): fmt.Println(flagsline) os.Exit(0) default: diff --git a/server/etcdmain/etcd.go b/server/etcdmain/etcd.go index ebb2964de86..16bba3736fd 100644 --- a/server/etcdmain/etcd.go +++ b/server/etcdmain/etcd.go @@ -15,6 +15,7 @@ package etcdmain import ( + errorspkg "errors" "fmt" "os" "runtime" @@ -63,8 +64,8 @@ func startEtcdOrProxyV2(args []string) { lg.Info("Running: ", zap.Strings("args", args)) if err != nil { lg.Warn("failed to verify flags", zap.Error(err)) - switch err { - case embed.ErrUnsetAdvertiseClientURLsFlag: + switch { + case errorspkg.Is(err, embed.ErrUnsetAdvertiseClientURLsFlag): lg.Warn("advertise client URLs are not set", zap.Error(err)) } os.Exit(1) @@ -129,9 +130,10 @@ func startEtcdOrProxyV2(args []string) { } if err != nil { - if derr, ok := err.(*errors.DiscoveryError); ok { - switch derr.Err { - case v2discovery.ErrDuplicateID: + var derr *errors.DiscoveryError + if errorspkg.As(err, &derr) { + switch { + case errorspkg.Is(derr.Err, v2discovery.ErrDuplicateID): lg.Warn( "member has been registered with discovery service", zap.String("name", cfg.ec.Name), @@ -145,7 +147,7 @@ func startEtcdOrProxyV2(args []string) { lg.Warn("check data dir if previous bootstrap succeeded") lg.Warn("or use a new discovery token if previous bootstrap failed") - case v2discovery.ErrDuplicateName: + case errorspkg.Is(derr.Err, v2discovery.ErrDuplicateName): lg.Warn( "member with duplicated name has already been registered", zap.String("discovery-token", cfg.ec.Durl), diff --git a/server/etcdserver/api/etcdhttp/peer.go b/server/etcdserver/api/etcdhttp/peer.go index e477f033c30..de5948d30f5 100644 --- a/server/etcdserver/api/etcdhttp/peer.go +++ b/server/etcdserver/api/etcdhttp/peer.go @@ -16,6 +16,7 @@ package etcdhttp import ( "encoding/json" + errorspkg "errors" "fmt" "net/http" "strconv" @@ -138,12 +139,12 @@ func (h *peerMemberPromoteHandler) ServeHTTP(w http.ResponseWriter, r *http.Requ resp, err := h.server.PromoteMember(r.Context(), id) if err != nil { - switch err { - case membership.ErrIDNotFound: + switch { + case errorspkg.Is(err, membership.ErrIDNotFound): http.Error(w, err.Error(), http.StatusNotFound) - case membership.ErrMemberNotLearner: + case errorspkg.Is(err, membership.ErrMemberNotLearner): http.Error(w, err.Error(), http.StatusPreconditionFailed) - case errors.ErrLearnerNotReady: + case errorspkg.Is(err, errors.ErrLearnerNotReady): http.Error(w, err.Error(), http.StatusPreconditionFailed) default: writeError(h.lg, w, r, err) diff --git a/server/etcdserver/api/etcdhttp/utils.go b/server/etcdserver/api/etcdhttp/utils.go index 268400d7640..0e6b6b019b9 100644 --- a/server/etcdserver/api/etcdhttp/utils.go +++ b/server/etcdserver/api/etcdhttp/utils.go @@ -15,6 +15,7 @@ package etcdhttp import ( + errorspkg "errors" "net/http" "go.uber.org/zap" @@ -57,9 +58,12 @@ func writeError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro } default: - switch err { - case errors.ErrTimeoutDueToLeaderFail, errors.ErrTimeoutDueToConnectionLost, errors.ErrNotEnoughStartedMembers, - errors.ErrUnhealthy: + switch { + case + errorspkg.Is(err, errors.ErrTimeoutDueToLeaderFail), + errorspkg.Is(err, errors.ErrTimeoutDueToConnectionLost), + errorspkg.Is(err, errors.ErrNotEnoughStartedMembers), + errorspkg.Is(err, errors.ErrUnhealthy): if lg != nil { lg.Warn( "v2 response error", diff --git a/server/etcdserver/api/membership/cluster.go b/server/etcdserver/api/membership/cluster.go index 1ec42c38b51..e35375a22c6 100644 --- a/server/etcdserver/api/membership/cluster.go +++ b/server/etcdserver/api/membership/cluster.go @@ -766,7 +766,7 @@ func ValidateClusterAndAssignIDs(lg *zap.Logger, local *RaftCluster, existing *R } } if !ok { - return fmt.Errorf("PeerURLs: no match found for existing member (%v, %v), last resolver error (%v)", ems[i].ID, ems[i].PeerURLs, err) + return fmt.Errorf("PeerURLs: no match found for existing member (%v, %v), last resolver error (%w)", ems[i].ID, ems[i].PeerURLs, err) } } local.members = make(map[types.ID]*Member) diff --git a/server/etcdserver/api/membership/errors.go b/server/etcdserver/api/membership/errors.go index e944d48c693..ff682971281 100644 --- a/server/etcdserver/api/membership/errors.go +++ b/server/etcdserver/api/membership/errors.go @@ -30,6 +30,6 @@ var ( ) func isKeyNotFound(err error) bool { - e, ok := err.(*v2error.Error) - return ok && e.ErrorCode == v2error.EcodeKeyNotFound + var e *v2error.Error + return errors.As(err, &e) && e.ErrorCode == v2error.EcodeKeyNotFound } diff --git a/server/etcdserver/api/membership/storev2.go b/server/etcdserver/api/membership/storev2.go index f43a1299221..f7a595ac3e1 100644 --- a/server/etcdserver/api/membership/storev2.go +++ b/server/etcdserver/api/membership/storev2.go @@ -155,14 +155,14 @@ func nodeToMember(lg *zap.Logger, n *v2store.NodeExtern) (*Member, error) { } if data := attrs[raftAttrKey]; data != nil { if err := json.Unmarshal(data, &m.RaftAttributes); err != nil { - return nil, fmt.Errorf("unmarshal raftAttributes error: %v", err) + return nil, fmt.Errorf("unmarshal raftAttributes error: %w", err) } } else { return nil, fmt.Errorf("raftAttributes key doesn't exist") } if data := attrs[attrKey]; data != nil { if err := json.Unmarshal(data, &m.Attributes); err != nil { - return m, fmt.Errorf("unmarshal attributes error: %v", err) + return m, fmt.Errorf("unmarshal attributes error: %w", err) } } return m, nil diff --git a/server/etcdserver/api/v2store/store.go b/server/etcdserver/api/v2store/store.go index 832ee46d0f0..117f1c03e14 100644 --- a/server/etcdserver/api/v2store/store.go +++ b/server/etcdserver/api/v2store/store.go @@ -534,7 +534,7 @@ func (s *store) Update(nodePath string, newValue string, expireOpts TTLOptionSet eNode := e.Node if err := n.Write(newValue, nextIndex); err != nil { - return nil, fmt.Errorf("nodePath %v : %v", nodePath, err) + return nil, fmt.Errorf("nodePath %v : %w", nodePath, err) } if n.IsDir() { diff --git a/server/etcdserver/api/v2store/store_ttl_test.go b/server/etcdserver/api/v2store/store_ttl_test.go index 0d9563a2df5..9a5d497f97b 100644 --- a/server/etcdserver/api/v2store/store_ttl_test.go +++ b/server/etcdserver/api/v2store/store_ttl_test.go @@ -111,7 +111,9 @@ func TestStoreUpdateValueTTL(t *testing.T) { s.DeleteExpiredKeys(fc.Now()) e, err = s.Get("/foo", false, false) assert.Nil(t, e) - assert.Equal(t, v2error.EcodeKeyNotFound, err.(*v2error.Error).ErrorCode) + var v2Err *v2error.Error + assert.ErrorAs(t, err, &v2Err) + assert.Equal(t, v2error.EcodeKeyNotFound, v2Err.ErrorCode) } // TestStoreUpdateDirTTL ensures that the store can update the TTL on a directory. @@ -137,7 +139,9 @@ func TestStoreUpdateDirTTL(t *testing.T) { s.DeleteExpiredKeys(fc.Now()) e, err = s.Get("/foo/bar", false, false) assert.Nil(t, e) - assert.Equal(t, v2error.EcodeKeyNotFound, err.(*v2error.Error).ErrorCode) + var v2Err *v2error.Error + assert.ErrorAs(t, err, &v2Err) + assert.Equal(t, v2error.EcodeKeyNotFound, v2Err.ErrorCode) } // TestStoreWatchExpire ensures that the store can watch for key expiration. diff --git a/server/etcdserver/api/v3rpc/watch.go b/server/etcdserver/api/v3rpc/watch.go index abb465e21d7..0cdf5b17c6f 100644 --- a/server/etcdserver/api/v3rpc/watch.go +++ b/server/etcdserver/api/v3rpc/watch.go @@ -273,12 +273,12 @@ func (sws *serverWatchStream) recvLoop() error { err := sws.isWatchPermitted(creq) if err != nil { var cancelReason string - switch err { - case auth.ErrInvalidAuthToken: + switch { + case errors.Is(err, auth.ErrInvalidAuthToken): cancelReason = rpctypes.ErrGRPCInvalidAuthToken.Error() - case auth.ErrAuthOldRevision: + case errors.Is(err, auth.ErrAuthOldRevision): cancelReason = rpctypes.ErrGRPCAuthOldRevision.Error() - case auth.ErrUserEmpty: + case errors.Is(err, auth.ErrUserEmpty): cancelReason = rpctypes.ErrGRPCUserEmpty.Error() default: if !errors.Is(err, auth.ErrPermissionDenied) { diff --git a/server/etcdserver/bootstrap.go b/server/etcdserver/bootstrap.go index 015ae3876a3..992e26d4f96 100644 --- a/server/etcdserver/bootstrap.go +++ b/server/etcdserver/bootstrap.go @@ -63,11 +63,11 @@ func bootstrap(cfg config.ServerConfig) (b *bootstrappedServer, err error) { } if terr := fileutil.TouchDirAll(cfg.Logger, cfg.DataDir); terr != nil { - return nil, fmt.Errorf("cannot access data directory: %v", terr) + return nil, fmt.Errorf("cannot access data directory: %w", terr) } if terr := fileutil.TouchDirAll(cfg.Logger, cfg.MemberDir()); terr != nil { - return nil, fmt.Errorf("cannot access member directory: %v", terr) + return nil, fmt.Errorf("cannot access member directory: %w", terr) } ss := bootstrapSnapshot(cfg) prt, err := rafthttp.NewRoundTripper(cfg.PeerTLSInfo, cfg.PeerDialTimeout()) @@ -85,7 +85,7 @@ func bootstrap(cfg config.ServerConfig) (b *bootstrappedServer, err error) { if haveWAL { if err = fileutil.IsDirWriteable(cfg.WALDir()); err != nil { - return nil, fmt.Errorf("cannot write to WAL directory: %v", err) + return nil, fmt.Errorf("cannot write to WAL directory: %w", err) } bwal = bootstrapWALFromSnapshot(cfg, backend.snapshot) } @@ -296,10 +296,10 @@ func bootstrapExistingClusterNoWAL(cfg config.ServerConfig, prt http.RoundTrippe } existingCluster, gerr := GetClusterFromRemotePeers(cfg.Logger, getRemotePeerURLs(cl, cfg.Name), prt) if gerr != nil { - return nil, fmt.Errorf("cannot fetch cluster info from peer urls: %v", gerr) + return nil, fmt.Errorf("cannot fetch cluster info from peer urls: %w", gerr) } if err := membership.ValidateClusterAndAssignIDs(cfg.Logger, cl, existingCluster); err != nil { - return nil, fmt.Errorf("error validating peerURLs %s: %v", existingCluster, err) + return nil, fmt.Errorf("error validating peerURLs %s: %w", existingCluster, err) } if !isCompatibleWithCluster(cfg.Logger, cl, cl.MemberByName(cfg.Name).ID, prt, cfg.ReqTimeout()) { return nil, fmt.Errorf("incompatible with current running cluster") @@ -363,7 +363,7 @@ func bootstrapNewClusterNoWAL(cfg config.ServerConfig, prt http.RoundTripper) (* func bootstrapClusterWithWAL(cfg config.ServerConfig, meta *snapshotMetadata) (*bootstrappedCluster, error) { if err := fileutil.IsDirWriteable(cfg.MemberDir()); err != nil { - return nil, fmt.Errorf("cannot write to member directory: %v", err) + return nil, fmt.Errorf("cannot write to member directory: %w", err) } if cfg.ShouldDiscover() { diff --git a/server/etcdserver/corrupt.go b/server/etcdserver/corrupt.go index 3df44bf54cb..5c16ffb7c9c 100644 --- a/server/etcdserver/corrupt.go +++ b/server/etcdserver/corrupt.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -95,7 +96,7 @@ func (cm *corruptionChecker) InitialCheck() error { h, _, err := cm.hasher.HashByRev(0) if err != nil { - return fmt.Errorf("%s failed to fetch hash (%v)", cm.hasher.MemberID(), err) + return fmt.Errorf("%s failed to fetch hash (%w)", cm.hasher.MemberID(), err) } peers := cm.hasher.PeerHashByRev(h.Revision) mismatch := 0 @@ -127,8 +128,8 @@ func (cm *corruptionChecker) InitialCheck() error { } if p.err != nil { - switch p.err { - case rpctypes.ErrFutureRev: + switch { + case errors.Is(p.err, rpctypes.ErrFutureRev): cm.lg.Warn( "cannot fetch hash from slow remote peer", zap.String("local-member-id", cm.hasher.MemberID().String()), @@ -139,7 +140,7 @@ func (cm *corruptionChecker) InitialCheck() error { zap.Strings("remote-peer-endpoints", p.eps), zap.Error(err), ) - case rpctypes.ErrCompacted: + case errors.Is(p.err, rpctypes.ErrCompacted): cm.lg.Warn( "cannot fetch hash from remote peer; local member is behind", zap.String("local-member-id", cm.hasher.MemberID().String()), @@ -150,7 +151,7 @@ func (cm *corruptionChecker) InitialCheck() error { zap.Strings("remote-peer-endpoints", p.eps), zap.Error(err), ) - case rpctypes.ErrClusterIDMismatch: + case errors.Is(p.err, rpctypes.ErrClusterIDMismatch): cm.lg.Warn( "cluster ID mismatch", zap.String("local-member-id", cm.hasher.MemberID().String()), diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go index 0615d20e8b3..6fc8db11e45 100644 --- a/server/etcdserver/server.go +++ b/server/etcdserver/server.go @@ -2355,12 +2355,12 @@ func (s *EtcdServer) updateClusterVersionV3(ver string) { _, err := s.raftRequest(ctx, pb.InternalRaftRequest{ClusterVersionSet: &req}) cancel() - switch err { - case nil: + switch { + case errorspkg.Is(err, nil): lg.Info("cluster version is updated", zap.String("cluster-version", version.Cluster(ver))) return - case errors.ErrStopped: + case errorspkg.Is(err, errors.ErrStopped): lg.Warn("aborting cluster version update; server is stopped", zap.Error(err)) return @@ -2391,11 +2391,11 @@ func (s *EtcdServer) monitorDowngrade() { } func (s *EtcdServer) parseProposeCtxErr(err error, start time.Time) error { - switch err { - case context.Canceled: + switch { + case errorspkg.Is(err, context.Canceled): return errors.ErrCanceled - case context.DeadlineExceeded: + case errorspkg.Is(err, context.DeadlineExceeded): s.leadTimeMu.RLock() curLeadElected := s.leadElectedTime s.leadTimeMu.RUnlock() diff --git a/server/lease/leasehttp/http.go b/server/lease/leasehttp/http.go index 9592b63cb97..ee97fc4cd14 100644 --- a/server/lease/leasehttp/http.go +++ b/server/lease/leasehttp/http.go @@ -197,7 +197,7 @@ func RenewHTTP(ctx context.Context, id lease.LeaseID, url string, rt http.RoundT lresp := &pb.LeaseKeepAliveResponse{} if err := lresp.Unmarshal(b); err != nil { - return -1, fmt.Errorf(`lease: %v. data = "%s"`, err, b) + return -1, fmt.Errorf(`lease: %w. data = "%s"`, err, b) } if lresp.ID != int64(id) { return -1, fmt.Errorf("lease: renew id mismatch") @@ -254,7 +254,7 @@ func TimeToLiveHTTP(ctx context.Context, id lease.LeaseID, keys bool, url string lresp := &leasepb.LeaseInternalResponse{} if err := lresp.Unmarshal(b); err != nil { - return nil, fmt.Errorf(`lease: %v. data = "%s"`, err, string(b)) + return nil, fmt.Errorf(`lease: %w. data = "%s"`, err, string(b)) } if lresp.LeaseTimeToLiveResponse.ID != int64(id) { return nil, fmt.Errorf("lease: TTL id mismatch") diff --git a/server/storage/backend.go b/server/storage/backend.go index 1137d838981..b7b0d6861ad 100644 --- a/server/storage/backend.go +++ b/server/storage/backend.go @@ -59,10 +59,10 @@ func newBackend(cfg config.ServerConfig, hooks backend.Hooks) backend.Backend { func OpenSnapshotBackend(cfg config.ServerConfig, ss *snap.Snapshotter, snapshot raftpb.Snapshot, hooks *BackendHooks) (backend.Backend, error) { snapPath, err := ss.DBFilePath(snapshot.Metadata.Index) if err != nil { - return nil, fmt.Errorf("failed to find database snapshot file (%v)", err) + return nil, fmt.Errorf("failed to find database snapshot file (%w)", err) } if err := os.Rename(snapPath, cfg.BackendPath()); err != nil { - return nil, fmt.Errorf("failed to rename database snapshot file (%v)", err) + return nil, fmt.Errorf("failed to rename database snapshot file (%w)", err) } return OpenBackend(cfg, hooks), nil } diff --git a/server/storage/mvcc/kvstore_test.go b/server/storage/mvcc/kvstore_test.go index 5bd23a8e366..e7906df5da8 100644 --- a/server/storage/mvcc/kvstore_test.go +++ b/server/storage/mvcc/kvstore_test.go @@ -665,12 +665,12 @@ func TestHashKVWithCompactedAndFutureRevisions(t *testing.T) { } _, _, errFutureRev := s.HashStorage().HashByRev(int64(rev + 1)) - if errFutureRev != ErrFutureRev { + if !errors.Is(errFutureRev, ErrFutureRev) { t.Error(errFutureRev) } _, _, errPastRev := s.HashStorage().HashByRev(int64(compactRev - 1)) - if errPastRev != ErrCompacted { + if !errors.Is(errPastRev, ErrCompacted) { t.Error(errPastRev) } diff --git a/server/storage/schema/membership.go b/server/storage/schema/membership.go index 0ee6fc66770..c79ab8b76a9 100644 --- a/server/storage/schema/membership.go +++ b/server/storage/schema/membership.go @@ -102,7 +102,7 @@ func (s *membershipBackend) readMembersFromBackend() (map[types.ID]*membership.M return nil }) if err != nil { - return nil, nil, fmt.Errorf("couldn't read members from backend: %v", err) + return nil, nil, fmt.Errorf("couldn't read members from backend: %w", err) } err = tx.UnsafeForEach(MembersRemoved, func(k, v []byte) error { @@ -111,7 +111,7 @@ func (s *membershipBackend) readMembersFromBackend() (map[types.ID]*membership.M return nil }) if err != nil { - return nil, nil, fmt.Errorf("couldn't read members_removed from backend: %v", err) + return nil, nil, fmt.Errorf("couldn't read members_removed from backend: %w", err) } return members, removed, nil } diff --git a/server/storage/schema/schema.go b/server/storage/schema/schema.go index eb4fd3a2a5b..6165d33d644 100644 --- a/server/storage/schema/schema.go +++ b/server/storage/schema/schema.go @@ -64,11 +64,11 @@ func Migrate(lg *zap.Logger, tx backend.BatchTx, w WALVersion, target semver.Ver func UnsafeMigrate(lg *zap.Logger, tx backend.UnsafeReadWriter, w WALVersion, target semver.Version) error { current, err := UnsafeDetectSchemaVersion(lg, tx) if err != nil { - return fmt.Errorf("cannot detect storage schema version: %v", err) + return fmt.Errorf("cannot detect storage schema version: %w", err) } plan, err := newPlan(lg, current, target) if err != nil { - return fmt.Errorf("cannot create migration plan: %v", err) + return fmt.Errorf("cannot create migration plan: %w", err) } if target.LessThan(current) { minVersion := w.MinimalEtcdVersion()