Merge pull request #251 from RedisLabs/fix/small-fixes

Linting and bugfixes

Author: burythehammer

.github/workflows/ci.yml

@@ -17,5 +17,10 @@ jobs:
           go-version-file: go.mod
           cache: true
 
-      - name: Build
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20 # v9.2.0
+        with:
+          version: v2.7.2
+
+      - name: Build and Test
         run: make

.golangci.yml

@@ -0,0 +1,72 @@
+version: "2"
+
+run:
+  timeout: 5m
+  tests: false
+  # Use go.mod version, not toolchain version, to avoid mismatch with golangci-lint build
+  go: '1.24'
+
+formatters:
+  enable:
+    - goimports
+
+linters:
+  enable:
+    # Default linters
+    - errcheck
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
+
+    # High-value additional linters
+    - bodyclose       # HTTP response body not closed
+    - durationcheck   # suspicious duration multiplications
+    - errorlint       # proper error wrapping/comparison
+    - gocritic        # opinionated but useful checks
+    - misspell        # spelling mistakes
+    - nilerr          # returning nil with non-nil error
+    - noctx           # HTTP requests without context
+    - unconvert       # unnecessary type conversions
+
+    # Code quality
+    - copyloopvar     # detects loop variable capture bugs (Go 1.22+)
+    - dupword         # duplicate words in comments/strings
+    - intrange        # suggests using integer range loops (Go 1.22+)
+    - makezero        # finds slice bugs from non-zero initial length
+    - perfsprint      # suggests more efficient string formatting
+    - prealloc        # suggests slice preallocation
+    - usestdlibvars   # use stdlib constants (http.StatusOK, etc.)
+    - wastedassign    # finds wasted assignments
+
+    # Security
+    - gosec           # security checker
+
+  settings:
+    gocritic:
+      enabled-tags:
+        - diagnostic
+        - performance
+      disabled-checks:
+        - hugeParam       # too noisy for this codebase
+
+    errorlint:
+      errorf: true
+      asserts: true
+      comparison: true
+
+    gosec:
+      excludes:
+        - G101  # false positives on env var names like "SECRET_KEY"
+
+    dupword:
+      ignore:
+        - "active"  # "active active" is intentional Redis terminology
+
+  exclusions:
+    paths:
+      - vendor
+    rules:
+      - linters:
+          - gosec
+        text: "G101"  # additional rule-based exclusion for credential false positives

CHANGELOG.md

@@ -2,6 +2,22 @@
 All notable changes to this project will be documented in this file.
 See updating [Changelog example here](https://keepachangelog.com/en/1.0.0/).
 
+## 0.44.1 (Unreleased)
+
+### Fixed:
+* Fixed nil pointer dereference when setting request headers before error check
+* Fixed race condition in rate limiter - mutex now released during sleep
+* Fixed context cancellation not being returned from rate limiter sleep
+* Fixed panic when database list page is empty
+* Fixed inconsistent receiver type in subscriptions List method
+* Fixed many other linting errors raised by new golangci-lint checks
+
+### Changed:
+* `DeleteWithQuery` now accepts a query parameter instead of always passing nil
+
+### Added:
+* Added golangci-lint to CI pipeline with comprehensive linting configuration
+
 ## 0.44.0 (5th December 2025)
 
 ### Added:

client.go

@@ -260,8 +260,8 @@ func redactPasswords(data string) string {
 }
 
 func escapePath(path string) string {
-	escapedPath := strings.Replace(path, "\n", "", -1)
-	escapedPath = strings.Replace(escapedPath, "\r", "", -1)
+	escapedPath := strings.ReplaceAll(path, "\n", "")
+	escapedPath = strings.ReplaceAll(escapedPath, "\r", "")
 	return escapedPath
 }
 

client_test.go

@@ -3,7 +3,7 @@ package rediscloud_api
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -71,7 +71,7 @@ func TestCredentialTripper_LogsRequestAndResponseBodies(t *testing.T) {
 		ProtoMajor:    1,
 		ProtoMinor:    1,
 		Header:        map[string][]string{},
-		Body:          ioutil.NopCloser(bytes.NewBufferString(`{"Here":"Value"}`)),
+		Body:          io.NopCloser(bytes.NewBufferString(`{"Here":"Value"}`)),
 		ContentLength: 16,
 		Host:          "example.org",
 	}
@@ -81,7 +81,7 @@ func TestCredentialTripper_LogsRequestAndResponseBodies(t *testing.T) {
 		ProtoMajor: 1,
 		ProtoMinor: 1,
 		Header:     map[string][]string{},
-		Body:       ioutil.NopCloser(bytes.NewBufferString(`{"Response":"Value"}`)),
+		Body:       io.NopCloser(bytes.NewBufferString(`{"Response":"Value"}`)),
 	}
 
 	mockTripper.On("RoundTrip", request).Return(expected, nil)
@@ -195,7 +195,7 @@ func TestCredentialTripper_RedactPasswordFromBody(t *testing.T) {
 		ProtoMajor:    1,
 		ProtoMinor:    1,
 		Header:        map[string][]string{},
-		Body:          ioutil.NopCloser(bytes.NewBufferString(`{"password":"pass"}`)),
+		Body:          io.NopCloser(bytes.NewBufferString(`{"password":"pass"}`)),
 		ContentLength: 19,
 		Host:          "example.org",
 	}
@@ -205,7 +205,7 @@ func TestCredentialTripper_RedactPasswordFromBody(t *testing.T) {
 		ProtoMajor: 1,
 		ProtoMinor: 1,
 		Header:     map[string][]string{},
-		Body:       ioutil.NopCloser(bytes.NewBufferString(`{"password":"REDACTED"}`)),
+		Body:       io.NopCloser(bytes.NewBufferString(`{"password":"REDACTED"}`)),
 	}
 
 	mockTripper.On("RoundTrip", request).Return(expected, nil)
@@ -252,7 +252,7 @@ func TestCredentialTripper_RedactPasswordFromNestedBody(t *testing.T) {
 		ProtoMajor:    1,
 		ProtoMinor:    1,
 		Header:        map[string][]string{},
-		Body:          ioutil.NopCloser(bytes.NewBufferString(`{"security": {"password":"pass", "global_password":"globalpass"}}`)),
+		Body:          io.NopCloser(bytes.NewBufferString(`{"security": {"password":"pass", "global_password":"globalpass"}}`)),
 		ContentLength: 65,
 		Host:          "example.org",
 	}
@@ -262,7 +262,7 @@ func TestCredentialTripper_RedactPasswordFromNestedBody(t *testing.T) {
 		ProtoMajor: 1,
 		ProtoMinor: 1,
 		Header:     map[string][]string{},
-		Body:       ioutil.NopCloser(bytes.NewBufferString(`{"security": {"password":"REDACTED", "global_password":"REDACTED"}}`)),
+		Body:       io.NopCloser(bytes.NewBufferString(`{"security": {"password":"REDACTED", "global_password":"REDACTED"}}`)),
 	}
 
 	mockTripper.On("RoundTrip", request).Return(expected, nil)

go.mod

@@ -7,16 +7,16 @@ toolchain go1.25.5
 require (
 	github.com/avast/retry-go/v4 v4.7.0
 	github.com/stretchr/testify v1.11.1
-	golang.org/x/tools v0.39.0
+	golang.org/x/tools v0.40.0
 )
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/stretchr/objx v0.5.2 // indirect
-	golang.org/x/mod v0.30.0 // indirect
-	golang.org/x/sync v0.18.0 // indirect
-	golang.org/x/sys v0.38.0 // indirect
-	golang.org/x/telemetry v0.0.0-20251111182119-bc8e575c7b54 // indirect
+	golang.org/x/mod v0.31.0 // indirect
+	golang.org/x/sync v0.19.0 // indirect
+	golang.org/x/sys v0.39.0 // indirect
+	golang.org/x/telemetry v0.0.0-20251203150158-8fff8a5912fc // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -10,16 +10,16 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
 github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
 github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
-golang.org/x/mod v0.30.0 h1:fDEXFVZ/fmCKProc/yAXXUijritrDzahmwwefnjoPFk=
-golang.org/x/mod v0.30.0/go.mod h1:lAsf5O2EvJeSFMiBxXDki7sCgAxEUcZHXoXMKT4GJKc=
-golang.org/x/sync v0.18.0 h1:kr88TuHDroi+UVf+0hZnirlk8o8T+4MrK6mr60WkH/I=
-golang.org/x/sync v0.18.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
-golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc=
-golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
-golang.org/x/telemetry v0.0.0-20251111182119-bc8e575c7b54 h1:E2/AqCUMZGgd73TQkxUMcMla25GB9i/5HOdLr+uH7Vo=
-golang.org/x/telemetry v0.0.0-20251111182119-bc8e575c7b54/go.mod h1:hKdjCMrbv9skySur+Nek8Hd0uJ0GuxJIoIX2payrIdQ=
-golang.org/x/tools v0.39.0 h1:ik4ho21kwuQln40uelmciQPp9SipgNDdrafrYA4TmQQ=
-golang.org/x/tools v0.39.0/go.mod h1:JnefbkDPyD8UU2kI5fuf8ZX4/yUeh9W877ZeBONxUqQ=
+golang.org/x/mod v0.31.0 h1:HaW9xtz0+kOcWKwli0ZXy79Ix+UW/vOfmWI5QVd2tgI=
+golang.org/x/mod v0.31.0/go.mod h1:43JraMp9cGx1Rx3AqioxrbrhNsLl2l/iNAvuBkrezpg=
+golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
+golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
+golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk=
+golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
+golang.org/x/telemetry v0.0.0-20251203150158-8fff8a5912fc h1:bH6xUXay0AIFMElXG2rQ4uiE+7ncwtiOdPfYK1NK2XA=
+golang.org/x/telemetry v0.0.0-20251203150158-8fff8a5912fc/go.mod h1:hKdjCMrbv9skySur+Nek8Hd0uJ0GuxJIoIX2payrIdQ=
+golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA=
+golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=

internal/http_client.go

@@ -71,8 +71,8 @@ func (c *HttpClient) Delete(ctx context.Context, name, path string, requestBody
 	return c.connectionWithRetries(ctx, http.MethodDelete, name, path, nil, requestBody, responseBody)
 }
 
-func (c *HttpClient) DeleteWithQuery(ctx context.Context, name, path string, requestBody interface{}, responseBody interface{}) error {
-	return c.connectionWithRetries(ctx, http.MethodDelete, name, path, nil, requestBody, responseBody)
+func (c *HttpClient) DeleteWithQuery(ctx context.Context, name, path string, query url.Values, requestBody interface{}, responseBody interface{}) error {
+	return c.connectionWithRetries(ctx, http.MethodDelete, name, path, query, requestBody, responseBody)
 }
 
 func (c *HttpClient) connectionWithRetries(ctx context.Context, method, name, path string, query url.Values, requestBody interface{}, responseBody interface{}) error {
@@ -88,7 +88,7 @@ func (c *HttpClient) connectionWithRetries(ctx context.Context, method, name, pa
 			}
 			var target *HTTPError
 			if errors.As(err, &target) && target.StatusCode == http.StatusTooManyRequests {
-				c.logger.Println(fmt.Sprintf("status code 429 received, request will be retried"))
+				c.logger.Println("status code 429 received, request will be retried")
 				return true
 			}
 			return false
@@ -126,14 +126,13 @@ func (c *HttpClient) connection(ctx context.Context, method, name, path string,
 	}
 
 	request, err := http.NewRequestWithContext(ctx, method, u, body)
-
-	// The API expects this entry in the header in all requests.
-	request.Header.Set("Content-Type", "application/json")
-
 	if err != nil {
 		return fmt.Errorf("failed to create request to %s: %w", name, err)
 	}
 
+	// The API expects this entry in the header in all requests.
+	request.Header.Set("Content-Type", "application/json")
+
 	response, err := c.client.Do(request)
 	if err != nil {
 		return fmt.Errorf("failed to %s: %w", name, err)
@@ -149,7 +148,7 @@ func (c *HttpClient) connection(ctx context.Context, method, name, path string,
 		}
 	}
 
-	defer response.Body.Close()
+	defer func() { _ = response.Body.Close() }()
 
 	if response.StatusCode > 299 {
 		body, _ := io.ReadAll(response.Body)

internal/model.go

@@ -52,7 +52,7 @@ func (e *Error) Error() string {
 	return fmt.Sprintf("%s - %s: %s", redis.StringValue(e.Status), redis.StringValue(e.Type), redis.StringValue(e.Description))
 }
 
-var errorStatusCode = regexp.MustCompile("^(\\d*).*$")
+var errorStatusCode = regexp.MustCompile(`^(\d*).*$`)
 var _ error = &Error{}
 
 // TaskResponse is the high-level response when a Create/Update/Delete operation is in progress.

internal/rate_limit.go

@@ -2,7 +2,6 @@ package internal
 
 import (
 	"context"
-	"fmt"
 	"sync"
 	"time"
 )
@@ -59,12 +58,21 @@ func (rl *fixedWindowCountRateLimiter) Wait(ctx context.Context) error {
 		windowEnd = rl.windowStart.Add(rl.period)
 	}
 
-	if rl.count == rl.limit {
-		delay := windowEnd.Sub(time.Now())
+	if rl.count >= rl.limit {
+		delay := time.Until(windowEnd)
+		rl.mu.Unlock()
 		err := sleepWithContext(ctx, delay)
+		rl.mu.Lock()
 		if err != nil {
 			return err
 		}
+		// After sleeping, the window may have reset - recheck and reset count if needed
+		now := time.Now()
+		windowEnd = rl.windowStart.Add(rl.period)
+		if now.After(windowEnd) {
+			rl.count = 0
+			rl.windowStart = &windowEnd
+		}
 	}
 	rl.count++
 	return nil
@@ -82,11 +90,12 @@ func sleepWithContext(ctx context.Context, d time.Duration) error {
 	select {
 	case <-ctx.Done():
 		if !timer.Stop() {
-			return fmt.Errorf("context expired before timer stopped")
+			<-timer.C // Drain the timer channel to prevent leaks
 		}
+		return ctx.Err()
 	case <-timer.C:
+		return nil
 	}
-	return nil
 }
 
 var _ RateLimiter = &fixedWindowCountRateLimiter{}