retry InvalidIdentityToken errors #1858
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1858 +/- ##
============================================
- Coverage 93.77% 93.75% -0.03%
- Complexity 3278 3282 +4
============================================
Files 184 184
Lines 8713 8652 -61
============================================
- Hits 8171 8112 -59
+ Misses 542 540 -2
Continue to review full report at Codecov.
|
src/RetryMiddleware.php
Outdated
| @@ -31,6 +31,7 @@ class RetryMiddleware | |||
| 'RequestThrottledException' => true, | |||
| 'TooManyRequestsException' => true, | |||
| 'IDPCommunicationError' => true, | |||
| 'InvalidIdentityToken' => true, | |||
There was a problem hiding this comment.
Do we want this error code to be retried for all services?
There was a problem hiding this comment.
AssumeRoleWithSAML also returns InvalidIdentityToken. Other services shouldn't return this error.
There was a problem hiding this comment.
Ideally retrying InvalidIdentityToken would only be limited in scope to STS (as it's a special use case), as it's not inherently a throttling or retryable error, and we should allow for the possibility of other services using that exception name at some point.
| } catch (\Exception $e) { | ||
| if ($e->getAwsErrorCode() == 'InvalidIdentityToken') { | ||
| if ($this->attempts < $this->retries) { | ||
| sleep(pow(1.2, $this->attempts)); |
There was a problem hiding this comment.
Is this retry strategy enough for the cold start scenario we're covering for?
There was a problem hiding this comment.
A single retry may not succeed when encountering cold starts. Bumping the default retries to 3 should be sufficient for that scenario.
Retry InvalidIdentityToken errors. Used by AssumeRoleWithWebIdentityCredentialProvider when StsClient makes call to asssume role.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.