client: surface last-response details after retries are exhausted

pkg/client/rekor_client.go

@@ -15,7 +15,10 @@
 package client
 
 import (
+	"bytes"
 	"crypto/tls"
+	"fmt"
+	"io"
 	"net/http"
 	"net/url"
 
@@ -29,6 +32,36 @@ import (
 	"github.com/sigstore/rekor/pkg/util"
 )
 
+// maxErrorBodyBytes caps how much of the final response body we embed in
+// the error message to avoid flooding terminals with large payloads.
+const maxErrorBodyBytes = 512
+
+// retryErrorHandler makes the final error surfaced after retries include the
+// underlying cause (transport error or final response status + body snippet).
+// Without a custom handler retryablehttp's default message is just
+// "<METHOD> <URL> giving up after N attempt(s)", which hides the actual
+// reason the retries failed — especially when the server returned an error
+// response (5xx) rather than a transport error. See
+// https://github.com/sigstore/rekor/issues/2640.
+func retryErrorHandler(resp *http.Response, err error, numTries int) (*http.Response, error) {
+	if err != nil {
+		return nil, fmt.Errorf("giving up after %d attempt(s): %w", numTries, err)
+	}
+	if resp != nil {
+		defer resp.Body.Close()
+		body, readErr := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+		snippet := string(bytes.TrimSpace(body))
+		if readErr == nil && snippet != "" {
+			return nil, fmt.Errorf("giving up after %d attempt(s): status %d: %s",
+				numTries, resp.StatusCode, snippet)
+		}
+		return nil, fmt.Errorf("giving up after %d attempt(s): status %d",
+			numTries, resp.StatusCode)
+	}
+
+	return nil, fmt.Errorf("giving up after %d attempt(s)", numTries)
+}
+
 func GetRekorClient(rekorServerURL string, opts ...Option) (*client.Rekor, error) {
 	url, err := url.Parse(rekorServerURL)
 	if err != nil {
@@ -54,6 +87,7 @@ func GetRekorClient(rekorServerURL string, opts ...Option) (*client.Rekor, error
 	retryableClient.RetryWaitMin = o.RetryWaitMin
 	retryableClient.RetryWaitMax = o.RetryWaitMax
 	retryableClient.Logger = o.Logger
+	retryableClient.ErrorHandler = retryErrorHandler
 
 	httpClient := retryableClient.StandardClient()
 	httpClient.Transport = createRoundTripper(httpClient.Transport, o)

pkg/client/rekor_client_test.go

@@ -131,3 +131,32 @@ func TestRekorLeakedGoroutine_SearchByHash(t *testing.T) {
 	rekor, _ := GetRekorClient(testServer.URL, WithInsecureTLS(true))
 	rekor.Index.SearchIndex(index.NewSearchIndexParams())
 }
+
+func TestRetryErrorHandlerSurfacesServerResponse(t *testing.T) {
+	// When retries exhaust against a server that keeps returning 5xx,
+	// the final error must include the status code and body — not just
+	// "giving up after N attempt(s)". See sigstore/rekor#2640.
+	testServer := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+			_, _ = w.Write([]byte("database unavailable"))
+		}))
+	defer testServer.Close()
+
+	client, err := GetRekorClient(testServer.URL,
+		WithRetryCount(1), WithRetryWaitMin(1*time.Millisecond), WithRetryWaitMax(2*time.Millisecond))
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = client.Tlog.GetLogInfo(nil)
+	if err == nil {
+		t.Fatal("expected an error after retries were exhausted")
+	}
+	msg := err.Error()
+	if !strings.Contains(msg, "status 500") {
+		t.Errorf("expected error to include 'status 500', got: %s", msg)
+	}
+	if !strings.Contains(msg, "database unavailable") {
+		t.Errorf("expected error to include server body 'database unavailable', got: %s", msg)
+	}
+}