fix: preserve response on error in middlewares and clean up linter issues

- fix cache and repeater middlewares to return response with error per http.RoundTripper contract
- fix wrapcheck linter issues by properly wrapping errors
- fix testifylint issues by using proper assertion methods
- update golangci-lint configuration for cleaner exclusion rules
- fix unused parameter warnings in test files

Author: umputun

.github/workflows/ci.yml

@@ -31,7 +31,7 @@ jobs:
           TZ: "America/Chicago"
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v7
         with:
           version: latest
 

.golangci.yml

@@ -1,104 +1,84 @@
-linters-settings:
-  govet:
-    shadow: true
-  golint:
-    min-confidence: 0.6
-  gocyclo:
-    min-complexity: 15
-  maligned:
-    suggest-new: true
-  dupl:
-    threshold: 100
-  goconst:
-    min-len: 2
-    min-occurrences: 2
-  misspell:
-    locale: US
-  lll:
-    line-length: 140
-  gocritic:
-    enabled-tags:
-      - performance
-      - style
-      - experimental
-    disabled-checks:
-      - wrapperFunc
-      - hugeParam
-      - rangeValCopy
-
+version: "2"
+run:
+  concurrency: 4
 linters:
-  disable-all: true
+  default: none
   enable:
-    - revive
-    - govet
-    - unconvert
-    - gosec
-    - unparam
-    - unused
-    - typecheck
-    - ineffassign
-    - stylecheck
+    - contextcheck
+    - decorder
+    - errorlint
+    - exptostd
+    - gochecknoglobals
     - gochecknoinits
     - gocritic
+    - gosec
+    - govet
+    - ineffassign
     - nakedret
-    - gosimple
+    - nilerr
     - prealloc
-    - gofmt
-
-  fast: false
-
-
-run:
-  concurrency: 4
+    - predeclared
+    - revive
+    - staticcheck
+    - testifylint
+    - thelper
+    - unconvert
+    - unparam
+    - unused
+    - nestif
+    - wrapcheck
+  settings:
+    goconst:
+      min-len: 2
+      min-occurrences: 2
+    gocritic:
+      disabled-checks:
+        - wrapperFunc
+      enabled-tags:
+        - performance
+        - style
+        - experimental
+    gocyclo:
+      min-complexity: 15
+    govet:
+      enable-all: true
+      disable:
+        - fieldalignment
+    lll:
+      line-length: 140
+    misspell:
+      locale: US
 
-issues:
-  exclude-dirs:
-    - vendor
-  exclude-rules:
-    - text: "should have a package comment, unless it's in another file for this package"
-      linters:
-        - golint
-    - text: "exitAfterDefer:"
-      linters:
-        - gocritic
-    - text: "whyNoLint: include an explanation for nolint directive"
-      linters:
-        - gocritic
-    - text: "go.mongodb.org/mongo-driver/bson/primitive.E"
-      linters:
-        - govet
-    - text: "weak cryptographic primitive"
-      linters:
-        - gosec
-    - text: "integer overflow conversion"
-      linters:
-        - gosec
-    - text: "should have a package comment"
-      linters:
-        - revive
-    - text: "at least one file in a package should have a package comment"
-      linters:
-        - stylecheck
-    - text: "commentedOutCode: may want to remove commented-out code"
-      linters:
-        - gocritic
-    - text: "unnamedResult: consider giving a name to these results"
-      linters:
-        - gocritic
-    - text: "var-naming: don't use an underscore in package name"
-      linters:
-        - revive
-    - text: "should not use underscores in package names"
-      linters:
-        - stylecheck
-    - text: "struct literal uses unkeyed fields"
-      linters:
-        - govet
-    - linters:
-        - unparam
-        - unused
-        - revive
-      path: _test\.go$
-      text: "unused-parameter"
-  exclude-use-default: false
+  exclusions:
+    generated: lax
+    rules:
+      - linters:
+          - staticcheck
+        text: at least one file in a package should have a package comment
+      - linters:
+          - revive
+        text: should have a package comment
+      - linters:
+          - revive
+        text: 'unused-parameter'
+      - linters:
+          - dupl
+          - gosec
+        path: _test\.go
+    paths:
+      - vendor
+      - third_party$
+      - builtin$
+      - examples$
 
+formatters:
+  enable:
+    - gofmt
+  exclusions:
+    generated: lax
+    paths:
+      - vendor
+      - third_party$
+      - builtin$
+      - examples$
+      - mocks$

middleware/cache.go

@@ -31,7 +31,7 @@ type CacheMiddleware struct {
 	allowedMethods []string
 
 	cache map[string]CacheEntry
-	keys  []string // Maintains insertion order
+	keys  []string // maintains insertion order
 	mu    sync.Mutex
 }
 
@@ -46,7 +46,11 @@ func (c *CacheMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
 		}
 	}
 	if !methodAllowed {
-		return c.next.RoundTrip(req)
+		resp, err := c.next.RoundTrip(req)
+		if err != nil {
+			return resp, fmt.Errorf("cache: transport error: %w", err)
+		}
+		return resp, nil
 	}
 
 	key := c.makeKey(req) // generate cache key based on request
@@ -56,7 +60,7 @@ func (c *CacheMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
 	for len(c.keys) > 0 {
 		oldestKey := c.keys[0]
 		if time.Since(c.cache[oldestKey].createdAt) < c.ttl {
-			break // Stop once we find a non-expired entry
+			break // stop once we find a non-expired entry
 		}
 		delete(c.cache, oldestKey)
 		c.keys = c.keys[1:]
@@ -83,7 +87,7 @@ func (c *CacheMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
 	// fetch fresh response
 	resp, err := c.next.RoundTrip(req)
 	if err != nil {
-		return resp, err
+		return resp, fmt.Errorf("cache: transport error: %w", err)
 	}
 
 	// check if response code is allowed for caching
@@ -94,7 +98,7 @@ func (c *CacheMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
 	// read and store response body
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return resp, err
+		return resp, fmt.Errorf("cache: failed to read response body: %w", err)
 	}
 	_ = resp.Body.Close()
 	resp.Body = io.NopCloser(bytes.NewReader(body))

middleware/cache/cache.go

@@ -76,7 +76,7 @@ func (m *Middleware) Middleware(next http.RoundTripper) http.RoundTripper {
 		cachedResp, e := m.Get(key, func() (interface{}, error) {
 			resp, err = next.RoundTrip(req)
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("cache: transport error: %w", err)
 			}
 			if resp.Body == nil {
 				return nil, nil
@@ -101,7 +101,7 @@ func (m *Middleware) extractCacheKey(req *http.Request) (key string, err error)
 		}
 		reqBody, e := io.ReadAll(io.LimitReader(req.Body, maxBodySize))
 		if e != nil {
-			return "", e
+			return "", fmt.Errorf("cache: failed to read body: %w", e)
 		}
 		_ = req.Body.Close()
 		req.Body = io.NopCloser(bytes.NewReader(reqBody))

middleware/cache/cache_test.go

@@ -19,7 +19,7 @@ import (
 func Test_extractCacheKey(t *testing.T) {
 	makeReq := func(method, url string, body io.Reader, headers http.Header) *http.Request {
 		res, err := http.NewRequest(method, url, body)
-		require.NoError(t, err)
+		assert.NoError(t, err)
 		if headers != nil {
 			res.Header = headers
 		}
@@ -104,12 +104,12 @@ func Test_extractCacheKey(t *testing.T) {
 			c := New(nil, tt.opts...)
 			c.dbg = true
 			keyDbg, err := c.extractCacheKey(tt.req)
-			require.NoError(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, tt.keyDbg, keyDbg)
 
 			c.dbg = false
 			keyHash, err := c.extractCacheKey(tt.req)
-			require.NoError(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, tt.keyHash, keyHash)
 
 		})
@@ -118,7 +118,7 @@ func Test_extractCacheKey(t *testing.T) {
 }
 
 func TestMiddleware_Handle(t *testing.T) {
-	cacheMock := mocks.CacheSvc{GetFunc: func(key string, fn func() (interface{}, error)) (interface{}, error) {
+	cacheMock := mocks.CacheSvc{GetFunc: func(_ string, fn func() (interface{}, error)) (interface{}, error) {
 		return fn()
 	}}
 	c := New(&cacheMock)
@@ -127,7 +127,7 @@ func TestMiddleware_Handle(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("k1", "v1")
 		_, err := w.Write([]byte("something"))
-		require.NoError(t, err)
+		assert.NoError(t, err)
 	}))
 
 	client := http.Client{Transport: c.Middleware(http.DefaultTransport)}
@@ -142,7 +142,7 @@ func TestMiddleware_Handle(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "something", string(v))
 	assert.Equal(t, "v1", resp.Header.Get("k1"))
-	assert.Equal(t, 1, len(cacheMock.GetCalls()))
+	assert.Len(t, cacheMock.GetCalls(), 1)
 	assert.Contains(t, cacheMock.GetCalls()[0].Key, "?k=v##GET####")
 
 	req, err = http.NewRequest("GET", ts.URL+"?k=v", http.NoBody)
@@ -155,12 +155,12 @@ func TestMiddleware_Handle(t *testing.T) {
 	v, err = io.ReadAll(resp.Body)
 	require.NoError(t, err)
 	assert.Equal(t, "something", string(v))
-	assert.Equal(t, 2, len(cacheMock.GetCalls()))
+	assert.Len(t, cacheMock.GetCalls(), 2)
 	assert.Contains(t, cacheMock.GetCalls()[1].Key, "?k=v##GET####")
 }
 
 func TestMiddleware_HandleMethodDisabled(t *testing.T) {
-	cacheMock := mocks.CacheSvc{GetFunc: func(key string, fn func() (interface{}, error)) (interface{}, error) {
+	cacheMock := mocks.CacheSvc{GetFunc: func(_ string, fn func() (interface{}, error)) (interface{}, error) {
 		return fn()
 	}}
 	c := New(&cacheMock, Methods("PUT"))
@@ -169,7 +169,7 @@ func TestMiddleware_HandleMethodDisabled(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("k1", "v1")
 		_, err := w.Write([]byte("something"))
-		require.NoError(t, err)
+		assert.NoError(t, err)
 	}))
 
 	client := http.Client{Transport: c.Middleware(http.DefaultTransport)}
@@ -179,14 +179,14 @@ func TestMiddleware_HandleMethodDisabled(t *testing.T) {
 	resp, err := client.Do(req)
 	require.NoError(t, err)
 	assert.Equal(t, 200, resp.StatusCode)
-	assert.Equal(t, 0, len(cacheMock.GetCalls()))
+	assert.Empty(t, cacheMock.GetCalls())
 
 	req, err = http.NewRequest("PUT", ts.URL+"?k=v", http.NoBody)
 	require.NoError(t, err)
 	resp, err = client.Do(req)
 	require.NoError(t, err)
 	assert.Equal(t, 200, resp.StatusCode)
-	assert.Equal(t, 1, len(cacheMock.GetCalls()))
+	assert.Len(t, cacheMock.GetCalls(), 1)
 }
 
 func TestMiddleware_EdgeCases(t *testing.T) {
@@ -261,6 +261,6 @@ type errorReader struct {
 	err error
 }
 
-func (e *errorReader) Read(p []byte) (n int, err error) {
+func (e *errorReader) Read(_ []byte) (n int, err error) {
 	return 0, e.err
 }

middleware/cache_test.go

@@ -155,8 +155,8 @@ func TestCache_Options(t *testing.T) {
 
 		// second request - should be cached
 		req2, _ := http.NewRequest(http.MethodGet, "http://example.com/2", http.NoBody)
-		_, _ = h.RoundTrip(req2) // First call: should hit the backend
-		_, _ = h.RoundTrip(req2) // Second call: should be served from cache
+		_, _ = h.RoundTrip(req2) // first call: should hit the backend
+		_, _ = h.RoundTrip(req2) // second call: should be served from cache
 
 		// third request - triggers eviction of first request
 		req3, _ := http.NewRequest(http.MethodGet, "http://example.com/3", http.NoBody)

middleware/circuit_breaker_test.go

@@ -35,5 +35,5 @@ func TestCircuitBreaker(t *testing.T) {
 	assert.Equal(t, 201, resp.StatusCode)
 
 	assert.Equal(t, 1, rmock.Calls())
-	assert.Equal(t, 1, len(cbMock.ExecuteCalls()))
+	assert.Len(t, cbMock.ExecuteCalls(), 1)
 }