Do not send ResponseInterface to decider/backoff

symfony · Sep 16, 2020 · 240e18d · 240e18d
1 parent ecfdf60
commit 240e18d
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 16 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -1486,7 +1486,7 @@ public function testHttpClientOverrideDefaultOptions()
 
     public function testHttpClientRetry()
     {
-        if (!\class_exists(RetryHttpClient::class)) {
+        if (!class_exists(RetryHttpClient::class)) {
             $this->expectException(LogicException::class);
         }
         $container = $this->createContainerFromFile('http_client_retry');

diff --git a/src/Symfony/Component/HttpClient/Retry/ExponentialBackOff.php b/src/Symfony/Component/HttpClient/Retry/ExponentialBackOff.php
@@ -57,7 +57,7 @@ public function __construct(int $delayMilliseconds = 1000, float $multiplier = 2
         $this->maxDelayMilliseconds = $maxDelayMilliseconds;
     }
 
-    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $response, \Throwable $throwable = null): int
+    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, \Throwable $throwable = null): int
     {
         $delay = $this->delayMilliseconds * $this->multiplier ** $retryCount;
 

diff --git a/src/Symfony/Component/HttpClient/Retry/HttpCodeDecider.php b/src/Symfony/Component/HttpClient/Retry/HttpCodeDecider.php
@@ -31,12 +31,12 @@ public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503,
         $this->statusCodes = $statusCodes;
     }
 
-    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $response, \Throwable $throwable = null): bool
+    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, \Throwable $throwable = null): bool
     {
         if ($throwable instanceof TransportExceptionInterface) {
             return true;
         }
 
-        return \in_array($response->getStatusCode(), $this->statusCodes, true);
+        return \in_array($responseStatusCode, $this->statusCodes, true);
     }
 }
diff --git a/src/Symfony/Component/HttpClient/Retry/RetryBackOffInterface.php b/src/Symfony/Component/HttpClient/Retry/RetryBackOffInterface.php
@@ -21,5 +21,5 @@ interface RetryBackOffInterface
     /**
      * Returns the time to wait in milliseconds.
      */
-    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $response, \Throwable $throwable = null): int;
+    public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, \Throwable $throwable = null): int;
 }
diff --git a/src/Symfony/Component/HttpClient/Retry/RetryDeciderInterface.php b/src/Symfony/Component/HttpClient/Retry/RetryDeciderInterface.php
@@ -21,5 +21,5 @@ interface RetryDeciderInterface
     /**
      * Returns whether the request should be retried.
      */
-    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $response, \Throwable $throwable = null): bool;
+    public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, \Throwable $throwable = null): bool;
 }
diff --git a/src/Symfony/Component/HttpClient/RetryHttpClient.php b/src/Symfony/Component/HttpClient/RetryHttpClient.php
@@ -13,6 +13,7 @@
 
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
+use Symfony\Component\HttpClient\Chunk\ErrorChunk;
 use Symfony\Component\HttpClient\Response\AsyncContext;
 use Symfony\Component\HttpClient\Response\AsyncResponse;
 use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
@@ -57,16 +58,14 @@ public function request(string $method, string $url, array $options = []): Respo
         return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, &$retryCount) {
             $exception = null;
             try {
-                // clone the chunk to let the original value of `$didthrow`
-                $clonedChunk = clone $chunk;
-                if ($clonedChunk->isTimeout() || null !== $clonedChunk->getInformationalStatus()) {
+                if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus()) {
                     yield $chunk;
 
                     return;
                 }
 
                 // only retry first chunk
-                if (!$clonedChunk->isFirst()) {
+                if (!$chunk->isFirst()) {
                     $context->passthru();
                     yield $chunk;
 
@@ -76,32 +75,37 @@ public function request(string $method, string $url, array $options = []): Respo
                 // catch TransportExceptionInterface to send it to strategy.
             }
 
-            if ($retryCount >= $this->maxRetries || !$this->decider->shouldRetry($method, $url, $options, $response = $context->getResponse(), $exception)) {
+            if ($retryCount >= $this->maxRetries || !$this->decider->shouldRetry($method, $url, $options, $statusCode = $context->getStatusCode(), $headers = $context->getHeaders(), $exception)) {
                 $context->passthru();
                 yield $chunk;
 
                 return;
             }
 
             $context->setInfo('retry_count', $retryCount);
-            $response->cancel();
+            $context->getResponse()->cancel();
 
-            $delay = $this->getDelayFromHeader($response) ?? $this->strategy->getDelay($retryCount, $method, $url, $options, $response = $context->getResponse(), $exception);
+            $delay = $this->getDelayFromHeader($headers) ?? $this->strategy->getDelay($retryCount, $method, $url, $options, $statusCode, $headers, $exception);
             ++$retryCount;
 
-            $this->logger->info('Error returned by the server. Retrying #{retryCount} using {delay} ms delay: '.($exception ? $exception->getMessage() : 'StatusCode: '.$response->getStatusCode()), [
+            $this->logger->info('Error returned by the server. Retrying #{retryCount} using {delay} ms delay: '.($exception ? $exception->getMessage() : 'StatusCode: '.$statusCode), [
                 'retryCount' => $retryCount,
                 'delay' => $delay,
             ]);
 
+            try {
+                $chunk->getContent();
+            } catch (TransportExceptionInterface $exception) {
+                // silent the failsafe exception
+            }
             $context->replaceRequest($method, $url, $options);
             $context->pause($delay / 1000);
         });
     }
 
-    private function getDelayFromHeader(ResponseInterface $response): ?int
+    private function getDelayFromHeader(array $headers): ?int
     {
-        if (null !== $after = $response->getHeaders(false)['retry-after'][0] ?? null) {
+        if (null !== $after = $headers['retry-after'][0] ?? null) {
             if (is_numeric($after)) {
                 return (int) $after * 1000;
             }