🧹 chore(proxy): address round-robin review feedback

Author: james-yusuke

app.go

@@ -1205,11 +1205,12 @@ func (e *Error) Error() string {
 
 // NewError creates a new Error instance with an optional message
 func NewError(code int, message ...string) *Error {
-	err := &Error{Code: code}
+	err := &Error{
+		Code:    code,
+		Message: utils.StatusMessage(code),
+	}
 	if len(message) > 0 {
 		err.Message = message[0]
-	} else {
-		err.Message = utils.StatusMessage(code)
 	}
 	return err
 }

app_test.go

@@ -1982,22 +1982,9 @@ func Benchmark_NewError_Parallel(b *testing.B) {
 // go test -run Test_NewError
 func Test_NewError(t *testing.T) {
 	t.Parallel()
-
-	t.Run("custom message", func(t *testing.T) {
-		t.Parallel()
-
-		e := NewError(StatusForbidden, "permission denied")
-		require.Equal(t, StatusForbidden, e.Code)
-		require.Equal(t, "permission denied", e.Message)
-	})
-
-	t.Run("status message", func(t *testing.T) {
-		t.Parallel()
-
-		e := NewError(StatusForbidden)
-		require.Equal(t, StatusForbidden, e.Code)
-		require.Equal(t, "Forbidden", e.Message)
-	})
+	e := NewError(StatusForbidden, "permission denied")
+	require.Equal(t, StatusForbidden, e.Code)
+	require.Equal(t, "permission denied", e.Message)
 }
 
 // go test -run Test_NewError_Format

middleware/proxy/bench_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"sync"
 	"testing"
 
 	"github.com/gofiber/fiber/v3"
@@ -273,6 +274,51 @@ func BenchmarkDomainForward_HostMatchPath(b *testing.B) {
 	}
 }
 
+func BenchmarkURLRoundrobinGet(b *testing.B) {
+	pool := []*url.URL{
+		{Scheme: schemeHTTP, Host: "a.example"},
+		{Scheme: schemeHTTP, Host: "b.example"},
+		{Scheme: schemeHTTP, Host: "c.example"},
+	}
+	b.Run("atomic", func(b *testing.B) {
+		r := &urlRoundrobin{pool: pool}
+		b.ReportAllocs()
+		b.RunParallel(func(pb *testing.PB) {
+			for pb.Next() {
+				_ = r.get()
+			}
+		})
+	})
+	b.Run("mutex", func(b *testing.B) {
+		r := &mutexURLRoundrobin{pool: pool}
+		b.ReportAllocs()
+		b.RunParallel(func(pb *testing.PB) {
+			for pb.Next() {
+				_ = r.get()
+			}
+		})
+	})
+}
+
+type mutexURLRoundrobin struct {
+	pool []*url.URL
+
+	current int
+	sync.Mutex
+}
+
+func (r *mutexURLRoundrobin) get() *url.URL {
+	r.Lock()
+	defer r.Unlock()
+
+	if r.current >= len(r.pool) {
+		r.current %= len(r.pool)
+	}
+	result := r.pool[r.current]
+	r.current++
+	return result
+}
+
 // noopRoundTripper is the minimum surface for fasthttp.RoundTripper used
 // by BenchmarkDomainForward_HostMatchPath. It returns 204 No Content
 // without touching the network.

middleware/proxy/roundrobin_bench_test.go

@@ -9,6 +9,8 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -854,6 +856,63 @@ func Test_Security_BalancerForward_InvalidUpstreamPanicsAtConstruction(t *testin
 	})
 }
 
+// Test_Security_RoundRobin_WrapsAround covers both the normal cyclic
+// selection and recovery from a counter value near the uint64 limit.
+func Test_Security_RoundRobin_WrapsAround(t *testing.T) {
+	t.Parallel()
+	a := &url.URL{Scheme: schemeHTTP, Host: "a"}
+	b := &url.URL{Scheme: schemeHTTP, Host: "b"}
+	c := &url.URL{Scheme: schemeHTTP, Host: "c"}
+
+	t.Run("current index", func(t *testing.T) {
+		t.Parallel()
+
+		r := &urlRoundrobin{pool: []*url.URL{a, b}}
+		r.next.Store(5)
+		require.Same(t, b, r.get(), "5 %% 2 should select pool[1]")
+		require.Same(t, a, r.get())
+	})
+
+	t.Run("counter limit", func(t *testing.T) {
+		t.Parallel()
+
+		r := &urlRoundrobin{pool: []*url.URL{a, b, c}}
+		r.next.Store(^uint64(0) - 1)
+		require.Same(t, c, r.get())
+		require.Same(t, a, r.get())
+		require.Same(t, b, r.get())
+	})
+}
+
+func Test_Security_RoundRobin_ConcurrentSelection(t *testing.T) {
+	t.Parallel()
+	pool := []*url.URL{
+		{Scheme: schemeHTTP, Host: "a"},
+		{Scheme: schemeHTTP, Host: "b"},
+		{Scheme: schemeHTTP, Host: "c"},
+	}
+	r := &urlRoundrobin{pool: pool}
+
+	var counts [3]atomic.Int32
+	var wg sync.WaitGroup
+	for range 300 {
+		wg.Go(func() {
+			selected := r.get()
+			for i, candidate := range pool {
+				if selected == candidate {
+					counts[i].Add(1)
+					return
+				}
+			}
+		})
+	}
+	wg.Wait()
+
+	for i := range counts {
+		require.Equal(t, int32(100), counts[i].Load())
+	}
+}
+
 // Test_Security_Balancer_PanicsOnInvalidUpstream covers the panic
 // branch when a server fails validation at Config-time.
 func Test_Security_Balancer_PanicsOnInvalidUpstream(t *testing.T) {