Skip to content

Commit

Permalink
Merge pull request #18780 from mmorel-35/server/errorlint
Browse files Browse the repository at this point in the history
fix: enable errorlint in server directory
  • Loading branch information
ahrtr authored Oct 30, 2024
2 parents a654cde + 3abdf61 commit 8ec90c6
Show file tree
Hide file tree
Showing 30 changed files with 106 additions and 87 deletions.
4 changes: 2 additions & 2 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (c *ServerConfig) advertiseMatchesCluster() error {
}
mstr := strings.Join(missing, ",")
apStr := strings.Join(apurls, ",")
return fmt.Errorf("--initial-cluster has %s but missing from --initial-advertise-peer-urls=%s (%v)", mstr, apStr, err)
return fmt.Errorf("--initial-cluster has %s but missing from --initial-advertise-peer-urls=%s (%w)", mstr, apStr, err)
}

for url := range apMap {
Expand All @@ -302,7 +302,7 @@ func (c *ServerConfig) advertiseMatchesCluster() error {
// resolved URLs from "--initial-advertise-peer-urls" and "--initial-cluster" did not match or failed
apStr := strings.Join(apurls, ",")
umap := types.URLsMap(map[string]types.URLs{c.Name: c.PeerURLs})
return fmt.Errorf("failed to resolve %s to match --initial-cluster=%s (%v)", apStr, umap.String(), err)
return fmt.Errorf("failed to resolve %s to match --initial-cluster=%s (%w)", apStr, umap.String(), err)
}

func (c *ServerConfig) MemberDir() string { return datadir.ToMemberDir(c.DataDir) }
Expand Down
6 changes: 3 additions & 3 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -993,11 +993,11 @@ func (cfg *Config) Validate() error {
}
if err := checkHostURLs(cfg.AdvertisePeerUrls); err != nil {
addrs := cfg.getAdvertisePeerURLs()
return fmt.Errorf(`--initial-advertise-peer-urls %q must be "host:port" (%v)`, strings.Join(addrs, ","), err)
return fmt.Errorf(`--initial-advertise-peer-urls %q must be "host:port" (%w)`, strings.Join(addrs, ","), err)
}
if err := checkHostURLs(cfg.AdvertiseClientUrls); err != nil {
addrs := cfg.getAdvertiseClientURLs()
return fmt.Errorf(`--advertise-client-urls %q must be "host:port" (%v)`, strings.Join(addrs, ","), err)
return fmt.Errorf(`--advertise-client-urls %q must be "host:port" (%w)`, strings.Join(addrs, ","), err)
}
// Check if conflicting flags are passed.
nSet := 0
Expand Down Expand Up @@ -1066,7 +1066,7 @@ func (cfg *Config) Validate() error {
// Validate distributed tracing configuration but only if enabled.
if cfg.ExperimentalEnableDistributedTracing {
if err := validateTracingConfig(cfg.ExperimentalDistributedTracingSamplingRatePerMillion); err != nil {
return fmt.Errorf("distributed tracing configurition is not valid: (%v)", err)
return fmt.Errorf("distributed tracing configurition is not valid: (%w)", err)
}
}

Expand Down
6 changes: 3 additions & 3 deletions server/embed/config_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,11 +273,11 @@ func setupLogRotation(logOutputs []string, logRotateConfigJSON string) error {
var syntaxError *json.SyntaxError
switch {
case errors.As(err, &syntaxError):
return fmt.Errorf("improperly formatted log rotation config: %v", err)
return fmt.Errorf("improperly formatted log rotation config: %w", err)
case errors.As(err, &unmarshalTypeError):
return fmt.Errorf("invalid log rotation config: %v", err)
return fmt.Errorf("invalid log rotation config: %w", err)
default:
return fmt.Errorf("fail to unmarshal log rotation config: %v", err)
return fmt.Errorf("fail to unmarshal log rotation config: %w", err)
}
}
zap.RegisterSink("rotate", func(u *url.URL) (zap.Sink, error) {
Expand Down
2 changes: 1 addition & 1 deletion server/embed/config_logging_journal_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
func getJournalWriteSyncer() (zapcore.WriteSyncer, error) {
jw, err := logutil.NewJournalWriter(os.Stderr)
if err != nil {
return nil, fmt.Errorf("can't find journal (%v)", err)
return nil, fmt.Errorf("can't find journal (%w)", err)
}
return zapcore.AddSync(jw), nil
}
4 changes: 2 additions & 2 deletions server/embed/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func StartEtcd(inCfg *Config) (e *Etcd, err error) {
memberInitialized = false
urlsmap, token, err = cfg.PeerURLsMapAndToken("etcd")
if err != nil {
return e, fmt.Errorf("error setting up initial cluster: %v", err)
return e, fmt.Errorf("error setting up initial cluster: %w", err)
}
}

Expand Down Expand Up @@ -907,7 +907,7 @@ func parseCompactionRetention(mode, retention string) (ret time.Duration, err er
// periodic compaction
ret, err = time.ParseDuration(retention)
if err != nil {
return 0, fmt.Errorf("error parsing CompactionRetention: %v", err)
return 0, fmt.Errorf("error parsing CompactionRetention: %w", err)
}
}
return ret, nil
Expand Down
6 changes: 3 additions & 3 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 perr == nil:
case errors.Is(perr, flag.ErrHelp):
fmt.Println(flagsline)
os.Exit(0)
default:
Expand Down
14 changes: 8 additions & 6 deletions server/etcdmain/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package etcdmain

import (
errorspkg "errors"
"fmt"
"os"
"runtime"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down
9 changes: 5 additions & 4 deletions server/etcdserver/api/etcdhttp/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package etcdhttp

import (
"encoding/json"
errorspkg "errors"
"fmt"
"net/http"
"strconv"
Expand Down Expand Up @@ -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)
Expand Down
24 changes: 15 additions & 9 deletions server/etcdserver/api/etcdhttp/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package etcdhttp

import (
errorspkg "errors"
"net/http"

"go.uber.org/zap"
Expand All @@ -40,26 +41,31 @@ func writeError(lg *zap.Logger, w http.ResponseWriter, r *http.Request, err erro
if err == nil {
return
}
switch e := err.(type) {
case *v2error.Error:
e.WriteTo(w)
var v2Err *v2error.Error
var httpErr *httptypes.HTTPError
switch {
case errorspkg.As(err, &v2Err):
v2Err.WriteTo(w)

case *httptypes.HTTPError:
if et := e.WriteTo(w); et != nil {
case errorspkg.As(err, &httpErr):
if et := httpErr.WriteTo(w); et != nil {
if lg != nil {
lg.Debug(
"failed to write v2 HTTP error",
zap.String("remote-addr", r.RemoteAddr),
zap.String("internal-server-error", e.Error()),
zap.String("internal-server-error", httpErr.Error()),
zap.Error(et),
)
}
}

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",
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/api/membership/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions server/etcdserver/api/membership/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions server/etcdserver/api/membership/storev2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 8 additions & 6 deletions server/etcdserver/api/rafthttp/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,10 @@ func (h *pipelineHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
receivedBytes.WithLabelValues(types.ID(m.From).String()).Add(float64(len(b)))

if err := h.r.Process(context.TODO(), m); err != nil {
switch v := err.(type) {
case writerToResponse:
v.WriteTo(w)
var writerErr writerToResponse
switch {
case errors.As(err, &writerErr):
writerErr.WriteTo(w)
default:
h.lg.Warn(
"failed to process Raft message",
Expand Down Expand Up @@ -294,11 +295,12 @@ func (h *snapshotHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
)

if err := h.r.Process(context.TODO(), m); err != nil {
switch v := err.(type) {
var writerErr writerToResponse
switch {
// Process may return writerToResponse error when doing some
// additional checks before calling raft.Node.Step.
case writerToResponse:
v.WriteTo(w)
case errors.As(err, &writerErr):
writerErr.WriteTo(w)
default:
msg := fmt.Sprintf("failed to process raft message (%v)", err)
h.lg.Warn(
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/api/rafthttp/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ func (cr *streamReader) dial(t streamType) (io.ReadCloser, error) {
req, err := http.NewRequest(http.MethodGet, uu.String(), nil)
if err != nil {
cr.picker.unreachable(u)
return nil, fmt.Errorf("failed to make http request to %v (%v)", u, err)
return nil, fmt.Errorf("failed to make http request to %v (%w)", u, err)
}
req.Header.Set("X-Server-From", cr.tr.ID.String())
req.Header.Set("X-Server-Version", version.Version)
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/api/snap/snapshotter.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (s *Snapshotter) cleanupSnapdir(filenames []string) (names []string, err er
if strings.HasPrefix(filename, "db.tmp") {
s.lg.Info("found orphaned defragmentation file; deleting", zap.String("path", filename))
if rmErr := os.Remove(filepath.Join(s.dir, filename)); rmErr != nil && !os.IsNotExist(rmErr) {
return names, fmt.Errorf("failed to remove orphaned .snap.db file %s: %v", filename, rmErr)
return names, fmt.Errorf("failed to remove orphaned .snap.db file %s: %w", filename, rmErr)
}
} else {
names = append(names, filename)
Expand Down
5 changes: 3 additions & 2 deletions server/etcdserver/api/v2discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func newProxyFunc(lg *zap.Logger, proxy string) (func(*http.Request) (*url.URL,
}
}
if err != nil {
return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
return nil, fmt.Errorf("invalid proxy address %q: %w", proxy, err)
}

lg.Info("running proxy with discovery", zap.String("proxy-url", proxyURL.String()))
Expand Down Expand Up @@ -361,7 +361,8 @@ func (d *discovery) waitNodes(nodes []*client.Node, size uint64, index uint64) (
)
resp, err := w.Next(context.Background())
if err != nil {
if ce, ok := err.(*client.ClusterError); ok {
var ce *client.ClusterError
if errors.As(err, &ce) {
d.lg.Warn(
"error while waiting for peers",
zap.String("discovery-url", d.url.String()),
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/api/v2store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 6 additions & 2 deletions server/etcdserver/api/v2store/store_ttl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions server/etcdserver/api/v3rpc/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 8ec90c6

Please sign in to comment.