-
Notifications
You must be signed in to change notification settings - Fork 598
Description
Description
HTTP 429 responses are not retried by default in the retry transport, even though TooManyRequestsErrorCode is included in temporaryErrorCodes. The OCI Distribution Spec defines 429 as a standard error response: https://github.com/distribution/distribution/blob/da404778edd3faa665e48ca3bb791b6144f3355e/docs/content/spec/api.md#base
Current Behavior
-
The
defaultRetryStatusCodes(408, 500, 502, 503, 504, 499, 522) does not include 429:
go-containerregistry/pkg/v1/remote/options.go
Lines 93 to 101 in 59a4b85
var defaultRetryStatusCodes = []int{ http.StatusRequestTimeout, http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout, 499, // nginx-specific, client closed request 522, // Cloudflare-specific, connection timeout } -
TooManyRequestsErrorCodeis included intemporaryErrorCodes:
go-containerregistry/pkg/v1/remote/transport/error.go
Lines 140 to 145 in 59a4b85
var temporaryErrorCodes = map[ErrorCode]struct{}{ BlobUploadInvalidErrorCode: {}, TooManyRequestsErrorCode: {}, UnknownErrorCode: {}, UnavailableErrorCode: {}, } -
The retry logic in
retryTransport.RoundTriponly converts responses totransport.Errorif the status code is int.codes:
go-containerregistry/pkg/v1/remote/transport/retry.go
Lines 101 to 105 in 59a4b85
for _, code := range t.codes { if out.StatusCode == code { return retryError(out) } }
Problem
Since 429 is not in defaultRetryStatusCodes, the response is never converted to a transport.Error via retryError(). This means:
- The
Temporary()method is never called - The check for
TooManyRequestsErrorCodeintemporaryErrorCodesis never reached from retryTransport. - 429 responses are not retried
Question
Is this behavior intentional? It seems reasonable to retry rate-limited requests with appropriate backoff.
If this is intentional (perhaps because rate limiting requires different handling than other temporary errors), should users be expected to explicitly configure retry behavior for rate limiting using WithRetryStatusCodes()?
Please let me know if I'm missing something in my understanding of the retry logic.