Make EcsCredentialProvider retry behavior configurable#3279
Make EcsCredentialProvider retry behavior configurable#3279kieranbrown wants to merge 2 commits intoaws:masterfrom
Conversation
The EKS Pod Identity Agent rate-limits the credentials endpoint with a token-bucket limiter and returns 429 when the bucket is empty. The previous retry check only matched ConnectException, so HTTP-level throttling responses bubbled up immediately and crashed the client. Treat a Guzzle BadResponseException whose status is 429 as retryable in addition to ConnectException, mirroring the explicit allowlist used by aws-sdk-go-v2's endpointcreds client. Other 4xx responses (e.g. 401) remain non-retryable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi @kieranbrown, Thanks for the contribution. We're hesitant to make this default behavior, only because HTTP/Container credential provider retry behavior is largely undefined across AWS SDKs. We'd be open to something like a configurable If you're open to implementing something like that, we'd accept changes as long as they come with sufficient test coverage. If not, feel free to open a feature request in our issues section and we could implement this when able. |
Add retryable_exceptions and retryable_error_codes options to the constructor config, replacing the hardcoded ConnectException + 429 check with a private isRetryable() helper that consults both lists. retryable_exceptions defaults to [ConnectException::class], preserving existing behavior. retryable_error_codes defaults to [], so HTTP 429 responses are no longer retried by default — callers running against the EKS Pod Identity Agent opt in via ['retryable_error_codes' => [429]]. Addresses review feedback on aws#3279 — the SDK has not previously specified retry behavior for HTTP/container credential providers, so making 429 retry default would set a precedent the maintainers want to avoid. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kieranbrown
force-pushed
the
retry-ecs-credentials-on-429
branch
from
f5e6fe6 to
9be2775
Compare
kieranbrown
changed the title
|
Hey @stobrien89, totally understandable, I've made the changes you've requested |
stobrien89
left a comment
stobrien89
left a comment
There was a problem hiding this comment.
Looks good- just a suggestion and question
| private $attempts; | ||
|
|
||
| /** @var string[] */ | ||
| private $retryableExceptions; |
There was a problem hiding this comment.
This should have a default value of [ConnectException::class] and any exceptions passed in retryable_exceptions should be added to it. Open to alternatives if your use case requires the current pattern
| } | ||
| } | ||
|
|
||
| if ($reason instanceof BadResponseException |
There was a problem hiding this comment.
Is RequestException too broad?
2 participants
Description of changes
Replaces the hardcoded retry condition in
EcsCredentialProviderwith two configurable allowlists, addressing review feedback that retry behavior for HTTP/container credential providers is largely undefined across AWS SDKs and shouldn't be expanded by default.Two new constructor options:
retryable_exceptions— array of exception class names. Defaults to[ConnectException::class], preserving existing behavior.retryable_error_codes— array of HTTP status codes to retry when returned in a GuzzleBadResponseException. Defaults to[].The retry decision now lives in a private
isRetryable()helper that consults both lists.Motivation
The EKS Pod Identity Agent's rate limiter returns HTTP 429 when its token bucket is empty. Previously these surfaced immediately as
CredentialsException. Callers in that environment can now opt in:This mirrors the explicit allowlist used by
aws-sdk-go-v2's endpointcreds client, without changing default behavior for any existing caller.Tests
Added coverage for: opt-in 429 retry succeeds, opt-in 429 retry exhausts attempts, custom exception class added via config, custom config replacing defaults, 429 not retried by default. Existing
ConnectExceptionretry and 401 non-retryable cases unchanged.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Generated by AI tools (Claude), and reviewed by Kieran Brown.