Improve code quality

Author: tstromberg

Makefile

@@ -2,7 +2,7 @@ cover: test ## Run all the tests and opens the coverage report
 	go tool cover -html=coverage.txt
 
 test:
-	go test ./...
+	go test -race ./...
 
 fmt: ## gofmt and goimports all go files
 	find . -name '*.go' -not -wholename './vendor/*' | while read -r file; do gofmt -w -s "$$file"; goimports -w "$$file"; done

README.md

@@ -5,130 +5,102 @@
 [![Go Report Card](https://goreportcard.com/badge/github.com/codeGROOVE-dev/retry-go?style=flat-square)](https://goreportcard.com/report/github.com/codeGROOVE-dev/retry-go)
 [![Go Reference](https://pkg.go.dev/badge/github.com/codeGROOVE-dev/retry-go.svg)](https://pkg.go.dev/github.com/codeGROOVE-dev/retry-go)
 
-**Zero dependencies. Memory-bounded. Uptime-focused.**
+**Zero dependencies. Memory-bounded. No goroutine leaks. No panics.**
 
-*Because 99.99% uptime means your retry logic can't be the failure point.*
+*Hard guarantees: Bounded memory (1000 errors max). No allocations in hot path. Context-aware cancellation.*
 
 Actively maintained fork of [avast/retry-go](https://github.com/avast/retry-go) focused on correctness and resource efficiency. 100% API compatible drop-in replacement.
 
-**Key improvements:**
-- ⚡ Zero external dependencies
-- 🔒 Memory-bounded error accumulation  
-- 🛡️ Integer overflow protection in backoff
-- 🎯 Enhanced readability and debuggability
-- 📊 Predictable behavior under load
+**Production guarantees:**
+- Memory bounded: Max 1000 errors stored (configurable via maxErrors constant)
+- No goroutine leaks: Uses caller's goroutine exclusively
+- Integer overflow safe: Backoff capped at 2^62 to prevent wraparound
+- Context-aware: Cancellation checked before each attempt
+- No panics: All edge cases return errors
+- Predictable jitter: Uses math/rand/v2 for consistent performance
+- Zero allocations after init in success path
 
 ## Quick Start
 
-### Basic Retry with Error Handling
+### Simple Retry
 
 ```go
-import (
-    "net/http"
-    "github.com/codeGROOVE-dev/retry-go"
-)
+// Retry a flaky operation up to 10 times (default)
+err := retry.Do(func() error {
+    return doSomethingFlaky()
+})
+```
 
-// Retry API call with exponential backoff + jitter
+### Retry with Custom Attempts
+
+```go
+// Retry up to 5 times with exponential backoff
 err := retry.Do(
     func() error {
-        resp, err := http.Get("https://api.stripe.com/v1/charges")
+        resp, err := http.Get("https://api.example.com/data")
         if err != nil {
             return err
         }
         defer resp.Body.Close()
-        
-        if resp.StatusCode >= 500 {
-            return fmt.Errorf("server error: %d", resp.StatusCode)
-        }
         return nil
     },
     retry.Attempts(5),
-    retry.DelayType(retry.CombineDelay(retry.BackOffDelay, retry.RandomDelay)),
 )
 ```
 
-### Retry with Data Return (Generics)
+### Overly-complicated production configuration
 
 ```go
-import (
-    "context"
-    "encoding/json"
-    "github.com/codeGROOVE-dev/retry-go"
-)
-
-// Database query with timeout and retry
-users, err := retry.DoWithData(
-    func() ([]User, error) {
-        return db.FindActiveUsers(ctx)
-    },
-    retry.Attempts(3),
-    retry.Context(ctx),
-    retry.DelayType(retry.BackOffDelay),
-)
-```
-
-### Production Configuration
 
-```go
-// Payment processing with comprehensive retry logic
+// Overly-complex production pattern: bounded retries with circuit breaking
 err := retry.Do(
-    func() error { return paymentGateway.Charge(ctx, amount) },
-    retry.Attempts(5),
-    retry.Context(ctx),
-    retry.DelayType(retry.FullJitterBackoffDelay),
-    retry.MaxDelay(30*time.Second),
+    func() error {
+        return processPayment(ctx, req)
+    },
+    retry.Attempts(3),                        // Hard limit
+    retry.Context(ctx),                       // Respect cancellation
+    retry.MaxDelay(10*time.Second),           // Cap backoff
+    retry.AttemptsForError(0, ErrRateLimit),  // Stop on rate limit
     retry.OnRetry(func(n uint, err error) {
-        log.Warn("Payment retry", "attempt", n+1, "error", err)
+        log.Printf("retry attempt %d: %v", n, err)
     }),
     retry.RetryIf(func(err error) bool {
-        return !isAuthError(err) // Don't retry 4xx errors
+        // Only retry on network errors
+        var netErr net.Error
+        return errors.As(err, &netErr) && netErr.Temporary()
     }),
 )
 ```
 
-## Key Features
+### Preventing Cascading Failures
 
-**Unrecoverable Errors** - Stop immediately for certain errors:
 ```go
-if resp.StatusCode == 401 {
-    return retry.Unrecoverable(errors.New("auth failed"))
+// Stop retry storms with Unrecoverable
+if errors.Is(err, context.DeadlineExceeded) {
+    return retry.Unrecoverable(err) // Don't retry timeouts
 }
-```
 
-**Context Integration** - Timeout and cancellation:
-```go
-ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
-retry.Do(dbQuery, retry.Context(ctx))
+// Per-error type limits prevent thundering herd
+retry.AttemptsForError(0, ErrCircuitOpen)    // Fail fast on circuit breaker
+retry.AttemptsForError(1, sql.ErrTxDone)     // One retry for tx errors
+retry.AttemptsForError(5, ErrServiceUnavailable) // More retries for 503s
 ```
 
-**Error-Specific Limits** - Different retry counts per error type:
-```go
-retry.AttemptsForError(1, sql.ErrTxDone)  // Don't retry transaction errors
-```
-
-## Performance & Scale
-
-| Metric | Value |
-|--------|-------|
-| Memory overhead | ~200 bytes per operation |
-| Error accumulation | Bounded at 1000 errors (DoS protection) |
-| Goroutines | Uses calling goroutine only |
-| High-throughput safe | No hidden allocations or locks |
+## Failure Modes & Limits
 
-## Library Comparison
-
-**[cenkalti/backoff](https://github.com/cenkalti/backoff)** - Complex interface, requires manual retry loops, no error accumulation.
-
-**[sethgrid/pester](https://github.com/sethgrid/pester)** - HTTP-only, lacks general-purpose retry logic.
-
-**[matryer/try](https://github.com/matryer/try)** - Popular but non-standard API, missing production features.
-
-**[rafaeljesus/retry-go](https://github.com/rafaeljesus/retry-go)** - Similar design but lacks error-specific limits and comprehensive context handling.
-
-**This fork** builds on avast/retry-go's solid foundation with correctness fixes and resource optimizations.
+| Scenario | Behavior | Limit |
+|----------|----------|-------|
+| Error accumulation | Old errors dropped after limit | 1000 errors |
+| Attempt overflow | Stops retrying | uint max (~4B) |
+| Backoff overflow | Capped at max duration | 2^62 ns |
+| Context cancelled | Returns immediately | No retries |
+| Timer returns nil | Returns error | Fail safe |
+| Panic in retryable func | Propagates panic | No swallowing |
 
 ## Installation
 
+Requires Go 1.22 or higher.
+
 ```bash
 go get github.com/codeGROOVE-dev/retry-go
 ```
@@ -141,4 +113,4 @@ go get github.com/codeGROOVE-dev/retry-go
 
 ---
 
-*Production retry logic that just works.*
+*Production retry logic that just works.*

examples/custom_retry_function_test.go

@@ -13,20 +13,20 @@ import (
 	"github.com/codeGROOVE-dev/retry-go"
 )
 
-// RetriableError is a custom error that contains a positive duration for the next retry
+// RetriableError is a custom error that contains a positive duration for the next retry.
 type RetriableError struct {
 	Err        error
 	RetryAfter time.Duration
 }
 
-// Error returns error message and a Retry-After duration
+// Error returns error message and a Retry-After duration.
 func (e *RetriableError) Error() string {
 	return fmt.Sprintf("%s (retry after %v)", e.Err.Error(), e.RetryAfter)
 }
 
 var _ error = (*RetriableError)(nil)
 
-// TestCustomRetryFunction shows how to use a custom retry function
+// TestCustomRetryFunction shows how to use a custom retry function.
 func TestCustomRetryFunction(t *testing.T) {
 	attempts := 5 // server succeeds after 5 attempts
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -90,7 +90,6 @@ func TestCustomRetryFunction(t *testing.T) {
 			return retry.BackOffDelay(n, err, config)
 		}),
 	)
-
 	// Server responds with: <body content>
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)

examples/delay_based_on_error_test.go

@@ -62,9 +62,8 @@ func TestCustomRetryFunctionBasedOnKindOfError(t *testing.T) {
 		retry.DelayType(func(n uint, err error, config *retry.Config) time.Duration {
 			var retryAfterErr RetryAfterError
 			if errors.As(err, &retryAfterErr) {
-				if t, err := parseRetryAfter(retryAfterErr.response.Header.Get("Retry-After")); err == nil {
-					return time.Until(t)
-				}
+				t := parseRetryAfter(retryAfterErr.response.Header.Get("Retry-After"))
+				return time.Until(t)
 			}
 			var someOtherErr SomeOtherError
 			if errors.As(err, &someOtherErr) {
@@ -83,7 +82,7 @@ func TestCustomRetryFunctionBasedOnKindOfError(t *testing.T) {
 	}
 }
 
-// use https://github.com/aereal/go-httpretryafter instead
-func parseRetryAfter(_ string) (time.Time, error) {
-	return time.Now().Add(1 * time.Second), nil //nolint:unparam // error is always nil for test simplicity
+// use https://github.com/aereal/go-httpretryafter instead.
+func parseRetryAfter(_ string) time.Time {
+	return time.Now().Add(1 * time.Second)
 }

examples/errors_history_test.go

@@ -10,7 +10,7 @@ import (
 )
 
 // TestErrorHistory shows an example of how to get all the previous errors when
-// retry.Do ends in success
+// retry.Do ends in success.
 func TestErrorHistory(t *testing.T) {
 	attempts := 3 // server succeeds after 3 attempts
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

go.mod

@@ -1,3 +1,3 @@
 module github.com/codeGROOVE-dev/retry-go
 
-go 1.20
+go 1.22

options.go

@@ -2,11 +2,10 @@ package retry //nolint:revive // More than 5 public structs are necessary for AP
 
 import (
 	"context"
-	cryptorand "crypto/rand"
-	"encoding/binary"
 	"errors"
 	"fmt"
 	"math"
+	"math/rand/v2"
 	"time"
 )
 
@@ -217,29 +216,7 @@ func RandomDelay(_ uint, _ error, config *Config) time.Duration {
 	if config.maxJitter <= 0 {
 		return 0
 	}
-	return time.Duration(secureRandomInt63n(int64(config.maxJitter)))
-}
-
-// secureRandomInt63n returns a non-negative pseudo-random number in [0,n) using crypto/rand.
-func secureRandomInt63n(n int64) int64 {
-	if n <= 0 {
-		return 0
-	}
-	var b [8]byte
-	for {
-		if _, err := cryptorand.Read(b[:]); err != nil {
-			// Fall back to 0 on error rather than panic
-			return 0
-		}
-		// Clear sign bit to ensure non-negative
-		val := int64(binary.BigEndian.Uint64(b[:]) & 0x7FFFFFFFFFFFFFFF) //nolint:gosec // Bitwise AND to clear sign bit is safe
-		// Rejection sampling to avoid modulo bias
-		const maxInt63 = 0x7FFFFFFFFFFFFFFF // max int63
-		maxVal := int64(maxInt63)
-		if val < maxVal-(maxVal%n) {
-			return val % n
-		}
-	}
+	return time.Duration(rand.Int64N(int64(config.maxJitter))) //nolint:gosec // Cryptographic randomness not needed for retry jitter
 }
 
 // CombineDelay creates a DelayTypeFunc that sums the delays from multiple strategies.
@@ -293,7 +270,7 @@ func FullJitterBackoffDelay(attempt uint, _ error, config *Config) time.Duration
 	if ceiling <= 0 {
 		return 0
 	}
-	return time.Duration(secureRandomInt63n(int64(ceiling)))
+	return time.Duration(rand.Int64N(int64(ceiling))) //nolint:gosec // Cryptographic randomness not needed for retry jitter
 }
 
 // OnRetry sets a callback function that is called after each failed attempt.

retry.go

@@ -239,7 +239,10 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) (
 	}
 
 	if config.lastErrorOnly {
-		return emptyT, errorLog.Unwrap()
+		if len(errorLog) > 0 {
+			return emptyT, errorLog.Unwrap()
+		}
+		return emptyT, nil
 	}
 	return emptyT, errorLog
 }
@@ -256,7 +259,10 @@ func (e Error) Error() string {
 		return "retry: all attempts failed"
 	}
 
+	// Pre-size builder for better performance
+	// Estimate: prefix (30) + each error (~50 chars avg)
 	var b strings.Builder
+	b.Grow(30 + len(e)*50)
 	b.WriteString("retry: all attempts failed:")
 
 	for i, err := range e {