From c7b59007eb9d7b7a992982bb9d90815f44824bbe Mon Sep 17 00:00:00 2001 From: samzong Date: Fri, 26 Jun 2026 11:53:05 -0400 Subject: [PATCH] fix(runtime): skip default retries for unsafe methods Signed-off-by: samzong --- pkg/runtime/client.go | 5 +++-- pkg/runtime/retry.go | 21 +++++++++++++++------ pkg/runtime/retry_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/pkg/runtime/client.go b/pkg/runtime/client.go index ccb24e5..00ecdb9 100644 --- a/pkg/runtime/client.go +++ b/pkg/runtime/client.go @@ -55,11 +55,12 @@ func HTTPClient(opts ClientOptions) *http.Client { transport = &http.Transport{TLSClientConfig: tlsCfg} } maxRetries := opts.MaxRetries - if maxRetries == 0 { + safeMethodsOnly := maxRetries == 0 + if safeMethodsOnly { maxRetries = 3 } if maxRetries > 0 { - transport = &retryTransport{inner: transport, maxRetries: maxRetries, debug: opts.Debug} + transport = &retryTransport{inner: transport, maxRetries: maxRetries, debug: opts.Debug, safeMethodsOnly: safeMethodsOnly} } if opts.Debug { transport = &debugTransport{inner: transport} diff --git a/pkg/runtime/retry.go b/pkg/runtime/retry.go index 7070e2b..0151ccd 100644 --- a/pkg/runtime/retry.go +++ b/pkg/runtime/retry.go @@ -10,10 +10,11 @@ import ( ) type retryTransport struct { - inner http.RoundTripper - maxRetries int - sleepFn func(time.Duration) - debug bool + inner http.RoundTripper + maxRetries int + sleepFn func(time.Duration) + debug bool + safeMethodsOnly bool } func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) { @@ -48,7 +49,7 @@ func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) { if err != nil { return nil, err } - if !isRetryable(resp.StatusCode) { + if !isRetryable(req.Method, resp.StatusCode, t.safeMethodsOnly) { return resp, nil } if attempt < t.maxRetries { @@ -58,9 +59,17 @@ func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } -func isRetryable(status int) bool { +func isRetryable(method string, status int, safeMethodsOnly bool) bool { switch status { case 429, 500, 502, 503, 504: + default: + return false + } + if !safeMethodsOnly { + return true + } + switch method { + case http.MethodGet, http.MethodHead, http.MethodOptions: return true } return false diff --git a/pkg/runtime/retry_test.go b/pkg/runtime/retry_test.go index 1fe8c7e..85540ef 100644 --- a/pkg/runtime/retry_test.go +++ b/pkg/runtime/retry_test.go @@ -75,6 +75,32 @@ func TestRetryTransport_RetriesOn5xx(t *testing.T) { } } +func TestHTTPClient_DefaultRetriesSkipPost(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if atomic.AddInt32(&calls, 1) > 1 { + w.WriteHeader(200) + return + } + w.WriteHeader(500) + })) + defer srv.Close() + + req, _ := http.NewRequestWithContext(context.Background(), "POST", srv.URL, strings.NewReader(`{"key":"val"}`)) + resp, err := HTTPClient(ClientOptions{Timeout: 3 * time.Second}).Do(req) + if err != nil { + t.Fatalf("Do: %v", err) + } + resp.Body.Close() + + if resp.StatusCode != 500 { + t.Fatalf("status = %d, want 500", resp.StatusCode) + } + if got := atomic.LoadInt32(&calls); got != 1 { + t.Fatalf("calls = %d, want 1", got) + } +} + func TestRetryTransport_NoRetryOn4xx(t *testing.T) { var calls int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {