Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions src/Credentials/EcsCredentialProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use Aws\Arn\Arn;
use Aws\Exception\CredentialsException;
use GuzzleHttp\Exception\BadResponseException;
use GuzzleHttp\Exception\ConnectException;
use GuzzleHttp\Exception\GuzzleException;
use GuzzleHttp\Psr7\Request;
Expand Down Expand Up @@ -41,11 +42,22 @@ class EcsCredentialProvider
/** @var int */
private $attempts;

/** @var string[] */
private $retryableExceptions;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


/** @var int[] */
private $retryableErrorCodes;

/**
* The constructor accepts following options:
* - timeout: (optional) Connection timeout, in seconds, default 1.0
* - retries: Optional number of retries to be attempted, default 3.
* - client: An EcsClient to make request from
* - retryable_exceptions: Optional array of exception class names that
* should be retried. Defaults to [ConnectException::class].
* - retryable_error_codes: Optional array of HTTP status codes that
* should be retried when returned in a BadResponseException. Defaults
* to an empty array.
*
* @param array $config Configuration options
*/
Expand All @@ -59,6 +71,9 @@ public function __construct(array $config = [])
: ((int) getenv(self::ENV_RETRIES) ?: self::DEFAULT_ENV_RETRIES);

$this->client = $config['client'] ?? \Aws\default_http_handler();
$this->retryableExceptions = $config['retryable_exceptions']
?? [ConnectException::class];
$this->retryableErrorCodes = $config['retryable_error_codes'] ?? [];
}

/**
Expand Down Expand Up @@ -107,8 +122,7 @@ public function __invoke()
})->otherwise(function ($reason) {
$reason = is_array($reason) ? $reason['exception'] : $reason;

$isRetryable = $reason instanceof ConnectException;
if ($isRetryable && ($this->attempts < $this->retries)) {
if ($this->isRetryable($reason) && ($this->attempts < $this->retries)) {
sleep((int)pow(1.2, $this->attempts));
} else {
$msg = $reason->getMessage();
Expand Down Expand Up @@ -221,6 +235,31 @@ private function getEcsUri()
return self::SERVER_URI . $credsUri;
}

/**
* Determines whether a failed request should be retried, based on the
* configured retryable_exceptions and retryable_error_codes.
*/
private function isRetryable($reason): bool
{
foreach ($this->retryableExceptions as $exceptionClass) {
if ($reason instanceof $exceptionClass) {
return true;
}
}

if ($reason instanceof BadResponseException
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is RequestException too broad?

&& in_array(
$reason->getResponse()->getStatusCode(),
$this->retryableErrorCodes,
true
)
) {
return true;
}

return false;
}

private function decodeResult($response)
{
$result = json_decode($response, true);
Expand Down
183 changes: 183 additions & 0 deletions tests/Credentials/EcsCredentialProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Aws\Exception\CredentialsException;
use Aws\Handler\Guzzle\GuzzleHandler;
use GuzzleHttp\Client;
use GuzzleHttp\Exception\BadResponseException;
use GuzzleHttp\Exception\ConnectException;
use GuzzleHttp\Exception\GuzzleException;
use GuzzleHttp\Exception\RequestException;
Expand Down Expand Up @@ -414,6 +415,15 @@ public static function successDataProvider(): array
'exception' => $connectException,
]);

$tooManyRequestsException = new BadResponseException(
'429 Too Many Requests',
new Psr7\Request('GET', '/latest'),
new Psr7\Response(429)
);
$rejectionTooManyRequests = Promise\Create::rejectionFor([
'exception' => $tooManyRequestsException,
]);

$promiseCreds = Promise\Create::promiseFor(
new Response(200, [], Psr7\Utils::streamFor(
json_encode(call_user_func_array(
Expand Down Expand Up @@ -456,6 +466,134 @@ public static function successDataProvider(): array
];
}

public function testRetriesOptedInErrorCode()
{
$expiry = time() + 1000;
$creds = ['foo_key', 'baz_secret', 'qux_token', "@{$expiry}"];

$rejectionTooManyRequests = Promise\Create::rejectionFor([
'exception' => new BadResponseException(
'429 Too Many Requests',
new Psr7\Request('GET', '/latest'),
new Psr7\Response(429)
),
]);
$promiseCreds = Promise\Create::promiseFor(
new Response(200, [], Psr7\Utils::streamFor(
json_encode(call_user_func_array(
[self::class, 'getCredentialArray'],
$creds
)))
)
);

$provider = new EcsCredentialProvider([
'client' => $this->getTestClient([
$rejectionTooManyRequests,
$promiseCreds,
], $creds),
'retries' => 2,
'retryable_error_codes' => [429],
]);

$credentials = $provider()->wait();
$this->assertSame('foo_key', $credentials->getAccessKeyId());
$this->assertSame('baz_secret', $credentials->getSecretKey());
}

public function testDoesNotRetry429ByDefault()
{
$rejectionTooManyRequests = Promise\Create::rejectionFor([
'exception' => new BadResponseException(
'429 Too Many Requests',
new Psr7\Request('GET', '/latest'),
new Psr7\Response(429)
),
]);

$provider = new EcsCredentialProvider([
'client' => $this->getTestClient([
$rejectionTooManyRequests,
]),
'retries' => 3,
]);

try {
$provider()->wait();
$this->fail('Provider should have thrown an exception.');
} catch (CredentialsException $e) {
$this->assertStringContainsString(
'attempt 0/3',
$e->getMessage()
);
$this->assertStringContainsString('429 Too Many Requests', $e->getMessage());
}

$this->assertSame(0, $provider->getAttempts());
}

public function testRetriesOptedInExceptionClass()
{
$expiry = time() + 1000;
$creds = ['foo_key', 'baz_secret', 'qux_token', "@{$expiry}"];

$rejectionRequest = Promise\Create::rejectionFor([
'exception' => new RequestException(
'Boom',
new Psr7\Request('GET', '/latest'),
new Psr7\Response(500)
),
]);
$promiseCreds = Promise\Create::promiseFor(
new Response(200, [], Psr7\Utils::streamFor(
json_encode(call_user_func_array(
[self::class, 'getCredentialArray'],
$creds
)))
)
);

$provider = new EcsCredentialProvider([
'client' => $this->getTestClient([
$rejectionRequest,
$promiseCreds,
], $creds),
'retries' => 2,
'retryable_exceptions' => [
ConnectException::class,
RequestException::class,
],
]);

$credentials = $provider()->wait();
$this->assertSame('foo_key', $credentials->getAccessKeyId());
}

public function testCustomRetryableExceptionsReplaceDefaults()
{
$rejectionConnection = Promise\Create::rejectionFor([
'exception' => new ConnectException(
'cURL error 28: Connection timed out after 1000 milliseconds',
new Psr7\Request('GET', '/latest')
),
]);

$provider = new EcsCredentialProvider([
'client' => $this->getTestClient([
$rejectionConnection,
]),
'retries' => 3,
'retryable_exceptions' => [],
]);

try {
$provider()->wait();
$this->fail('Provider should have thrown an exception.');
} catch (CredentialsException $e) {
$this->assertSame(0, $provider->getAttempts());
}
}

/**
* @param $client
* @param \Exception $expected
Expand Down Expand Up @@ -501,6 +639,13 @@ public static function failureDataProvider(): array
$rejectionConnection = Promise\Create::rejectionFor([
'exception' => $connectException,
]);
$rejectionTooManyRequests = Promise\Create::rejectionFor([
'exception' => new BadResponseException(
'429 Too Many Requests',
$getRequest,
new Psr7\Response(429)
)
]);

return [
'Non-retryable error' => [
Expand All @@ -520,9 +665,47 @@ public static function failureDataProvider(): array
'Error retrieving credentials from container metadata after attempt 1/1 (cURL error 28: Connection timed out after 1000 milliseconds)'
)
],
'Non-retryable HTTP 429 by default' => [
[
$rejectionTooManyRequests,
],
new CredentialsException(
'Error retrieving credentials from container metadata after attempt 0/1 (429 Too Many Requests)'
)
],
];
}

public function testOptedInHTTP429RetryExhaustsAttempts()
{
$rejectionTooManyRequests = Promise\Create::rejectionFor([
'exception' => new BadResponseException(
'429 Too Many Requests',
new Psr7\Request('GET', '/latest'),
new Psr7\Response(429)
),
]);

$provider = new EcsCredentialProvider([
'client' => $this->getTestClient([
$rejectionTooManyRequests,
$rejectionTooManyRequests,
]),
'retries' => 1,
'retryable_error_codes' => [429],
]);

try {
$provider()->wait();
$this->fail('Provider should have thrown an exception.');
} catch (CredentialsException $e) {
$this->assertSame(
'Error retrieving credentials from container metadata after attempt 1/1 (429 Too Many Requests)',
$e->getMessage()
);
}
}

public function testReadsRetriesFromEnvironment()
{
putenv('AWS_METADATA_SERVICE_NUM_ATTEMPTS=1');
Expand Down