diff --git a/.changes/nextrelease/bugfix-retries.json b/.changes/nextrelease/bugfix-retries.json new file mode 100644 index 0000000000..7645a9243e --- /dev/null +++ b/.changes/nextrelease/bugfix-retries.json @@ -0,0 +1,7 @@ +[ + { + "type": "enhancement", + "category": "Retries", + "description": "Updated the opt-in retry path (AWS_NEW_RETRIES_2026) to emit SEP-compliant retry headers, honor AWS_MAX_ATTEMPTS and shared-config max_attempts, and align long-polling retry behavior with the new retry spec." + } +] \ No newline at end of file diff --git a/src/Middleware.php b/src/Middleware.php index 13a19d6b92..76006abf14 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -265,7 +265,7 @@ public static function invocationId() RequestInterface $request ) use ($handler){ return $handler($command, $request->withHeader( - 'aws-sdk-invocation-id', + 'amz-sdk-invocation-id', md5(uniqid(gethostname(), true)) )); }; diff --git a/src/Retry/ConfigurationProvider.php b/src/Retry/ConfigurationProvider.php index 5194dcdfd8..a631906286 100644 --- a/src/Retry/ConfigurationProvider.php +++ b/src/Retry/ConfigurationProvider.php @@ -108,17 +108,21 @@ public static function env() return function () { // Use config from environment variables, if available $mode = getenv(self::ENV_MODE); - $maxAttempts = getenv(self::ENV_MAX_ATTEMPTS) - ? getenv(self::ENV_MAX_ATTEMPTS) - : self::DEFAULT_MAX_ATTEMPTS; - if (!empty($mode)) { + $maxAttempts = getenv(self::ENV_MAX_ATTEMPTS); + $hasMode = $mode !== false && $mode !== ''; + $hasMaxAttempts = $maxAttempts !== false && $maxAttempts !== ''; + + if ($hasMode || $hasMaxAttempts) { return Promise\Create::promiseFor( - new Configuration($mode, $maxAttempts) - ); + new Configuration( + $hasMode ? $mode : self::getDefaultMode(), + $hasMaxAttempts ? $maxAttempts : self::DEFAULT_MAX_ATTEMPTS + )); } return self::reject('Could not find environment variable config' - . ' in ' . self::ENV_MODE); + . ' in ' . self::ENV_MODE + . ' or ' . self::ENV_MAX_ATTEMPTS); }; } @@ -175,18 +179,27 @@ public static function ini( if (!isset($data[$profile])) { return self::reject("'$profile' not found in config file"); } - if (!isset($data[$profile][self::INI_MODE])) { + $hasMode = isset($data[$profile][self::INI_MODE]) + && $data[$profile][self::INI_MODE] !== ''; + $hasMaxAttempts = array_key_exists( + self::INI_MAX_ATTEMPTS, + $data[$profile] + ) && $data[$profile][self::INI_MAX_ATTEMPTS] !== ''; + + if (!$hasMode && !$hasMaxAttempts) { return self::reject("Required retry config values not present in INI profile '{$profile}' ({$filename})"); } - $maxAttempts = isset($data[$profile][self::INI_MAX_ATTEMPTS]) + $maxAttempts = $hasMaxAttempts ? $data[$profile][self::INI_MAX_ATTEMPTS] : self::DEFAULT_MAX_ATTEMPTS; return Promise\Create::promiseFor( new Configuration( - $data[$profile][self::INI_MODE], + $hasMode + ? $data[$profile][self::INI_MODE] + : self::getDefaultMode(), $maxAttempts ) ); diff --git a/src/Retry/RetryHelperTrait.php b/src/Retry/RetryHelperTrait.php index a7edb72b59..5e174bda0a 100644 --- a/src/Retry/RetryHelperTrait.php +++ b/src/Retry/RetryHelperTrait.php @@ -6,9 +6,14 @@ trait RetryHelperTrait { - private function addRetryHeader($request, $retries, $delayBy) + private function addRetryHeader($request, $retries, $maxAttempts = null) { - return $request->withHeader('aws-sdk-retry', "{$retries}/{$delayBy}"); + $header = ['attempt=' . ($retries + 1)]; + if ($maxAttempts !== null) { + $header[] = 'max=' . $maxAttempts; + } + + return $request->withHeader('amz-sdk-request', implode('; ', $header)); } diff --git a/src/Retry/V3/RetryMiddleware.php b/src/Retry/V3/RetryMiddleware.php index e3b9de208f..4d8bd0a39e 100644 --- a/src/Retry/V3/RetryMiddleware.php +++ b/src/Retry/V3/RetryMiddleware.php @@ -75,6 +75,16 @@ class RetryMiddleware public static function wrap(ConfigurationInterface $config, array $options): \Closure { + if (!isset($options['quota_manager'])) { + $options['quota_manager'] = new QuotaManager(); + } + + if ($config->getMode() === 'adaptive' + && !isset($options['rate_limiter']) + ) { + $options['rate_limiter'] = new RateLimiter(); + } + return function (callable $handler) use ($config, $options) { return new static($config, $handler, $options); }; @@ -134,7 +144,7 @@ public function __invoke(CommandInterface $cmd, RequestInterface $req): PromiseI $requestStats = []; $capacityUsed = null; - $req = $this->addRetryHeader($req, 0, 0); + $req = $this->addRetryHeader($req, 0, $this->resolveMaxAttempts($cmd)); $callback = function ($value) use ( $handler, @@ -200,9 +210,7 @@ public function __invoke(CommandInterface $cmd, RequestInterface $req): PromiseI } // Max attempts is checked before retry quota. - $maxAttempts = ($cmd['@retries'] !== null) - ? $cmd['@retries'] + 1 - : $this->maxAttempts; + $maxAttempts = $this->resolveMaxAttempts($cmd); if ($attempts >= $maxAttempts) { if ($value instanceof AwsException) { @@ -256,7 +264,11 @@ public function __invoke(CommandInterface $cmd, RequestInterface $req): PromiseI $this->updateStats($attempts - 1, $delayByMs, $requestStats); } - $req = $this->addRetryHeader($req, $attempts - 1, $delayByMs); + $req = $this->addRetryHeader( + $req, + $attempts - 1, + $maxAttempts + ); if ($this->mode === 'adaptive') { $this->rateLimiter->getSendToken(); @@ -415,4 +427,11 @@ private function isThrottlingError(mixed $result): bool return false; } + + private function resolveMaxAttempts(CommandInterface $cmd): int + { + return ($cmd['@retries'] !== null) + ? $cmd['@retries'] + 1 + : $this->maxAttempts; + } } diff --git a/src/RetryMiddleware.php b/src/RetryMiddleware.php index 702402061f..ad595e240b 100644 --- a/src/RetryMiddleware.php +++ b/src/RetryMiddleware.php @@ -190,7 +190,11 @@ public function __invoke( $decider = $this->decider; $delay = $this->delay; - $request = $this->addRetryHeader($request, 0, 0); + $request = $this->addRetryHeader( + $request, + 0, + $command['@retries'] !== null ? $command['@retries'] + 1 : null + ); $g = function ($value) use ( $handler, @@ -234,7 +238,11 @@ public function __invoke( } // Update retry header with retry count and delayBy - $request = $this->addRetryHeader($request, $retries, $delayBy); + $request = $this->addRetryHeader( + $request, + $retries, + $command['@retries'] !== null ? $command['@retries'] + 1 : null + ); return $handler($command, $request)->then($g, $g); }; diff --git a/src/RetryMiddlewareV2.php b/src/RetryMiddlewareV2.php index c3febe2d9e..f27639da11 100644 --- a/src/RetryMiddlewareV2.php +++ b/src/RetryMiddlewareV2.php @@ -168,7 +168,7 @@ public function __invoke(CommandInterface $cmd, RequestInterface $req) $monitoringEvents = []; $requestStats = []; - $req = $this->addRetryHeader($req, 0, 0); + $req = $this->addRetryHeader($req, 0, $this->resolveMaxAttempts($cmd)); $callback = function ($value) use ( $handler, @@ -215,7 +215,11 @@ public function __invoke(CommandInterface $cmd, RequestInterface $req) } // Update retry header with retry count and delayBy - $req = $this->addRetryHeader($req, $attempts - 1, $delayBy); + $req = $this->addRetryHeader( + $req, + $attempts - 1, + $this->resolveMaxAttempts($cmd) + ); // Get token from rate limiter, which will sleep if necessary if ($this->mode === 'adaptive') { @@ -352,4 +356,11 @@ private function isThrottlingError($result) return false; } + + private function resolveMaxAttempts(CommandInterface $cmd) + { + return $cmd['@retries'] !== null + ? $cmd['@retries'] + 1 + : $this->maxAttempts; + } } diff --git a/src/Signature/SignatureV4.php b/src/Signature/SignatureV4.php index 94ec7416e8..7e13c81d7b 100644 --- a/src/Signature/SignatureV4.php +++ b/src/Signature/SignatureV4.php @@ -66,8 +66,8 @@ protected function getHeaderBlacklist() 'referer' => true, 'user-agent' => true, 'x-amzn-trace-id' => true, - 'aws-sdk-invocation-id' => true, - 'aws-sdk-retry' => true, + 'amz-sdk-invocation-id' => true, + 'amz-sdk-request' => true, ]; } @@ -498,8 +498,8 @@ private function removeIllegalV4aHeaders(&$request) { static $illegalV4aHeaders = [ self::AMZ_CONTENT_SHA256_HEADER, - 'aws-sdk-invocation-id', - 'aws-sdk-retry', + 'amz-sdk-invocation-id', + 'amz-sdk-request', 'x-amz-region-set', 'transfer-encoding', ]; diff --git a/tests/MiddlewareTest.php b/tests/MiddlewareTest.php index 7b3a3da995..9685e4b0ef 100644 --- a/tests/MiddlewareTest.php +++ b/tests/MiddlewareTest.php @@ -79,7 +79,7 @@ public function testAddInvocationId() { $list = new HandlerList(); $mock = function ($command, $request) { - $this->assertTrue($request->hasHeader('aws-sdk-invocation-id')); + $this->assertTrue($request->hasHeader('amz-sdk-invocation-id')); return Promise\Create::promiseFor( new Result(['@metadata' => ['statusCode' => 200]]) ); diff --git a/tests/Retry/ConfigurationProviderTest.php b/tests/Retry/ConfigurationProviderTest.php index 939e2b1c36..532acc7856 100644 --- a/tests/Retry/ConfigurationProviderTest.php +++ b/tests/Retry/ConfigurationProviderTest.php @@ -92,6 +92,17 @@ public function testCreatesFromEnvironmentVariables() $this->assertSame($expected->toArray(), $result->toArray()); } + public function testCreatesFromEnvironmentMaxAttemptsUsingDefaultMode() + { + $this->clearEnv(); + putenv(OptIn::ENV . '=true'); + OptIn::reset(); + $expected = new Configuration('standard', 17); + putenv(ConfigurationProvider::ENV_MAX_ATTEMPTS . '=17'); + $result = call_user_func(ConfigurationProvider::env())->wait(); + $this->assertSame($expected->toArray(), $result->toArray()); + } + public function testRejectsOnNoEnvironmentVars() { $this->clearEnv(); @@ -202,6 +213,19 @@ public function testCreatesFromIniFileWithSpecifiedProfile() unlink($dir . '/config'); } + public function testCreatesFromIniFileWithMaxAttemptsOnlyUsingDefaultMode() + { + $dir = $this->clearEnv(); + putenv(OptIn::ENV . '=true'); + OptIn::reset(); + $expected = new Configuration('standard', 7); + file_put_contents($dir . '/config', "[default]\nmax_attempts = 7\n"); + putenv('HOME=' . dirname($dir)); + $result = call_user_func(ConfigurationProvider::ini(null, null))->wait(); + $this->assertSame($expected->toArray(), $result->toArray()); + unlink($dir . '/config'); + } + public function testEnsuresIniFileExists() { $this->expectException(\Aws\Retry\Exception\ConfigurationException::class); diff --git a/tests/Retry/V3/RetryMiddlewareTest.php b/tests/Retry/V3/RetryMiddlewareTest.php index c6579821f5..66e9360d5d 100644 --- a/tests/Retry/V3/RetryMiddlewareTest.php +++ b/tests/Retry/V3/RetryMiddlewareTest.php @@ -12,6 +12,7 @@ use Aws\Retry\Configuration; use Aws\Retry\ConfigurationProvider; use Aws\Retry\V3\QuotaManager; +use Aws\Retry\V3\OptIn; use Aws\Retry\V3\RetryMiddleware; use Aws\Retry\RateLimiter; use GuzzleHttp\Promise\RejectedPromise; @@ -28,6 +29,23 @@ class RetryMiddlewareTest extends TestCase { use UsesServiceTrait; + private string $previousOptIn; + + protected function setUp(): void + { + parent::setUp(); + $this->previousOptIn = getenv(OptIn::ENV) ?: ''; + putenv(OptIn::ENV . '='); + OptIn::reset(); + } + + protected function tearDown(): void + { + putenv(OptIn::ENV . '=' . $this->previousOptIn); + OptIn::reset(); + parent::tearDown(); + } + /** * @param CommandInterface $command * @param array $queue @@ -711,8 +729,10 @@ public function testRetriesForAdapativeMode() public function testAddRetryHeader() { - $nextHandler = function (CommandInterface $command, RequestInterface $request) { - $this->assertTrue($request->hasHeader('aws-sdk-retry')); + $observedHeaders = []; + $nextHandler = function (CommandInterface $command, RequestInterface $request) use (&$observedHeaders) { + $this->assertTrue($request->hasHeader('amz-sdk-request')); + $observedHeaders[] = $request->getHeaderLine('amz-sdk-request'); return new RejectedPromise( new AwsException('e', $command, ['connection_error' => true]) ); @@ -729,6 +749,17 @@ public function testAddRetryHeader() $retryMW(new Command('SomeCommand'), new Request('GET', ''))->wait(); $this->fail(); } catch (AwsException $e) { } + + $this->assertSame( + [ + 'attempt=1; max=5', + 'attempt=2; max=5', + 'attempt=3; max=5', + 'attempt=4; max=5', + 'attempt=5; max=5', + ], + $observedHeaders + ); } public function testDeciderRetriesWhenStatusCodeMatches() @@ -1534,6 +1565,48 @@ public static function longPollingOperationsProvider(): array ]; } + public function testWrapSharesQuotaManagerState() + { + $this->enableOptInFlag(); + $factory = RetryMiddleware::wrap( + new Configuration('standard', 3), + [] + ); + + $first = $factory(static fn ($cmd, $req) => null); + $second = $factory(static fn ($cmd, $req) => null); + + $this->assertInstanceOf( + QuotaManager::class, + $this->getPrivateProperty($first, 'quotaManager') + ); + $this->assertSame( + $this->getPrivateProperty($first, 'quotaManager'), + $this->getPrivateProperty($second, 'quotaManager') + ); + } + + public function testWrapSharesAdaptiveRateLimiterState() + { + $this->enableOptInFlag(); + $factory = RetryMiddleware::wrap( + new Configuration('adaptive', 3), + [] + ); + + $first = $factory(static fn ($cmd, $req) => null); + $second = $factory(static fn ($cmd, $req) => null); + + $this->assertInstanceOf( + RateLimiter::class, + $this->getPrivateProperty($first, 'rateLimiter') + ); + $this->assertSame( + $this->getPrivateProperty($first, 'rateLimiter'), + $this->getPrivateProperty($second, 'rateLimiter') + ); + } + public function testRetryAfterHeaderClamping() { $command = new Command('foo'); @@ -1640,4 +1713,19 @@ public function testRetryAfterHeaderMinClamp() // retry-after=0 clamped to [50, 5050] = 50ms (the computed delay minimum) $this->assertSame(50, $delays[0]); } + + private function enableOptInFlag(): void + { + putenv(OptIn::ENV . '=true'); + OptIn::reset(); + } + + private function getPrivateProperty(object $object, string $property): mixed + { + $reflection = new \ReflectionClass($object); + $prop = $reflection->getProperty($property); + $prop->setAccessible(true); + + return $prop->getValue($object); + } } diff --git a/tests/RetryMiddlewareTest.php b/tests/RetryMiddlewareTest.php index 644be48dc0..fe42684ae6 100644 --- a/tests/RetryMiddlewareTest.php +++ b/tests/RetryMiddlewareTest.php @@ -22,8 +22,10 @@ class RetryMiddlewareTest extends TestCase { public function testAddRetryHeader() { - $nextHandler = function (CommandInterface $command, RequestInterface $request) { - $this->assertTrue($request->hasHeader('aws-sdk-retry')); + $observedHeaders = []; + $nextHandler = function (CommandInterface $command, RequestInterface $request) use (&$observedHeaders) { + $this->assertTrue($request->hasHeader('amz-sdk-request')); + $observedHeaders[] = $request->getHeaderLine('amz-sdk-request'); return new RejectedPromise( new AwsException('e', $command, ['connection_error' => true]) ); @@ -39,6 +41,11 @@ public function testAddRetryHeader() $retryMW(new Command('SomeCommand'), new Request('GET', ''))->wait(); $this->fail(); } catch (AwsException $e) { } + + $this->assertSame( + ['attempt=1', 'attempt=2', 'attempt=3', 'attempt=4'], + $observedHeaders + ); } public function testDeciderRetriesWhenStatusCodeMatches() diff --git a/tests/RetryMiddlewareV2Test.php b/tests/RetryMiddlewareV2Test.php index 48ff803c84..9ae5e718e7 100644 --- a/tests/RetryMiddlewareV2Test.php +++ b/tests/RetryMiddlewareV2Test.php @@ -455,8 +455,10 @@ public function testRetriesForAdapativeMode() public function testAddRetryHeader() { - $nextHandler = function (CommandInterface $command, RequestInterface $request) { - $this->assertTrue($request->hasHeader('aws-sdk-retry')); + $observedHeaders = []; + $nextHandler = function (CommandInterface $command, RequestInterface $request) use (&$observedHeaders) { + $this->assertTrue($request->hasHeader('amz-sdk-request')); + $observedHeaders[] = $request->getHeaderLine('amz-sdk-request'); return new RejectedPromise( new AwsException('e', $command, ['connection_error' => true]) ); @@ -473,6 +475,17 @@ public function testAddRetryHeader() $retryMW(new Command('SomeCommand'), new Request('GET', ''))->wait(); $this->fail(); } catch (AwsException $e) { } + + $this->assertSame( + [ + 'attempt=1; max=5', + 'attempt=2; max=5', + 'attempt=3; max=5', + 'attempt=4; max=5', + 'attempt=5; max=5', + ], + $observedHeaders + ); } public function testDeciderRetriesWhenStatusCodeMatches() diff --git a/tests/Signature/SignatureV4Test.php b/tests/Signature/SignatureV4Test.php index 1c0ecfeac6..f57eda619e 100644 --- a/tests/Signature/SignatureV4Test.php +++ b/tests/Signature/SignatureV4Test.php @@ -673,8 +673,8 @@ public function testRemovesIllegalV4aHeadersBeforeSigning() static $headers = [ 'X-Amz-Content-Sha256' => 'blah', - 'aws-sdk-invocation-id' => '1', - 'aws-sdk-retry' => 'foo', + 'amz-sdk-invocation-id' => '1', + 'amz-sdk-request' => 'foo', 'transfer-encoding' => 'chunked' ]; $sig = new SignatureV4('foo', 'bar', ['use_v4a' => true]); diff --git a/tests/Sts/StsClientTest.php b/tests/Sts/StsClientTest.php index 4b6bc3eb9d..146f2ce7fa 100644 --- a/tests/Sts/StsClientTest.php +++ b/tests/Sts/StsClientTest.php @@ -477,4 +477,5 @@ private function createTestWebIdentityToken(): string return $tokenPath; } + }