Skip to content

Commit 07aa7e9

Browse files
egorikasegorikas
authored
Fix golangci-lint errors across codebase #6 (#455)
Fixed 79+ lint issues across 20 files by standardizing error handling and test patterns. Signed-off-by: egorikas <egorgrishechko@gmail.com>
1 parent 306f0f4 commit 07aa7e9

File tree

28 files changed

+180
-82
lines changed

28 files changed

+180
-82
lines changed

‎agent/agentserver/server.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/uber/kraken/lib/store"
3333
"github.com/uber/kraken/lib/torrent/scheduler"
3434
"github.com/uber/kraken/tracker/announceclient"
35+
"github.com/uber/kraken/utils/closers"
3536
"github.com/uber/kraken/utils/handler"
3637
"github.com/uber/kraken/utils/httputil"
3738

@@ -125,7 +126,9 @@ func (s *Server) getTagHandler(w http.ResponseWriter, r *http.Request) error {
125126
}
126127
return handler.Errorf("get tag: %s", err)
127128
}
128-
io.WriteString(w, d.String())
129+
if _, err := io.WriteString(w, d.String()); err != nil {
130+
return fmt.Errorf("write response: %s", err)
131+
}
129132
return nil
130133
}
131134

@@ -208,15 +211,19 @@ func (s *Server) healthHandler(w http.ResponseWriter, r *http.Request) error {
208211
if err := s.sched.Probe(); err != nil {
209212
return handler.Errorf("probe torrent client: %s", err)
210213
}
211-
io.WriteString(w, "OK")
214+
if _, err := io.WriteString(w, "OK"); err != nil {
215+
return fmt.Errorf("write response: %s", err)
216+
}
212217
return nil
213218
}
214219

215220
func (s *Server) readinessCheckHandler(w http.ResponseWriter, r *http.Request) error {
216221
if s.config.readinessCacheTTL != 0 {
217222
rCacheValid := s.lastReady.Add(s.config.readinessCacheTTL).After(time.Now())
218223
if rCacheValid {
219-
io.WriteString(w, "OK")
224+
if _, err := io.WriteString(w, "OK"); err != nil {
225+
return fmt.Errorf("write response: %s", err)
226+
}
220227
return nil
221228
}
222229
}
@@ -251,14 +258,16 @@ func (s *Server) readinessCheckHandler(w http.ResponseWriter, r *http.Request) e
251258
}
252259

253260
s.lastReady = time.Now()
254-
io.WriteString(w, "OK")
261+
if _, err := io.WriteString(w, "OK"); err != nil {
262+
return fmt.Errorf("write response: %s", err)
263+
}
255264
return nil
256265
}
257266

258267
// patchSchedulerConfigHandler restarts the agent torrent scheduler with
259268
// the config in request body.
260269
func (s *Server) patchSchedulerConfigHandler(w http.ResponseWriter, r *http.Request) error {
261-
defer r.Body.Close()
270+
defer closers.Close(r.Body)
262271
var config scheduler.Config
263272
if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
264273
return handler.Errorf("json decode: %s", err).Status(http.StatusBadRequest)

‎agent/cmd/cmd.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"sync"
2121
"time"
2222

23+
"github.com/uber-go/tally"
2324
"github.com/uber/kraken/agent/agentserver"
2425
"github.com/uber/kraken/build-index/tagclient"
2526
"github.com/uber/kraken/core"
@@ -32,11 +33,10 @@ import (
3233
"github.com/uber/kraken/metrics"
3334
"github.com/uber/kraken/nginx"
3435
"github.com/uber/kraken/tracker/announceclient"
36+
"github.com/uber/kraken/utils/closers"
3537
"github.com/uber/kraken/utils/configutil"
3638
"github.com/uber/kraken/utils/log"
3739
"github.com/uber/kraken/utils/netutil"
38-
39-
"github.com/uber-go/tally"
4040
"go.uber.org/zap"
4141
)
4242

@@ -133,7 +133,11 @@ func Run(flags *Flags, opts ...Option) {
133133
log.SetGlobalLogger(overrides.logger.Sugar())
134134
} else {
135135
zlog := log.ConfigureLogger(config.ZapLogging)
136-
defer zlog.Sync()
136+
defer func() {
137+
if err := zlog.Sync(); err != nil {
138+
fmt.Printf("Failed to sync logger: %s", err)
139+
}
140+
}()
137141
}
138142

139143
stats := overrides.metrics
@@ -143,7 +147,7 @@ func Run(flags *Flags, opts ...Option) {
143147
log.Fatalf("Failed to init metrics: %s", err)
144148
}
145149
stats = s
146-
defer closer.Close()
150+
defer closers.Close(closer)
147151
}
148152

149153
go metrics.EmitVersion(stats)

‎lib/backend/hdfsbackend/webhdfs/client.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ func (c *client) GetFileStatus(path string) (FileStatus, error) {
282282
}
283283
return FileStatus{}, nnErr
284284
}
285-
defer resp.Body.Close()
285+
defer closers.Close(resp.Body)
286286
var fsr fileStatusResponse
287287
if err := json.NewDecoder(resp.Body).Decode(&fsr); err != nil {
288288
return FileStatus{}, fmt.Errorf("decode body: %s", err)
@@ -308,7 +308,7 @@ func (c *client) ListFileStatus(path string) ([]FileStatus, error) {
308308
}
309309
return nil, nnErr
310310
}
311-
defer resp.Body.Close()
311+
defer closers.Close(resp.Body)
312312
var lsr listStatusResponse
313313
if err := json.NewDecoder(resp.Body).Decode(&lsr); err != nil {
314314
return nil, fmt.Errorf("decode body: %s", err)

‎lib/backend/registrybackend/tagclient.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/uber/kraken/lib/backend"
2727
"github.com/uber/kraken/lib/backend/backenderrors"
2828
"github.com/uber/kraken/lib/backend/registrybackend/security"
29+
"github.com/uber/kraken/utils/closers"
2930
"github.com/uber/kraken/utils/dockerutil"
3031
"github.com/uber/kraken/utils/httputil"
3132
"go.uber.org/zap"
@@ -138,7 +139,7 @@ func (c *TagClient) Download(namespace, name string, dst io.Writer) error {
138139
if err != nil {
139140
return fmt.Errorf("check blob exists: %s", err)
140141
}
141-
defer resp.Body.Close()
142+
defer closers.Close(resp.Body)
142143

143144
if resp.StatusCode == http.StatusNotFound {
144145
return backenderrors.ErrBlobNotFound

‎lib/hrw/fixtures.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package hrw
1616
import (
1717
"crypto/rand"
1818
"encoding/hex"
19+
"fmt"
1920
"strconv"
2021
"testing"
2122

@@ -60,15 +61,17 @@ func RendezvousHashFixture(t *testing.T, numKeys int, hashFactory HashFactory, s
6061
}
6162

6263
// HashKeyFixture generate #numkeys random keys according to a hash function
63-
func HashKeyFixture(numKeys int, hashFactory HashFactory) []string {
64+
func HashKeyFixture(numKeys int, hashFactory HashFactory) ([]string, error) {
6465
var keys []string
6566
b := make([]byte, 64)
6667

6768
for i := 0; i < numKeys; i++ {
68-
rand.Read(b)
69+
if _, err := rand.Read(b); err != nil {
70+
return nil, fmt.Errorf("failed to read random bytes: %w", err)
71+
}
6972
key := hex.EncodeToString(b)
7073
keys = append(keys, key)
7174
}
7275

73-
return keys
76+
return keys, nil
7477
}

‎lib/hrw/rendezvous_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/spaolacci/murmur3"
2727
"github.com/stretchr/testify/assert"
28+
"github.com/stretchr/testify/require"
2829
)
2930

3031
func TestScoreFunctionFloatPrecision(t *testing.T) {
@@ -193,8 +194,11 @@ func testRemoveNodes(numKeys int, hash HashFactory, scoreFunc UIntToFloat, t *te
193194
}
194195

195196
func testReturnNodesLength(numKeys int, hash HashFactory, scoreFunc UIntToFloat, t *testing.T) {
197+
require := require.New(t)
198+
196199
rh, _ := RendezvousHashFixture(t, 0, hash, scoreFunc, 100, 200, 400, 800)
197-
keys := HashKeyFixture(1, hash)
200+
keys, err := HashKeyFixture(1, hash)
201+
require.NoError(err)
198202

199203
var scores []float64
200204
for _, node := range rh.Nodes {
@@ -207,8 +211,11 @@ func testReturnNodesLength(numKeys int, hash HashFactory, scoreFunc UIntToFloat,
207211
}
208212

209213
func testReturnNodesOrder(numKeys int, hash HashFactory, scoreFunc UIntToFloat, t *testing.T) {
214+
require := require.New(t)
215+
210216
rh, _ := RendezvousHashFixture(t, 0, hash, scoreFunc, 100, 200, 400, 800)
211-
keys := HashKeyFixture(1, hash)
217+
keys, err := HashKeyFixture(1, hash)
218+
require.NoError(err)
212219

213220
var scores []float64
214221
for _, node := range rh.Nodes {

‎lib/middleware/middleware_test.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ func TestScopeByEndpoint(t *testing.T) {
4545
for _, test := range tests {
4646
t.Run(test.method+" "+test.path, func(t *testing.T) {
4747
require := require.New(t)
48-
4948
stats := tally.NewTestScope("", nil)
5049

5150
r := chi.NewRouter()
@@ -73,7 +72,6 @@ func TestScopeByEndpoint(t *testing.T) {
7372

7473
func TestLatencyTimer(t *testing.T) {
7574
require := require.New(t)
76-
7775
stats := tally.NewTestScope("", nil)
7876

7977
r := chi.NewRouter()
@@ -104,40 +102,50 @@ func TestLatencyTimer(t *testing.T) {
104102
func TestStatusCounter(t *testing.T) {
105103
tests := []struct {
106104
desc string
107-
handler func(http.ResponseWriter, *http.Request)
105+
handler func(*testing.T) http.HandlerFunc
108106
expectedStatus string
109107
}{
110108
{
111109
"empty handler counts 200",
112-
func(http.ResponseWriter, *http.Request) {},
110+
func(*testing.T) http.HandlerFunc {
111+
return func(http.ResponseWriter, *http.Request) {}
112+
},
113113
"200",
114114
}, {
115115
"writes count 200",
116-
func(w http.ResponseWriter, _ *http.Request) { io.WriteString(w, "OK") },
116+
func(t *testing.T) http.HandlerFunc {
117+
return func(w http.ResponseWriter, _ *http.Request) {
118+
_, err := io.WriteString(w, "OK")
119+
require.NoError(t, err)
120+
}
121+
},
117122
"200",
118123
}, {
119124
"write header",
120-
func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(500) },
125+
func(*testing.T) http.HandlerFunc {
126+
return func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(500) }
127+
},
121128
"500",
122129
}, {
123130
"multiple write header calls only measures first call",
124-
func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(400); w.WriteHeader(500) },
131+
func(*testing.T) http.HandlerFunc {
132+
return func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(400); w.WriteHeader(500) }
133+
},
125134
"400",
126135
},
127136
}
128137
for _, test := range tests {
129138
t.Run(test.desc, func(t *testing.T) {
130-
require := require.New(t)
131-
132139
stats := tally.NewTestScope("", nil)
133140

134141
r := chi.NewRouter()
135142
r.Use(StatusCounter(stats))
136-
r.Get("/foo/{foo}", test.handler)
143+
r.Get("/foo/{foo}", test.handler(t))
137144

138145
addr, stop := testutil.StartServer(r)
139146
defer stop()
140147

148+
require := require.New(t)
141149
for i := 0; i < 5; i++ {
142150
_, err := http.Get(fmt.Sprintf("http://%s/foo/x", addr))
143151
require.NoError(err)

‎lib/persistedretry/writeback/executor.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/uber/kraken/lib/persistedretry"
2424
"github.com/uber/kraken/lib/store"
2525
"github.com/uber/kraken/lib/store/metadata"
26+
"github.com/uber/kraken/utils/closers"
2627
"github.com/uber/kraken/utils/log"
2728
)
2829

@@ -101,7 +102,7 @@ func (e *Executor) upload(t *Task) error {
101102
}
102103
return fmt.Errorf("get file: %s", err)
103104
}
104-
defer f.Close()
105+
defer closers.Close(f)
105106

106107
if err := client.Upload(t.Namespace, t.Name, f); err != nil {
107108
return fmt.Errorf("upload: %s", err)

‎lib/store/base/file_op_test.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ func TestFileOp(t *testing.T) {
6161
for _, test := range tests {
6262
testName := runtime.FuncForPC(reflect.ValueOf(test).Pointer()).Name()
6363
t.Run(testName, func(t *testing.T) {
64-
require := require.New(t)
6564
s, cleanup := store.fixture()
6665
defer cleanup()
66+
require := require.New(t)
6767
test(require, s)
6868
})
6969
}
@@ -320,7 +320,9 @@ func testGetFileReader(require *require.Assertions, storeBundle *fileStoreTestBu
320320
// Get ReadWriter and modify the file.
321321
readWriter, err := store.NewFileOp().AcceptState(s1).GetFileReadWriter(fn, 100 /*readPartSize */, 100 /*writePartSize*/)
322322
require.NoError(err)
323-
defer readWriter.Close()
323+
defer func() {
324+
require.NoError(readWriter.Close())
325+
}()
324326
_, err = readWriter.Write([]byte{'t', 'e', 's', 't', '\n'})
325327
require.NoError(err)
326328

@@ -349,7 +351,7 @@ func testGetFileReader(require *require.Assertions, storeBundle *fileStoreTestBu
349351

350352
reader, err := store.NewFileOp().AcceptState(s1).GetFileReader(fn, 100 /*readPartSize */)
351353
require.NoError(err)
352-
reader.Close()
354+
require.NoError(reader.Close())
353355
}
354356

355357
func testGetFileReadWriter(require *require.Assertions, storeBundle *fileStoreTestBundle) {

‎lib/store/ca_store.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/uber/kraken/lib/store/base"
3434
"github.com/uber/kraken/lib/store/metadata"
3535
"github.com/uber/kraken/utils/cache"
36+
"github.com/uber/kraken/utils/closers"
3637
"github.com/uber/kraken/utils/log"
3738
)
3839

@@ -174,13 +175,13 @@ func (s *CAStore) MoveUploadFileToCache(uploadName, cacheName string) error {
174175
if err != nil {
175176
return err
176177
}
177-
defer s.DeleteUploadFile(uploadName)
178+
defer s.deferDeleteUploadFile(uploadName)()
178179

179180
f, err := s.uploadStore.newFileOp().GetFileReader(uploadName, s.uploadStore.readPartSize)
180181
if err != nil {
181182
return fmt.Errorf("get file reader %s: %s", uploadName, err)
182183
}
183-
defer f.Close()
184+
defer closers.Close(f)
184185
if err := s.verify(f, cacheName); err != nil {
185186
return fmt.Errorf("verify digest: %s", err)
186187
}
@@ -209,13 +210,13 @@ func (s *CAStore) writeCacheFile(name string, write func(w FileReadWriter) error
209210
if err := s.CreateUploadFile(tmp, 0); err != nil {
210211
return fmt.Errorf("create upload file: %s", err)
211212
}
212-
defer s.DeleteUploadFile(tmp)
213+
defer s.deferDeleteUploadFile(tmp)()
213214

214215
w, err := s.GetUploadFileReadWriter(tmp)
215216
if err != nil {
216217
return fmt.Errorf("get upload writer: %s", err)
217218
}
218-
defer w.Close()
219+
defer closers.Close(w)
219220

220221
if err := write(w); err != nil {
221222
return err

0 commit comments

Comments
 (0)