symfony · nicolas-grekas · Oct 5, 2020
diff --git a/src/Symfony/Component/HttpClient/Retry/HttpStatusCodeDecider.php b/src/Symfony/Component/HttpClient/Retry/HttpStatusCodeDecider.php
@@ -28,7 +28,7 @@ public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503,
         $this->statusCodes = $statusCodes;
     }
 
-    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
+    public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
     {
         return \in_array($responseStatusCode, $this->statusCodes, true);
     }

diff --git a/src/Symfony/Component/HttpClient/Retry/RetryDeciderInterface.php b/src/Symfony/Component/HttpClient/Retry/RetryDeciderInterface.php
@@ -23,5 +23,5 @@ 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(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
 }
diff --git a/src/Symfony/Component/HttpClient/Retry/RetryStrategyInterface.php b/src/Symfony/Component/HttpClient/Retry/RetryStrategyInterface.php
@@ -0,0 +1,20 @@
+<?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;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+interface RetryStrategyInterface
+{
+    public function getToken(string $requestMethod, string $requestUrl, array $requestOptions): ?RetryToken;
+}
diff --git a/src/Symfony/Component/HttpClient/Retry/RetryToken.php b/src/Symfony/Component/HttpClient/Retry/RetryToken.php
@@ -0,0 +1,47 @@
+<?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;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+class RetryToken
+{
+    private $shouldRetry;
+    private $getDelay;
+
+    public function __construct(\Closure $shouldRetry, \Closure $getDelay)
+    {
+        $this->shouldRetry = $shouldRetry;
+        $this->getDelay = $getDelay;
+    }
+
+    /**
+     * Returns whether the request should be retried.
+     *
+     * @param ?string $responseContent Null is passed when the body did not arrive yet
+     *
+     * @return ?bool Returns null to signal that the body is required to take a decision
+     */
+    public function shouldRetry(int $retryCount, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
+    {
+        return ($this->shouldRetry)($retryCount, $responseStatusCode, $responseHeaders, $responseContent);
+    }
+
+    /**
+     * Returns the time to wait in milliseconds.
+     */
+    public function getDelay(int $retryCount, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int
+    {
+        return ($this->getDelay)($retryCount, $responseStatusCode, $responseHeaders, $responseContent, $exception);
+    }
+}
diff --git a/src/Symfony/Component/HttpClient/Retry/StatelessStrategy.php b/src/Symfony/Component/HttpClient/Retry/StatelessStrategy.php
@@ -0,0 +1,40 @@
+<?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;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+class StatelessStrategy implements RetryStrategyInterface
+{
+    private $decider;
+    private $backoff;
+
+    public function __construct(RetryDeciderInterface $decider = null, RetryBackOffInterface $backoff = null)
+    {
+        $this->decider = $decider ?? new HttpStatusCodeDecider();
+        $this->backoff = $backoff ?? new ExponentialBackOff();
+    }
+
+    public function getToken(string $requestMethod, string $requestUrl, array $requestOptions): ?RetryToken
+    {
+        $decider = function ($retryCount, $responseStatusCode, $responseHeaders, $responseContent) use ($requestMethod, $requestUrl, $requestOptions) {
+            return $this->decider->shouldRetry($retryCount, $requestMethod, $requestUrl, $requestOptions, $responseStatusCode, $responseHeaders, $responseContent);
+        };
+
+        $backoff = function ($retryCount, $responseStatusCode, $responseHeaders, $responseContent, $exception) use ($requestMethod, $requestUrl, $requestOptions) {
+            return $this->backoff->getDelay($retryCount, $requestMethod, $requestUrl, $requestOptions, $responseStatusCode, $responseHeaders, $responseContent, $exception);
+        };
+
+        return new RetryToken($decider, $backoff);
+    }
+}
diff --git a/src/Symfony/Component/HttpClient/RetryableHttpClient.php b/src/Symfony/Component/HttpClient/RetryableHttpClient.php
@@ -15,10 +15,8 @@
 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\RetryStrategyInterface;
+use Symfony\Component\HttpClient\Retry\StatelessStrategy;
 use Symfony\Contracts\HttpClient\ChunkInterface;
 use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
@@ -33,34 +31,32 @@ class RetryableHttpClient implements HttpClientInterface
 {
     use AsyncDecoratorTrait;
 
-    private $decider;
     private $strategy;
     private $maxRetries;
     private $logger;
 
     /**
      * @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 StatelessStrategy();
         $this->maxRetries = $maxRetries;
         $this->logger = $logger ?: new NullLogger();
     }
 
     public function request(string $method, string $url, array $options = []): ResponseInterface
     {
-        if ($this->maxRetries <= 0) {
+        $retryToken = $this->strategy->getToken($method, $url, $options);
+
+        if (null === $retryToken || $this->maxRetries <= 0) {
             return new AsyncResponse($this->client, $method, $url, $options);
         }
 
         $retryCount = 0;
-        $content = '';
-        $firstChunk = null;
 
-        return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, &$retryCount, &$content, &$firstChunk) {
+        return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, $retryToken, &$retryCount, &$content, &$firstChunk) {
             $exception = null;
             try {
                 if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus()) {
@@ -69,14 +65,14 @@ public function request(string $method, string $url, array $options = []): Respo
                     return;
                 }
             } catch (TransportExceptionInterface $exception) {
-                // catch TransportExceptionInterface to send it to strategy.
+                // catch TransportExceptionInterface to send it to RetryToken::getDelay().
             }
 
             $statusCode = $context->getStatusCode();
             $headers = $context->getHeaders();
             if (null === $exception) {
                 if ($chunk->isFirst()) {
-                    $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null);
+                    $shouldRetry = $retryToken->shouldRetry($retryCount, $statusCode, $headers, null);
 
                     if (false === $shouldRetry) {
                         $context->passthru();
@@ -85,7 +81,7 @@ public function request(string $method, string $url, array $options = []): Respo
                         return;
                     }
 
-                    // Decider need body to decide
+                    // Body is needed to decide
                     if (null === $shouldRetry) {
                         $firstChunk = $chunk;
                         $content = '';
@@ -94,12 +90,15 @@ public function request(string $method, string $url, array $options = []): Respo
                     }
                 } else {
                     $content .= $chunk->getContent();
+
                     if (!$chunk->isLast()) {
                         return;
                     }
-                    $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content);
+
+                    $shouldRetry = $retryToken->shouldRetry($retryCount, $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)));
+                        throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', get_debug_type($retryToken)));
                     }
 
                     if (false === $shouldRetry) {
@@ -116,7 +115,7 @@ public function request(string $method, string $url, array $options = []): Respo
             $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($headers) ?? $retryToken->getDelay($retryCount, $statusCode, $headers, $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), [

diff --git a/src/Symfony/Component/HttpClient/Tests/Retry/HttpStatusCodeDeciderTest.php b/src/Symfony/Component/HttpClient/Tests/Retry/HttpStatusCodeDeciderTest.php
@@ -20,13 +20,13 @@ public function testShouldRetryStatusCode()
     {
         $decider = new HttpStatusCodeDecider([500]);
 
-        self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null));
+        self::assertTrue($decider->shouldRetry(1, 'GET', 'http://example.com/', [], 500, [], null));
     }
 
     public function testIsNotRetryableOk()
     {
         $decider = new HttpStatusCodeDecider([500]);
 
-        self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null));
+        self::assertFalse($decider->shouldRetry(1, 'GET', 'http://example.com/', [], 200, [], null));
     }
 }
diff --git a/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php
@@ -9,6 +9,7 @@
 use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
 use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
 use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
+use Symfony\Component\HttpClient\Retry\StatelessStrategy;
 use Symfony\Component\HttpClient\RetryableHttpClient;
 
 class RetryableHttpClientTest extends TestCase
@@ -20,8 +21,10 @@ public function testRetryOnError()
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new StatelessStrategy(
+                new HttpStatusCodeDecider([500]),
+                new ExponentialBackOff(0)
+            ),
             1
         );
 
@@ -38,8 +41,10 @@ public function testRetryRespectStrategy()
                 new MockResponse('', ['http_code' => 500]),
                 new MockResponse('', ['http_code' => 200]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new StatelessStrategy(
+                new HttpStatusCodeDecider([500]),
+                new ExponentialBackOff(0)
+            ),
             1
         );
 
@@ -56,13 +61,15 @@ public function testRetryWithBody()
                 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
-                {
-                    return null === $responseContent ? null : 200 !== $responseCode;
-                }
-            },
-            new ExponentialBackOff(0),
+            new StatelessStrategy(
+                new class() implements RetryDeciderInterface {
+                    public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
+                    {
+                        return null === $responseContent ? null : 200 !== $responseCode;
+                    }
+                },
+                new ExponentialBackOff(0)
+            ),
             1
         );
 
@@ -78,13 +85,15 @@ public function testRetryWithBodyInvalid()
                 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
-                {
-                    return null;
-                }
-            },
-            new ExponentialBackOff(0),
+            new StatelessStrategy(
+                new class() implements RetryDeciderInterface {
+                    public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, \Throwable $throwable = null): ?bool
+                    {
+                        return null;
+                    }
+                },
+                new ExponentialBackOff(0)
+            ),
             1
         );
 
@@ -100,8 +109,10 @@ public function testStreamNoRetry()
             new MockHttpClient([
                 new MockResponse('', ['http_code' => 500]),
             ]),
-            new HttpStatusCodeDecider([500]),
-            new ExponentialBackOff(0),
+            new StatelessStrategy(
+                new HttpStatusCodeDecider([500]),
+                new ExponentialBackOff(0)
+            ),
             0
         );