Fixing lint errors (#527)

egorikas · web-flow · commit 883c902a31bf · 2025-12-30T11:59:17.000+01:00
## Summary
  - Fix golangci-lint errcheck violations by properly handling errors.

## Changes
  - Use `closers.Close()` for deferred Close() calls in non-test code
  - Use `require.NoError()` for error checks in test files
  - Update `localdb.Fixture()` to accept `*testing.T` for proper test error handling
  - Fix type assertion checks in `file_map.go`

Signed-off-by: egorikas &lt;egorgrishechko@gmail.com&gt;
diff --git a/agent/agentclient/client_test.go b/agent/agentclient/client_test.go
@@ -26,7 +26,8 @@ func TestGetTag(t *testing.T) {
 			handler: func(w http.ResponseWriter, r *http.Request) {
 				require.Equal(t, http.MethodGet, r.Method)
 				require.Equal(t, "/tags/latest", r.URL.Path)
-				fmt.Fprint(w, digest.String())
+				_, err := fmt.Fprint(w, digest.String())
+				require.NoError(t, err)
 			},
 			wantDigest: digest,
 		},
@@ -47,7 +48,8 @@ func TestGetTag(t *testing.T) {
 		{
 			desc: "invalid digest response",
 			handler: func(w http.ResponseWriter, r *http.Request) {
-				fmt.Fprint(w, "invalid-digest")
+				_, err := fmt.Fprint(w, "invalid-digest")
+				require.NoError(t, err)
 			},
 			wantErr: true,
 		},
@@ -56,7 +58,8 @@ func TestGetTag(t *testing.T) {
 			handler: func(w http.ResponseWriter, r *http.Request) {
 				w.Header().Set("Content-Length", "10")
 				w.WriteHeader(http.StatusOK)
-				w.Write([]byte("123"))
+				_, err := w.Write([]byte("123"))
+				require.NoError(t, err)
 			},
 			wantErr: true,
 		},
@@ -95,7 +98,8 @@ func TestDownload(t *testing.T) {
 			handler: func(w http.ResponseWriter, r *http.Request) {
 				require.Equal(t, http.MethodGet, r.Method)
 				require.Equal(t, fmt.Sprintf("/namespace/%s/blobs/%s", namespace, digest), r.URL.Path)
-				fmt.Fprint(w, content)
+				_, err := fmt.Fprint(w, content)
+				require.NoError(t, err)
 			},
 			want: content,
 		},
@@ -119,7 +123,9 @@ func TestDownload(t *testing.T) {
 				require.Error(t, err)
 			} else {
 				require.NoError(t, err)
-				defer r.Close()
+				defer func() {
+					require.NoError(t, r.Close())
+				}()
 				b, err := io.ReadAll(r)
 				require.NoError(t, err)
 				require.Equal(t, test.want, string(b))
diff --git a/build-index/tagclient/client.go b/build-index/tagclient/client.go
@@ -106,7 +106,7 @@ func (c *singleClient) Get(tag string) (core.Digest, error) {
 		}
 		return core.Digest{}, err
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 	b, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return core.Digest{}, fmt.Errorf("read body: %s", err)
@@ -284,7 +284,7 @@ func (c *singleClient) Origin() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	defer resp.Body.Close()
+	defer closers.Close(resp.Body)
 	b, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return "", fmt.Errorf("read body: %s", err)
diff --git a/lib/persistedretry/tagreplication/store_test.go b/lib/persistedretry/tagreplication/store_test.go
@@ -41,7 +41,7 @@ func newStoreMocks(t *testing.T) (*storeMocks, func()) {
 	ctrl := gomock.NewController(t)
 	cleanup.Add(ctrl.Finish)
 
-	db, c := localdb.Fixture()
+	db, c := localdb.Fixture(t)
 	cleanup.Add(c)
 
 	rv := mocktagreplication.NewMockRemoteValidator(ctrl)
diff --git a/lib/persistedretry/writeback/store_test.go b/lib/persistedretry/writeback/store_test.go
@@ -69,7 +69,7 @@ func checkFailed(t *testing.T, store *Store, expected ...*Task) {
 func TestDatabaseNotLocked(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -91,7 +91,7 @@ func TestDatabaseNotLocked(t *testing.T) {
 func TestAddPending(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -106,7 +106,7 @@ func TestAddPending(t *testing.T) {
 func TestAddPendingTwiceReturnsErrTaskExists(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -120,7 +120,7 @@ func TestAddPendingTwiceReturnsErrTaskExists(t *testing.T) {
 func TestAddFailed(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -135,7 +135,7 @@ func TestAddFailed(t *testing.T) {
 func TestAddFailedTwiceReturnsErrTaskExists(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -149,7 +149,7 @@ func TestAddFailedTwiceReturnsErrTaskExists(t *testing.T) {
 func TestStateTransitions(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -172,7 +172,7 @@ func TestStateTransitions(t *testing.T) {
 func TestMarkTaskNotFound(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -186,7 +186,7 @@ func TestMarkTaskNotFound(t *testing.T) {
 func TestRemove(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -205,7 +205,7 @@ func TestRemove(t *testing.T) {
 func TestDelay(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -230,7 +230,7 @@ func TestDelay(t *testing.T) {
 func TestFind(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
@@ -249,7 +249,7 @@ func TestFind(t *testing.T) {
 func TestFindEmpty(t *testing.T) {
 	require := require.New(t)
 
-	db, cleanup := localdb.Fixture()
+	db, cleanup := localdb.Fixture(t)
 	defer cleanup()
 
 	store := NewStore(db)
diff --git a/lib/store/base/file_entry.go b/lib/store/base/file_entry.go
@@ -527,7 +527,7 @@ func (entry *localFileEntry) SetMetadataAt(
 	if err != nil {
 		return false, err
 	}
-	defer f.Close()
+	defer closers.Close(f)
 
 	prev := make([]byte, len(b))
 	if _, err := f.ReadAt(prev, offset); err != nil {
@@ -609,7 +609,7 @@ func compareAndWriteFile(filePath string, b []byte) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	defer f.Close()
+	defer closers.Close(f)
 
 	// Compare with existing data, overwrite if different.
 	buf := make([]byte, int(fs.Size()))
diff --git a/lib/store/base/file_map.go b/lib/store/base/file_map.go
@@ -93,7 +93,8 @@ func NewLATFileMap(clk clock.Clock) FileMap {
 func (fm *lruFileMap) get(name string) (*fileEntryWithAccessTime, bool) {
 	if element, ok := fm.elements[name]; ok {
 		fm.queue.MoveToFront(element)
-		return element.Value.(*fileEntryWithAccessTime), ok
+		entry, ok := element.Value.(*fileEntryWithAccessTime)
+		return entry, ok
 	}
 	return nil, false
 }
@@ -140,7 +141,8 @@ func (fm *lruFileMap) add(name string, e *fileEntryWithAccessTime) bool {
 
 func (fm *lruFileMap) getOldest() (*fileEntryWithAccessTime, bool) {
 	if e := fm.queue.Back(); e != nil {
-		return e.Value.(*fileEntryWithAccessTime), true
+		entry, ok := e.Value.(*fileEntryWithAccessTime)
+		return entry, ok
 	}
 	return nil, false
 }
@@ -149,7 +151,8 @@ func (fm *lruFileMap) remove(name string) (*fileEntryWithAccessTime, bool) {
 	if e, ok := fm.elements[name]; ok {
 		delete(fm.elements, name)
 		fm.queue.Remove(e)
-		return e.Value.(*fileEntryWithAccessTime), ok
+		entry, ok := e.Value.(*fileEntryWithAccessTime)
+		return entry, ok
 	}
 	return nil, false
 }
diff --git a/lib/store/base/file_map_test.go b/lib/store/base/file_map_test.go
@@ -388,7 +388,7 @@ func TestLRUUpdateLastAccessTimeOnMoveFrom(t *testing.T) {
 	fp := filepath.Join(s1.GetDirectory(), name)
 	f, err := os.Create(fp)
 	require.NoError(err)
-	f.Close()
+	require.NoError(f.Close())
 
 	require.NoError(store.NewFileOp().AcceptState(s2).MoveFileFrom(name, s2, fp))
 
diff --git a/lib/store/base/fixtures.go b/lib/store/base/fixtures.go
@@ -32,7 +32,11 @@ func fileStatesFixture() (state1, state2, state3 FileState, run func()) {
 	if err != nil {
 		log.Fatal(err)
 	}
-	cleanup.Add(func() { os.RemoveAll(root) })
+	cleanup.Add(func() {
+		if err := os.RemoveAll(root); err != nil {
+			log.Fatal(err)
+		}
+	})
 
 	state1Dir, err := os.MkdirTemp(root, "state1")
 	if err != nil {
diff --git a/lib/store/ca_store_test.go b/lib/store/ca_store_test.go
@@ -42,15 +42,21 @@ func TestCAStoreInitVolumes(t *testing.T) {
 
 	volume1, err := os.MkdirTemp("/tmp", "volume")
 	require.NoError(err)
-	defer os.RemoveAll(volume1)
+	t.Cleanup(func() {
+		require.NoError(os.RemoveAll(volume1))
+	})
 
 	volume2, err := os.MkdirTemp("/tmp", "volume")
 	require.NoError(err)
-	defer os.RemoveAll(volume2)
+	t.Cleanup(func() {
+		require.NoError(os.RemoveAll(volume2))
+	})
 
 	volume3, err := os.MkdirTemp("/tmp", "volume")
 	require.NoError(err)
-	defer os.RemoveAll(volume3)
+	t.Cleanup(func() {
+		require.NoError(os.RemoveAll(volume3))
+	})
 
 	config.Volumes = []Volume{
 		{Location: volume1, Weight: 100},
diff --git a/lib/torrent/scheduler/conn/conn.go b/lib/torrent/scheduler/conn/conn.go
@@ -32,6 +32,7 @@ import (
 	"github.com/uber/kraken/lib/torrent/storage"
 	"github.com/uber/kraken/lib/torrent/storage/piecereader"
 	"github.com/uber/kraken/utils/bandwidth"
+	"github.com/uber/kraken/utils/closers"
 	"github.com/uber/kraken/utils/memsize"
 )
 
@@ -179,7 +180,7 @@ func (c *Conn) Close() {
 	}
 	go func() {
 		close(c.done)
-		c.nc.Close()
+		closers.Close(c.nc)
 		c.wg.Wait()
 		c.events.ConnClosed(c)
 	}()
@@ -248,7 +249,7 @@ func (c *Conn) readLoop() {
 }
 
 func (c *Conn) sendPiecePayload(pr storage.PieceReader) error {
-	defer pr.Close()
+	defer closers.Close(pr)
 
 	if err := c.bandwidth.ReserveEgress(int64(pr.Length())); err != nil {
 		// TODO(codyg): This is bad. Consider alerting here.
diff --git a/lib/torrent/scheduler/conn/handshaker.go b/lib/torrent/scheduler/conn/handshaker.go
@@ -18,14 +18,14 @@ import (
 	"fmt"
 	"net"
 
+	"github.com/andres-erbsen/clock"
+	"github.com/uber-go/tally"
 	"github.com/uber/kraken/core"
 	"github.com/uber/kraken/gen/go/proto/p2p"
 	"github.com/uber/kraken/lib/torrent/networkevent"
 	"github.com/uber/kraken/lib/torrent/storage"
 	"github.com/uber/kraken/utils/bandwidth"
-
-	"github.com/andres-erbsen/clock"
-	"github.com/uber-go/tally"
+	"github.com/uber/kraken/utils/closers"
 	"github.com/willf/bitset"
 	"go.uber.org/zap"
 )
@@ -172,7 +172,7 @@ func (pc *PendingConn) Namespace() string {
 
 // Close closes the connection.
 func (pc *PendingConn) Close() {
-	pc.nc.Close()
+	closers.Close(pc.nc)
 }
 
 // HandshakeResult wraps data returned from a successful handshake.
@@ -271,7 +271,7 @@ func (h *Handshaker) Initialize(
 	}
 	r, err := h.fullHandshake(nc, peerID, info, remoteBitfields, namespace)
 	if err != nil {
-		nc.Close()
+		closers.Close(nc)
 		return nil, err
 	}
 	return r, nil
diff --git a/localdb/fixtures.go b/localdb/fixtures.go
@@ -16,29 +16,28 @@ package localdb
 import (
 	"os"
 	"path/filepath"
-
-	"github.com/uber/kraken/utils/testutil"
+	"testing"
 
 	"github.com/jmoiron/sqlx"
+	"github.com/stretchr/testify/require"
+	"github.com/uber/kraken/utils/testutil"
 )
 
 // Fixture returns a temporary test database for testing.
-func Fixture() (*sqlx.DB, func()) {
+func Fixture(t *testing.T) (*sqlx.DB, func()) {
 	var cleanup testutil.Cleanup
 	defer cleanup.Recover()
 
 	tmpdir, err := os.MkdirTemp(".", "test-db-")
-	if err != nil {
-		panic(err)
-	}
-	cleanup.Add(func() { os.RemoveAll(tmpdir) })
+	require.NoError(t, err)
+	cleanup.Add(func() {
+		require.NoError(t, os.RemoveAll(tmpdir))
+	})
 
 	source := filepath.Join(tmpdir, "test.db")
 
 	db, err := New(Config{Source: source})
-	if err != nil {
-		panic(err)
-	}
+	require.NoError(t, err)
 
 	return db, cleanup.Run
 }

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ func (c *singleClient) Get(tag string) (core.Digest, error) {`
`106`	`106`	`}`
`107`	`107`	`return core.Digest{}, err`
`108`	`108`	`}`
`109`		`- defer resp.Body.Close()`
	`109`	`+ defer closers.Close(resp.Body)`
`110`	`110`	`b, err := io.ReadAll(resp.Body)`
`111`	`111`	`if err != nil {`
`112`	`112`	`return core.Digest{}, fmt.Errorf("read body: %s", err)`
`@@ -284,7 +284,7 @@ func (c *singleClient) Origin() (string, error) {`
`284`	`284`	`if err != nil {`
`285`	`285`	`return "", err`
`286`	`286`	`}`
`287`		`- defer resp.Body.Close()`
	`287`	`+ defer closers.Close(resp.Body)`
`288`	`288`	`b, err := io.ReadAll(resp.Body)`
`289`	`289`	`if err != nil {`
`290`	`290`	`return "", fmt.Errorf("read body: %s", err)`
Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,8 @@ func NewLATFileMap(clk clock.Clock) FileMap {`
`93`	`93`	`func (fm lruFileMap) get(name string) (fileEntryWithAccessTime, bool) {`
`94`	`94`	`if element, ok := fm.elements[name]; ok {`
`95`	`95`	`fm.queue.MoveToFront(element)`
`96`		`- return element.Value.(*fileEntryWithAccessTime), ok`
	`96`	`+ entry, ok := element.Value.(*fileEntryWithAccessTime)`
	`97`	`+ return entry, ok`
`97`	`98`	`}`
`98`	`99`	`return nil, false`
`99`	`100`	`}`
`@@ -140,7 +141,8 @@ func (fm lruFileMap) add(name string, e fileEntryWithAccessTime) bool {`
`140`	`141`
`141`	`142`	`func (fm lruFileMap) getOldest() (fileEntryWithAccessTime, bool) {`
`142`	`143`	`if e := fm.queue.Back(); e != nil {`
`143`		`- return e.Value.(*fileEntryWithAccessTime), true`
	`144`	`+ entry, ok := e.Value.(*fileEntryWithAccessTime)`
	`145`	`+ return entry, ok`
`144`	`146`	`}`
`145`	`147`	`return nil, false`
`146`	`148`	`}`
`@@ -149,7 +151,8 @@ func (fm lruFileMap) remove(name string) (fileEntryWithAccessTime, bool) {`
`149`	`151`	`if e, ok := fm.elements[name]; ok {`
`150`	`152`	`delete(fm.elements, name)`
`151`	`153`	`fm.queue.Remove(e)`
`152`		`- return e.Value.(*fileEntryWithAccessTime), ok`
	`154`	`+ entry, ok := e.Value.(*fileEntryWithAccessTime)`
	`155`	`+ return entry, ok`
`153`	`156`	`}`
`154`	`157`	`return nil, false`
`155`	`158`	`}`