feature #38532 [HttpClient] simplify retry mechanism around RetryStrategyInterface (nicolas-grekas)

This PR was merged into the 5.x branch.

Discussion
----------

[HttpClient] simplify retry mechanism around RetryStrategyInterface

| Q             | A
| ------------- | ---
| Branch?       | 5.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Replaces #38466

I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons.

I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface.

Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that.

The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc).

/cc @jderusse

Commits
-------

544c3e8678 [HttpClient] simplify retry mechanism around RetryStrategyInterface

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

@@ -1641,19 +1641,15 @@ class Configuration implements ConfigurationInterface
                 ->addDefaultsIfNotSet()
                 ->beforeNormalization()
                     ->always(function ($v) {
-                        if (isset($v['backoff_service']) && (isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
-                            throw new \InvalidArgumentException('The "backoff_service" option cannot be used along with the "delay", "multiplier", "max_delay" or "jitter" options.');
-                        }
-                        if (isset($v['decider_service']) && (isset($v['http_codes']))) {
-                            throw new \InvalidArgumentException('The "decider_service" option cannot be used along with the "http_codes" options.');
+                        if (isset($v['retry_strategy']) && (isset($v['http_codes']) || isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
+                            throw new \InvalidArgumentException('The "retry_strategy" option cannot be used along with the "http_codes", "delay", "multiplier", "max_delay" or "jitter" options.');
                         }
 
                         return $v;
                     })
                 ->end()
                 ->children()
-                    ->scalarNode('backoff_service')->defaultNull()->info('service id to override the retry backoff')->end()
-                    ->scalarNode('decider_service')->defaultNull()->info('service id to override the retry decider')->end()
+                    ->scalarNode('retry_strategy')->defaultNull()->info('service id to override the retry strategy')->end()
                     ->arrayNode('http_codes')
                         ->performNoDeepMerging()
                         ->beforeNormalization()
@@ -1668,9 +1664,9 @@ class Configuration implements ConfigurationInterface
                     ->end()
                     ->integerNode('max_retries')->defaultValue(3)->min(0)->end()
                     ->integerNode('delay')->defaultValue(1000)->min(0)->info('Time in ms to delay (or the initial value when multiplier is used)')->end()
-                    ->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: (delay * (multiple ^ retries))')->end()
+                    ->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: delay * (multiple ^ retries)')->end()
                     ->integerNode('max_delay')->defaultValue(0)->min(0)->info('Max time in ms that a retry should ever be delayed (0 = infinite)')->end()
-                    ->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1)) to apply to the delay')->end()
+                    ->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1) to apply to the delay')->end()
                 ->end()
             ;
     }

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -2011,10 +2011,10 @@ class FrameworkExtension extends Extension
         }
 
         if ($this->isConfigEnabled($container, $retryOptions)) {
-            $this->registerHttpClientRetry($retryOptions, 'http_client', $container);
+            $this->registerRetryableHttpClient($retryOptions, 'http_client', $container);
         }
 
-        $httpClientId = $retryOptions['enabled'] ?? false ? 'http_client.retry.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
+        $httpClientId = ($retryOptions['enabled'] ?? false) ? 'http_client.retryable.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
         foreach ($config['scoped_clients'] as $name => $scopeConfig) {
             if ('http_client' === $name) {
                 throw new InvalidArgumentException(sprintf('Invalid scope name: "%s" is reserved.', $name));
@@ -2042,7 +2042,7 @@ class FrameworkExtension extends Extension
             }
 
             if ($this->isConfigEnabled($container, $retryOptions)) {
-                $this->registerHttpClientRetry($retryOptions, $name, $container);
+                $this->registerRetryableHttpClient($retryOptions, $name, $container);
             }
 
             $container->registerAliasForArgument($name, HttpClientInterface::class);
@@ -2062,42 +2062,31 @@ class FrameworkExtension extends Extension
         }
     }
 
-    private function registerHttpClientRetry(array $retryOptions, string $name, ContainerBuilder $container)
+    private function registerRetryableHttpClient(array $options, string $name, ContainerBuilder $container)
     {
         if (!class_exists(RetryableHttpClient::class)) {
-            throw new LogicException('Retry failed request support cannot be enabled as version 5.2+ of the HTTP Client component is required.');
+            throw new LogicException('Support for retrying failed requests requires symfony/http-client 5.2 or higher, try upgrading.');
         }
 
-        if (null !== $retryOptions['backoff_service']) {
-            $backoffReference = new Reference($retryOptions['backoff_service']);
+        if (null !== $options['retry_strategy']) {
+            $retryStrategy = new Reference($options['retry_strategy']);
         } else {
-            $retryServiceId = $name.'.retry.exponential_backoff';
-            $retryDefinition = new ChildDefinition('http_client.retry.abstract_exponential_backoff');
-            $retryDefinition
-                ->replaceArgument(0, $retryOptions['delay'])
-                ->replaceArgument(1, $retryOptions['multiplier'])
-                ->replaceArgument(2, $retryOptions['max_delay'])
-                ->replaceArgument(3, $retryOptions['jitter']);
-            $container->setDefinition($retryServiceId, $retryDefinition);
+            $retryStrategy = new ChildDefinition('http_client.abstract_retry_strategy');
+            $retryStrategy
+                ->replaceArgument(0, $options['http_codes'])
+                ->replaceArgument(1, $options['delay'])
+                ->replaceArgument(2, $options['multiplier'])
+                ->replaceArgument(3, $options['max_delay'])
+                ->replaceArgument(4, $options['jitter']);
+            $container->setDefinition($name.'.retry_strategy', $retryStrategy);
 
-            $backoffReference = new Reference($retryServiceId);
-        }
-        if (null !== $retryOptions['decider_service']) {
-            $deciderReference = new Reference($retryOptions['decider_service']);
-        } else {
-            $retryServiceId = $name.'.retry.decider';
-            $retryDefinition = new ChildDefinition('http_client.retry.abstract_httpstatuscode_decider');
-            $retryDefinition
-                ->replaceArgument(0, $retryOptions['http_codes']);
-            $container->setDefinition($retryServiceId, $retryDefinition);
-
-            $deciderReference = new Reference($retryServiceId);
+            $retryStrategy = new Reference($name.'.retry_strategy');
         }
 
         $container
-            ->register($name.'.retry', RetryableHttpClient::class)
+            ->register($name.'.retryable', RetryableHttpClient::class)
             ->setDecoratedService($name, null, -10) // lower priority than TraceableHttpClient
-            ->setArguments([new Reference($name.'.retry.inner'), $deciderReference, $backoffReference, $retryOptions['max_retries'], new Reference('logger')])
+            ->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')])
             ->addTag('monolog.logger', ['channel' => 'http_client']);
     }
 

src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.php

@@ -17,8 +17,7 @@ use Psr\Http\Message\StreamFactoryInterface;
 use Symfony\Component\HttpClient\HttpClient;
 use Symfony\Component\HttpClient\HttplugClient;
 use Symfony\Component\HttpClient\Psr18Client;
-use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
-use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
+use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 
 return static function (ContainerConfigurator $container) {
@@ -51,19 +50,14 @@ return static function (ContainerConfigurator $container) {
                 service(StreamFactoryInterface::class)->ignoreOnInvalid(),
             ])
 
-        // retry
-        ->set('http_client.retry.abstract_exponential_backoff', ExponentialBackOff::class)
+        ->set('http_client.abstract_retry_strategy', GenericRetryStrategy::class)
             ->abstract()
             ->args([
+                abstract_arg('http codes'),
                 abstract_arg('delay ms'),
                 abstract_arg('multiplier'),
                 abstract_arg('max delay ms'),
                 abstract_arg('jitter'),
             ])
-        ->set('http_client.retry.abstract_httpstatuscode_decider', HttpStatusCodeDecider::class)
-            ->abstract()
-            ->args([
-                abstract_arg('http codes'),
-            ])
     ;
 };

src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

@@ -581,8 +581,7 @@
             <xsd:element name="http-code" type="xsd:integer" minOccurs="0" maxOccurs="unbounded" />
         </xsd:sequence>
         <xsd:attribute name="enabled" type="xsd:boolean" />
-        <xsd:attribute name="backoff-service" type="xsd:string" />
-        <xsd:attribute name="decider-service" type="xsd:string" />
+        <xsd:attribute name="retry-strategy" type="xsd:string" />
         <xsd:attribute name="max-retries" type="xsd:integer" />
         <xsd:attribute name="delay" type="xsd:integer" />
         <xsd:attribute name="multiplier" type="xsd:float" />

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php

@@ -4,8 +4,7 @@ $container->loadFromExtension('framework', [
     'http_client' => [
         'default_options' => [
             'retry_failed' => [
-                'backoff_service' => null,
-                'decider_service' => null,
+                'retry_strategy' => null,
                 'http_codes' => [429, 500],
                 'max_retries' => 2,
                 'delay' => 100,

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml

@@ -2,8 +2,7 @@ framework:
     http_client:
         default_options:
             retry_failed:
-                backoff_service: null
-                decider_service: null
+                retry_strategy: null
                 http_codes: [429, 500]
                 max_retries: 2
                 delay: 100

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

@@ -1500,15 +1500,15 @@ abstract class FrameworkExtensionTest extends TestCase
         }
         $container = $this->createContainerFromFile('http_client_retry');
 
-        $this->assertSame([429, 500], $container->getDefinition('http_client.retry.decider')->getArgument(0));
-        $this->assertSame(100, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(0));
-        $this->assertSame(2, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(1));
-        $this->assertSame(0, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(2));
-        $this->assertSame(0.3, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(3));
-        $this->assertSame(2, $container->getDefinition('http_client.retry')->getArgument(3));
+        $this->assertSame([429, 500], $container->getDefinition('http_client.retry_strategy')->getArgument(0));
+        $this->assertSame(100, $container->getDefinition('http_client.retry_strategy')->getArgument(1));
+        $this->assertSame(2, $container->getDefinition('http_client.retry_strategy')->getArgument(2));
+        $this->assertSame(0, $container->getDefinition('http_client.retry_strategy')->getArgument(3));
+        $this->assertSame(0.3, $container->getDefinition('http_client.retry_strategy')->getArgument(4));
+        $this->assertSame(2, $container->getDefinition('http_client.retryable')->getArgument(2));
 
-        $this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retry')->getClass());
-        $this->assertSame(4, $container->getDefinition('foo.retry.exponential_backoff')->getArgument(1));
+        $this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retryable')->getClass());
+        $this->assertSame(4, $container->getDefinition('foo.retry_strategy')->getArgument(2));
     }
 
     public function testHttpClientWithQueryParameterKey()

src/Symfony/Component/HttpClient/Response/AmpResponse.php

@@ -28,7 +28,6 @@ use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
 use Symfony\Component\HttpClient\Exception\TransportException;
 use Symfony\Component\HttpClient\HttpClientTrait;
 use Symfony\Component\HttpClient\Internal\AmpBody;
-use Symfony\Component\HttpClient\Internal\AmpCanary;
 use Symfony\Component\HttpClient\Internal\AmpClientState;
 use Symfony\Component\HttpClient\Internal\Canary;
 use Symfony\Component\HttpClient\Internal\ClientState;

src/Symfony/Component/HttpClient/Retry/ExponentialBackOff.php

@@ -1,81 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Component\HttpClient\Retry;
-
-use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
-use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
-
-/**
- * A retry backOff with a constant or exponential retry delay.
- *
- * For example, if $delayMilliseconds=10000 & $multiplier=1,
- * each retry will wait exactly 10 seconds.
- *
- * But if $delayMilliseconds=10000 & $multiplier=2:
- *      * Retry 1: 10 second delay
- *      * Retry 2: 20 second delay (10000 * 2 = 20000)
- *      * Retry 3: 40 second delay (20000 * 2 = 40000)
- *
- * @author Ryan Weaver <ryan@symfonycasts.com>
- * @author Jérémy Derussé <jeremy@derusse.com>
- */
-final class ExponentialBackOff implements RetryBackOffInterface
-{
-    private $delayMilliseconds;
-    private $multiplier;
-    private $maxDelayMilliseconds;
-    private $jitter;
-
-    /**
-     * @param int   $delayMilliseconds    Amount of time to delay (or the initial value when multiplier is used)
-     * @param float $multiplier           Multiplier to apply to the delay each time a retry occurs
-     * @param int   $maxDelayMilliseconds Maximum delay to allow (0 means no maximum)
-     * @param float $jitter               Probability of randomness int delay (0 = none, 1 = 100% random)
-     */
-    public function __construct(int $delayMilliseconds = 1000, float $multiplier = 2.0, int $maxDelayMilliseconds = 0, float $jitter = 0.1)
-    {
-        if ($delayMilliseconds < 0) {
-            throw new InvalidArgumentException(sprintf('Delay must be greater than or equal to zero: "%s" given.', $delayMilliseconds));
-        }
-        $this->delayMilliseconds = $delayMilliseconds;
-
-        if ($multiplier < 1) {
-            throw new InvalidArgumentException(sprintf('Multiplier must be greater than or equal to one: "%s" given.', $multiplier));
-        }
-        $this->multiplier = $multiplier;
-
-        if ($maxDelayMilliseconds < 0) {
-            throw new InvalidArgumentException(sprintf('Max delay must be greater than or equal to zero: "%s" given.', $maxDelayMilliseconds));
-        }
-        $this->maxDelayMilliseconds = $maxDelayMilliseconds;
-
-        if ($jitter < 0 || $jitter > 1) {
-            throw new InvalidArgumentException(sprintf('Jitter must be between 0 and 1: "%s" given.', $jitter));
-        }
-        $this->jitter = $jitter;
-    }
-
-    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int
-    {
-        $delay = $this->delayMilliseconds * $this->multiplier ** $retryCount;
-        if ($this->jitter > 0) {
-            $randomness = $delay * $this->jitter;
-            $delay = $delay + random_int(-$randomness, +$randomness);
-        }
-
-        if ($delay > $this->maxDelayMilliseconds && 0 !== $this->maxDelayMilliseconds) {
-            return $this->maxDelayMilliseconds;
-        }
-
-        return (int) $delay;
-    }
-}

src/Symfony/Component/HttpClient/Retry/GenericRetryStrategy.php

@@ -0,0 +1,84 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpClient\Retry;
+
+use Symfony\Component\HttpClient\Response\AsyncContext;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
+
+/**
+ * Decides to retry the request when HTTP status codes belong to the given list of codes.
+ *
+ * @author Jérémy Derussé <jeremy@derusse.com>
+ */
+class GenericRetryStrategy implements RetryStrategyInterface
+{
+    public const DEFAULT_RETRY_STATUS_CODES = [423, 425, 429, 500, 502, 503, 504, 507, 510];
+
+    private $statusCodes;
+    private $delayMs;
+    private $multiplier;
+    private $maxDelayMs;
+    private $jitter;
+
+    /**
+     * @param array $statusCodes List of HTTP status codes that trigger a retry
+     * @param int   $delayMs     Amount of time to delay (or the initial value when multiplier is used)
+     * @param float $multiplier  Multiplier to apply to the delay each time a retry occurs
+     * @param int   $maxDelayMs  Maximum delay to allow (0 means no maximum)
+     * @param float $jitter      Probability of randomness int delay (0 = none, 1 = 100% random)
+     */
+    public function __construct(array $statusCodes = self::DEFAULT_RETRY_STATUS_CODES, int $delayMs = 1000, float $multiplier = 2.0, int $maxDelayMs = 0, float $jitter = 0.1)
+    {
+        $this->statusCodes = $statusCodes;
+
+        if ($delayMs < 0) {
+            throw new InvalidArgumentException(sprintf('Delay must be greater than or equal to zero: "%s" given.', $delayMs));
+        }
+        $this->delayMs = $delayMs;
+
+        if ($multiplier < 1) {
+            throw new InvalidArgumentException(sprintf('Multiplier must be greater than or equal to one: "%s" given.', $multiplier));
+        }
+        $this->multiplier = $multiplier;
+
+        if ($maxDelayMs < 0) {
+            throw new InvalidArgumentException(sprintf('Max delay must be greater than or equal to zero: "%s" given.', $maxDelayMs));
+        }
+        $this->maxDelayMs = $maxDelayMs;
+
+        if ($jitter < 0 || $jitter > 1) {
+            throw new InvalidArgumentException(sprintf('Jitter must be between 0 and 1: "%s" given.', $jitter));
+        }
+        $this->jitter = $jitter;
+    }
+
+    public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
+    {
+        return \in_array($context->getStatusCode(), $this->statusCodes, true);
+    }
+
+    public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int
+    {
+        $delay = $this->delayMs * $this->multiplier ** $context->getInfo('retry_count');
+
+        if ($this->jitter > 0) {
+            $randomness = $delay * $this->jitter;
+            $delay = $delay + random_int(-$randomness, +$randomness);
+        }
+
+        if ($delay > $this->maxDelayMs && 0 !== $this->maxDelayMs) {
+            return $this->maxDelayMs;
+        }
+
+        return (int) $delay;
+    }
+}

src/Symfony/Component/HttpClient/Retry/HttpStatusCodeDecider.php

@@ -1,35 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Component\HttpClient\Retry;
-
-/**
- * Decides to retry the request when HTTP status codes belong to the given list of codes.
- *
- * @author Jérémy Derussé <jeremy@derusse.com>
- */
-final class HttpStatusCodeDecider implements RetryDeciderInterface
-{
-    private $statusCodes;
-
-    /**
-     * @param array $statusCodes List of HTTP status codes that trigger a retry
-     */
-    public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503, 504, 507, 510])
-    {
-        $this->statusCodes = $statusCodes;
-    }
-
-    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
-    {
-        return \in_array($responseStatusCode, $this->statusCodes, true);
-    }
-}

src/Symfony/Component/HttpClient/Retry/RetryBackOffInterface.php

@@ -1,25 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Component\HttpClient\Retry;
-
-use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
-
-/**
- * @author Jérémy Derussé <jeremy@derusse.com>
- */
-interface RetryBackOffInterface
-{
-    /**
-     * Returns the time to wait in milliseconds.
-     */
-    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int;
-}

src/Symfony/Component/HttpClient/Retry/RetryStrategyInterface.php

@@ -11,10 +11,14 @@
 
 namespace Symfony\Component\HttpClient\Retry;
 
+use Symfony\Component\HttpClient\Response\AsyncContext;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
+
 /**
  * @author Jérémy Derussé <jeremy@derusse.com>
+ * @author Nicolas Grekas <p@tchwork.com>
  */
-interface RetryDeciderInterface
+interface RetryStrategyInterface
 {
     /**
      * Returns whether the request should be retried.
@@ -23,5 +27,10 @@ interface RetryDeciderInterface
      *
      * @return ?bool Returns null to signal that the body is required to take a decision
      */
-    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
+    public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool;
+
+    /**
+     * Returns the time to wait in milliseconds.
+     */
+    public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int;
 }

src/Symfony/Component/HttpClient/RetryableHttpClient.php

@@ -15,10 +15,8 @@ use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
 use Symfony\Component\HttpClient\Response\AsyncContext;
 use Symfony\Component\HttpClient\Response\AsyncResponse;
-use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
-use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
-use Symfony\Component\HttpClient\Retry\RetryBackOffInterface;
-use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
+use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
+use Symfony\Component\HttpClient\Retry\RetryStrategyInterface;
 use Symfony\Contracts\HttpClient\ChunkInterface;
 use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -33,7 +31,6 @@ class RetryableHttpClient implements HttpClientInterface
 {
     use AsyncDecoratorTrait;
 
-    private $decider;
     private $strategy;
     private $maxRetries;
     private $logger;
@@ -41,11 +38,10 @@ class RetryableHttpClient implements HttpClientInterface
     /**
      * @param int $maxRetries The maximum number of times to retry
      */
-    public function __construct(HttpClientInterface $client, RetryDeciderInterface $decider = null, RetryBackOffInterface $strategy = null, int $maxRetries = 3, LoggerInterface $logger = null)
+    public function __construct(HttpClientInterface $client, RetryStrategyInterface $strategy = null, int $maxRetries = 3, LoggerInterface $logger = null)
     {
         $this->client = $client;
-        $this->decider = $decider ?? new HttpStatusCodeDecider();
-        $this->strategy = $strategy ?? new ExponentialBackOff();
+        $this->strategy = $strategy ?? new GenericRetryStrategy();
         $this->maxRetries = $maxRetries;
         $this->logger = $logger ?: new NullLogger();
     }
@@ -69,23 +65,22 @@ class RetryableHttpClient implements HttpClientInterface
                     return;
                 }
             } catch (TransportExceptionInterface $exception) {
-                // catch TransportExceptionInterface to send it to strategy.
+                // catch TransportExceptionInterface to send it to the strategy
+                $context->setInfo('retry_count', $retryCount);
             }
 
-            $statusCode = $context->getStatusCode();
-            $headers = $context->getHeaders();
             if (null === $exception) {
                 if ($chunk->isFirst()) {
-                    $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null);
+                    $context->setInfo('retry_count', $retryCount);
 
-                    if (false === $shouldRetry) {
+                    if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
                         $context->passthru();
                         yield $chunk;
 
                         return;
                     }
 
-                    // Decider need body to decide
+                    // Body is needed to decide
                     if (null === $shouldRetry) {
                         $firstChunk = $chunk;
                         $content = '';
@@ -94,12 +89,13 @@ class RetryableHttpClient implements HttpClientInterface
                     }
                 } else {
                     $content .= $chunk->getContent();
+
                     if (!$chunk->isLast()) {
                         return;
                     }
-                    $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content);
-                    if (null === $shouldRetry) {
-                        throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with a body.', \get_class($this->decider)));
+
+                    if (null === $shouldRetry = $this->strategy->shouldRetry($context, $content, null)) {
+                        throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', \get_class($this->strategy)));
                     }
 
                     if (false === $shouldRetry) {
@@ -113,14 +109,13 @@ class RetryableHttpClient implements HttpClientInterface
                 }
             }
 
-            $context->setInfo('retry_count', $retryCount);
             $context->getResponse()->cancel();
 
-            $delay = $this->getDelayFromHeader($headers) ?? $this->strategy->getDelay($retryCount, $method, $url, $options, $statusCode, $headers, $chunk instanceof LastChunk ? $content : null, $exception);
+            $delay = $this->getDelayFromHeader($context->getHeaders()) ?? $this->strategy->getDelay($context, $chunk instanceof LastChunk ? $content : null, $exception);
             ++$retryCount;
 
-            $this->logger->info('Error returned by the server. Retrying #{retryCount} using {delay} ms delay: '.($exception ? $exception->getMessage() : 'StatusCode: '.$statusCode), [
-                'retryCount' => $retryCount,
+            $this->logger->info('Try #{count} after {delay}ms'.($exception ? ': '.$exception->getMessage() : ', status code: '.$context->getStatusCode()), [
+                'count' => $retryCount,
                 'delay' => $delay,
             ]);
 
@@ -139,6 +134,7 @@ class RetryableHttpClient implements HttpClientInterface
             if (is_numeric($after)) {
                 return (int) $after * 1000;
             }
+
             if (false !== $time = strtotime($after)) {
                 return max(0, $time - time()) * 1000;
             }

src/Symfony/Component/HttpClient/Tests/Retry/GenericRetryStrategyTest.php

@@ -12,18 +12,35 @@
 namespace Symfony\Component\HttpClient\Tests\Retry;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
+use Symfony\Component\HttpClient\MockHttpClient;
+use Symfony\Component\HttpClient\Response\AsyncContext;
+use Symfony\Component\HttpClient\Response\MockResponse;
+use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
 
-class ExponentialBackOffTest extends TestCase
+class GenericRetryStrategyTest extends TestCase
 {
+    public function testShouldRetryStatusCode()
+    {
+        $strategy = new GenericRetryStrategy([500]);
+
+        self::assertTrue($strategy->shouldRetry($this->getContext(0, 'GET', 'http://example.com/', 500), null, null));
+    }
+
+    public function testIsNotRetryableOk()
+    {
+        $strategy = new GenericRetryStrategy([500]);
+
+        self::assertFalse($strategy->shouldRetry($this->getContext(0, 'GET', 'http://example.com/', 200), null, null));
+    }
+
     /**
      * @dataProvider provideDelay
      */
     public function testGetDelay(int $delay, int $multiplier, int $maxDelay, int $previousRetries, int $expectedDelay)
     {
-        $backOff = new ExponentialBackOff($delay, $multiplier, $maxDelay, 0);
+        $strategy = new GenericRetryStrategy([], $delay, $multiplier, $maxDelay, 0);
 
-        self::assertSame($expectedDelay, $backOff->getDelay($previousRetries, 'GET', 'http://example.com/', [], 200, [], null, null));
+        self::assertSame($expectedDelay, $strategy->getDelay($this->getContext($previousRetries, 'GET', 'http://example.com/', 200), null, null));
     }
 
     public function provideDelay(): iterable
@@ -53,11 +70,11 @@ class ExponentialBackOffTest extends TestCase
 
     public function testJitter()
     {
-        $backOff = new ExponentialBackOff(1000, 1, 0, 1);
+        $strategy = new GenericRetryStrategy([], 1000, 1, 0, 1);
         $belowHalf = 0;
         $aboveHalf = 0;
         for ($i = 0; $i < 20; ++$i) {
-            $delay = $backOff->getDelay(0, 'GET', 'http://example.com/', [], 200, [], null, null);
+            $delay = $strategy->getDelay($this->getContext(0, 'GET', 'http://example.com/', 200), null, null);
             if ($delay < 500) {
                 ++$belowHalf;
             } elseif ($delay > 1500) {
@@ -68,4 +85,18 @@ class ExponentialBackOffTest extends TestCase
         $this->assertGreaterThanOrEqual(1, $belowHalf);
         $this->assertGreaterThanOrEqual(1, $aboveHalf);
     }
+
+    private function getContext($retryCount, $method, $url, $statusCode): AsyncContext
+    {
+        $passthru = null;
+        $info = [
+            'retry_count' => $retryCount,
+            'http_method' => $method,
+            'url' => $url,
+            'http_code' => $statusCode,
+        ];
+        $response = new MockResponse('', $info);
+
+        return new AsyncContext($passthru, new MockHttpClient(), $response, $info, null, 0);
+    }
 }

src/Symfony/Component/HttpClient/Tests/Retry/HttpStatusCodeDeciderTest.php

@@ -1,32 +0,0 @@
-<?php
-
-/*
- * This file is part of the Symfony package.
- *
- * (c) Fabien Potencier <fabien@symfony.com>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Component\HttpClient\Tests\Retry;
-
-use PHPUnit\Framework\TestCase;
-use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
-
-class HttpStatusCodeDeciderTest extends TestCase
-{
-    public function testShouldRetryStatusCode()
-    {
-        $decider = new HttpStatusCodeDecider([500]);
-
-        self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null));
-    }
-
-    public function testIsNotRetryableOk()
-    {
-        $decider = new HttpStatusCodeDecider([500]);
-
-        self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null));
-    }
-}

src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php

@@ -5,11 +5,11 @@ namespace Symfony\Component\HttpClient\Tests;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpClient\Exception\ServerException;
 use Symfony\Component\HttpClient\MockHttpClient;
+use Symfony\Component\HttpClient\Response\AsyncContext;
 use Symfony\Component\HttpClient\Response\MockResponse;
-use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
-use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
-use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
+use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
 use Symfony\Component\HttpClient\RetryableHttpClient;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 
 class RetryableHttpClientTest extends TestCase
 {
@@ -20,8 +20,7 @@ class RetryableHttpClientTest extends TestCase
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new GenericRetryStrategy([500], 0),
             1
         );
 
@@ -38,8 +37,7 @@ class RetryableHttpClientTest extends TestCase
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new GenericRetryStrategy([500], 0),
             1
         );
 
@@ -56,13 +54,12 @@ class RetryableHttpClientTest extends TestCase
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new class() implements RetryDeciderInterface {
-                public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
+            new class(GenericRetryStrategy::DEFAULT_RETRY_STATUS_CODES, 0) extends GenericRetryStrategy {
+                public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
                 {
-                    return null === $responseContent ? null : 200 !== $responseCode;
+                    return null === $responseContent ? null : 200 !== $context->getStatusCode();
                 }
             },
-            new ExponentialBackOff(0),
             1
         );
 
@@ -78,13 +75,12 @@ class RetryableHttpClientTest extends TestCase
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new class() implements RetryDeciderInterface {
-                public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, \Throwable $throwable = null): ?bool
+            new class(GenericRetryStrategy::DEFAULT_RETRY_STATUS_CODES, 0) extends GenericRetryStrategy {
+                public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
                 {
                     return null;
                 }
             },
-            new ExponentialBackOff(0),
             1
         );
 
@@ -100,8 +96,7 @@ class RetryableHttpClientTest extends TestCase
             new MockHttpClient([
                 new MockResponse('', ['http_code' => 500]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new GenericRetryStrategy([500], 0),
             0
         );