Skip to content

Commit 0619f04

Browse files
committed
feat: show rate-limit UI feedback during 429 retry backoff
1 parent fa5bb2f commit 0619f04

7 files changed

Lines changed: 281 additions & 3 deletions

File tree

pkg/mocks/networking.go

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/networking/middleware/response_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ func Test_ResponseMiddleware(t *testing.T) {
2828
w.WriteHeader(http.StatusUnauthorized)
2929
case "/500":
3030
w.WriteHeader(http.StatusInternalServerError)
31+
case "/429":
32+
w.WriteHeader(http.StatusTooManyRequests)
3133
case "/jsonapi-SNYK-0003":
3234
w.WriteHeader(http.StatusBadRequest)
3335
_, err := w.Write([]byte(`{"jsonapi":{"version":"1.0"},"errors":[{"status":"400","detail":"project found but does not contain a target id","id":"6b86a7a8-9efb-4ed1-b3a2-1ed78ced78a9","title":"Not Found","meta":{"created":"2025-02-21T03:14:00.318931623Z"}}]}`))
@@ -73,7 +75,7 @@ func Test_ResponseMiddleware(t *testing.T) {
7375
config.Set(configuration.AUTHENTICATION_ADDITIONAL_URLS, []string{server.URL})
7476
rt := middleware.NewReponseMiddleware(http.DefaultTransport, config, errHandler)
7577

76-
codes := []int{400, 401, 500}
78+
codes := []int{400, 401, 429, 500}
7779
for _, code := range codes {
7880
snykErr := snyk_errors.Error{}
7981
url := fmt.Sprintf("%s/%d", server.URL, code)
@@ -83,6 +85,10 @@ func Test_ResponseMiddleware(t *testing.T) {
8385
assert.NotNil(t, res)
8486
assert.ErrorAs(t, err, &snykErr)
8587
assert.Equal(t, code, snykErr.StatusCode)
88+
if code == http.StatusTooManyRequests {
89+
assert.Equal(t, "SNYK-0001", snykErr.ErrorCode)
90+
assert.Equal(t, "error", snykErr.Level)
91+
}
8692

8793
// observability metadata
8894
assert.Equal(t, snykErr.Meta["request-id"], "1234")

pkg/networking/middleware/retry_middleware.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/snyk/error-catalog-golang-public/snyk"
1515

1616
"github.com/snyk/go-application-framework/pkg/configuration"
17+
"github.com/snyk/go-application-framework/pkg/ui/uitypes"
1718
)
1819

1920
const defaultMaxAttemptsCount = 1 // Per default max network attempts (=1) this means retries are disabled and need to be enabled via the configuration
@@ -48,6 +49,7 @@ type RetryMiddleware struct {
4849
nextRoundtripper http.RoundTripper
4950
config configuration.Configuration
5051
logger *zerolog.Logger
52+
ui uitypes.UserInterface
5153
}
5254

5355
func NewRetryMiddleware(config configuration.Configuration, logger *zerolog.Logger, roundTripper http.RoundTripper) *RetryMiddleware {
@@ -58,6 +60,15 @@ func NewRetryMiddleware(config configuration.Configuration, logger *zerolog.Logg
5860
}
5961
}
6062

63+
func NewRetryMiddlewareWithUI(config configuration.Configuration, logger *zerolog.Logger, roundTripper http.RoundTripper, ui uitypes.UserInterface) *RetryMiddleware {
64+
return &RetryMiddleware{
65+
nextRoundtripper: roundTripper,
66+
config: config,
67+
logger: logger,
68+
ui: ui,
69+
}
70+
}
71+
6172
func (rm RetryMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
6273
var finalResponse *http.Response
6374
var finalError error
@@ -66,6 +77,7 @@ func (rm RetryMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
6677
var retryAfterSeconds = defaultRetryAfterSeconds
6778
var actualAttempts int = 0
6879
var cachedMaxRetries *int = nil // Per-request cached max retries
80+
var statusCode int = 0
6981

7082
if tmp := rm.config.GetInt(ConfigurationKeyRequestAttempts); tmp > 0 {
7183
maxAttempts = tmp
@@ -114,6 +126,8 @@ func (rm RetryMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
114126
return response, backoff.Permanent(err)
115127
}
116128

129+
statusCode = response.StatusCode
130+
117131
// Cache max retry attempts for the current request
118132
if cachedMaxRetries == nil {
119133
calculated := getMaxRetryAttempts(response, maxAttempts)
@@ -129,9 +143,20 @@ func (rm RetryMiddleware) RoundTrip(req *http.Request) (*http.Response, error) {
129143
return response, nil
130144
}
131145

146+
notify := func(_ error, duration time.Duration) {
147+
resolvedMax := maxAttempts
148+
if cachedMaxRetries != nil {
149+
resolvedMax = *cachedMaxRetries
150+
}
151+
rm.notifyRateLimit(duration, statusCode, actualAttempts, resolvedMax)
152+
}
153+
132154
backoffMethod := backoff.NewExponentialBackOff()
133155
backoffMethod.InitialInterval = time.Duration(retryAfterSeconds) * time.Second
134-
finalResponse, finalError = backoff.Retry(req.Context(), op, backoff.WithBackOff(backoffMethod))
156+
finalResponse, finalError = backoff.Retry(req.Context(), op,
157+
backoff.WithBackOff(backoffMethod),
158+
backoff.WithNotify(notify),
159+
)
135160

136161
finalError = rm.filterRetryError(finalError, actualAttempts)
137162
return finalResponse, finalError

pkg/networking/middleware/retry_middleware_test.go

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,20 @@ import (
99
"net/http"
1010
"net/http/httptest"
1111
"net/url"
12+
"sync"
1213
"testing"
1314
"time"
1415

1516
"github.com/rs/zerolog"
1617
"github.com/snyk/error-catalog-golang-public/snyk"
18+
"github.com/snyk/error-catalog-golang-public/snyk_errors"
1719
"github.com/stretchr/testify/assert"
1820
"github.com/stretchr/testify/require"
1921

2022
"github.com/cenkalti/backoff/v5"
2123

2224
"github.com/snyk/go-application-framework/pkg/configuration"
25+
"github.com/snyk/go-application-framework/pkg/ui/uitypes"
2326
)
2427

2528
// Helper to create a response
@@ -550,3 +553,184 @@ func Test_parseRetryDelay(t *testing.T) {
550553
})
551554
}
552555
}
556+
557+
func Test_filterRetryError(t *testing.T) {
558+
logger := zerolog.Nop()
559+
rm := RetryMiddleware{logger: &logger}
560+
561+
t.Run("retry exhausted returns nil", func(t *testing.T) {
562+
err := rm.filterRetryError(backoff.Permanent(errRetryNecessary), 3)
563+
assert.NoError(t, err)
564+
})
565+
566+
t.Run("delay max exceeded returns nil", func(t *testing.T) {
567+
err := rm.filterRetryError(backoff.Permanent(errRetryDelayMaxExceeded), 1)
568+
assert.NoError(t, err)
569+
})
570+
571+
t.Run("non-sentinel error passes through unchanged", func(t *testing.T) {
572+
originalErr := errors.New("some other error")
573+
err := rm.filterRetryError(originalErr, 1)
574+
assert.Equal(t, originalErr, err)
575+
})
576+
}
577+
578+
// setupRetryMiddleware wires RetryMiddleware with a counting transport and the given config.
579+
func setupRetryMiddleware(
580+
t *testing.T,
581+
logger *zerolog.Logger,
582+
ui uitypes.UserInterface,
583+
maxAttempts int,
584+
roundTrip func(req *http.Request, attempt int) (*http.Response, error),
585+
) (*RetryMiddleware, *int) {
586+
t.Helper()
587+
attemptCount := 0
588+
fn := func(req *http.Request) (*http.Response, error) {
589+
attemptCount++
590+
return roundTrip(req, attemptCount)
591+
}
592+
rt := &failRoundtripper{t: t, roundTripFn: &fn}
593+
config := configuration.NewWithOpts()
594+
config.Set(ConfigurationKeyRequestAttempts, maxAttempts)
595+
config.Set(configurationKeyRetryAfter, 1)
596+
if ui != nil {
597+
return NewRetryMiddlewareWithUI(config, logger, rt, ui), &attemptCount
598+
}
599+
return NewRetryMiddleware(config, logger, rt), &attemptCount
600+
}
601+
602+
func Test_retryMiddleware_429_exhausted(t *testing.T) {
603+
logger := zerolog.Nop()
604+
605+
always429 := func(req *http.Request, _ int) (*http.Response, error) {
606+
return &http.Response{
607+
StatusCode: http.StatusTooManyRequests,
608+
Header: http.Header{"Retry-After": []string{"1"}},
609+
Request: req,
610+
}, nil
611+
}
612+
613+
t.Run("returns last 429 response and nil error", func(t *testing.T) {
614+
sut, attemptCount := setupRetryMiddleware(t, &logger, nil, 1, always429)
615+
resp, err := sut.RoundTrip(httptest.NewRequest(http.MethodGet, "/", nil))
616+
617+
assert.NoError(t, err)
618+
assert.NotNil(t, resp)
619+
assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode)
620+
assert.Equal(t, 3, *attemptCount, "429 override enforces at least 3 attempts even when maxAttempts=1")
621+
})
622+
623+
t.Run("shows rate-limit warnings during retries", func(t *testing.T) {
624+
trackingUI := &trackingUserInterface{}
625+
sut, attemptCount := setupRetryMiddleware(t, &logger, trackingUI, 1, always429)
626+
resp, err := sut.RoundTrip(httptest.NewRequest(http.MethodGet, "/", nil))
627+
628+
assert.NoError(t, err)
629+
assert.NotNil(t, resp)
630+
assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode)
631+
assert.Equal(t, 3, *attemptCount, "429 override enforces at least 3 attempts even when maxAttempts=1")
632+
633+
trackingUI.mu.Lock()
634+
defer trackingUI.mu.Unlock()
635+
assert.NotEmpty(t, trackingUI.warnErrors, "Rate-limit warning should appear since 429 override causes retries")
636+
})
637+
}
638+
639+
func Test_retryMiddleware_429_then_success(t *testing.T) {
640+
logger := zerolog.Nop()
641+
642+
once429ThenOK := func(req *http.Request, attempt int) (*http.Response, error) {
643+
if attempt < 2 {
644+
return &http.Response{
645+
StatusCode: http.StatusTooManyRequests,
646+
Header: http.Header{},
647+
Request: req,
648+
}, nil
649+
}
650+
return &http.Response{
651+
StatusCode: http.StatusOK,
652+
Header: http.Header{},
653+
Request: req,
654+
}, nil
655+
}
656+
657+
t.Run("nil UI does not panic", func(t *testing.T) {
658+
sut, attemptCount := setupRetryMiddleware(t, &logger, nil, 3, once429ThenOK)
659+
resp, err := sut.RoundTrip(httptest.NewRequest(http.MethodGet, "/", nil))
660+
661+
require.NoError(t, err)
662+
require.NotNil(t, resp)
663+
assert.Equal(t, http.StatusOK, resp.StatusCode)
664+
assert.Equal(t, 2, *attemptCount)
665+
})
666+
667+
t.Run("UI receives warn with correct payload", func(t *testing.T) {
668+
trackingUI := &trackingUserInterface{}
669+
sut, attemptCount := setupRetryMiddleware(t, &logger, trackingUI, 3, once429ThenOK)
670+
resp, err := sut.RoundTrip(httptest.NewRequest(http.MethodGet, "/", nil))
671+
672+
require.NoError(t, err)
673+
require.NotNil(t, resp)
674+
assert.Equal(t, http.StatusOK, resp.StatusCode)
675+
assert.Equal(t, 2, *attemptCount)
676+
677+
trackingUI.mu.Lock()
678+
defer trackingUI.mu.Unlock()
679+
680+
assert.NotEmpty(t, trackingUI.warnErrors, "Expected rate-limit wait warning via OutputError")
681+
for _, warnErr := range trackingUI.warnErrors {
682+
assert.Equal(t, "warn", warnErr.Level)
683+
assert.Equal(t, "Rate limited", warnErr.Title)
684+
assert.Contains(t, warnErr.Description, "Waiting")
685+
assert.Contains(t, warnErr.Description, "before retry")
686+
assert.Empty(t, warnErr.ErrorCode)
687+
assert.Zero(t, warnErr.StatusCode)
688+
}
689+
})
690+
691+
t.Run("500 exhaustion does not show rate-limit warning", func(t *testing.T) {
692+
trackingUI := &trackingUserInterface{}
693+
694+
always500 := func(req *http.Request, _ int) (*http.Response, error) {
695+
return &http.Response{
696+
StatusCode: http.StatusInternalServerError,
697+
Header: http.Header{},
698+
Request: req,
699+
}, nil
700+
}
701+
702+
sut, _ := setupRetryMiddleware(t, &logger, trackingUI, 2, always500)
703+
resp, err := sut.RoundTrip(httptest.NewRequest(http.MethodGet, "/", nil))
704+
705+
assert.NoError(t, err)
706+
assert.NotNil(t, resp)
707+
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
708+
709+
trackingUI.mu.Lock()
710+
defer trackingUI.mu.Unlock()
711+
assert.Empty(t, trackingUI.warnErrors, "500 retries should not produce rate-limit warnings")
712+
})
713+
}
714+
715+
type trackingUserInterface struct {
716+
mu sync.Mutex
717+
warnErrors []snyk_errors.Error
718+
}
719+
720+
func (t *trackingUserInterface) Output(string) error { return nil }
721+
func (t *trackingUserInterface) OutputError(err error, _ ...uitypes.Opts) error {
722+
var catalogErr snyk_errors.Error
723+
if errors.As(err, &catalogErr) {
724+
t.mu.Lock()
725+
t.warnErrors = append(t.warnErrors, catalogErr)
726+
t.mu.Unlock()
727+
}
728+
return nil
729+
}
730+
func (t *trackingUserInterface) Input(string) (string, error) { return "", nil }
731+
func (t *trackingUserInterface) SelectOptions(string, []string) (int, string, error) {
732+
return 0, "", nil
733+
}
734+
func (t *trackingUserInterface) NewProgressBar() uitypes.ProgressBar {
735+
return uitypes.EmptyProgressBar{}
736+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package middleware
2+
3+
import (
4+
"fmt"
5+
"math"
6+
"net/http"
7+
"time"
8+
9+
"github.com/snyk/error-catalog-golang-public/snyk_errors"
10+
)
11+
12+
func (rm RetryMiddleware) notifyRateLimit(duration time.Duration, statusCode int, attempt int, resolvedMax int) {
13+
if rm.ui == nil || statusCode != http.StatusTooManyRequests {
14+
return
15+
}
16+
rm.showRateLimitWaitWarning(duration, attempt, resolvedMax)
17+
}
18+
19+
func (rm RetryMiddleware) showRateLimitWaitWarning(duration time.Duration, attempt int, maxAttempts int) {
20+
if rm.ui == nil {
21+
return
22+
}
23+
24+
totalSecs := int(math.Ceil(duration.Seconds()))
25+
if totalSecs <= 0 {
26+
totalSecs = 1
27+
}
28+
29+
waitErr := snyk_errors.Error{
30+
Title: "Rate limited",
31+
Description: fmt.Sprintf("Waiting up to %ds before retry (attempt %d/%d).", totalSecs, attempt, maxAttempts),
32+
Level: "warn",
33+
}
34+
if err := rm.ui.OutputError(waitErr); err != nil {
35+
rm.logger.Debug().Err(err).Msg("failed to show rate-limit wait warning")
36+
}
37+
}

0 commit comments

Comments
 (0)