bug #38418 [HttpClient] minor fixes in RetryableHttpClient (nicolas-grekas)

fabpot · fabpot · commit a324b22d1af4 · 2020-10-05T19:15:39.000+02:00
This PR was merged into the 5.2-dev branch. Discussion ---------- [HttpClient] minor fixes in RetryableHttpClient | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Commits ------- 4955628 [HttpClient] minor fixes in RetryableHttpClient
diff --git a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php
@@ -36,9 +36,9 @@ final class AsyncResponse implements ResponseInterface, StreamableInterface
     private $passthru;
 
     /**
-     * @param callable(ChunkInterface, AsyncContext): ?\Iterator $passthru
+     * @param ?callable(ChunkInterface, AsyncContext): ?\Iterator $passthru
      */
-    public function __construct(HttpClientInterface $client, string $method, string $url, array $options, callable $passthru)
+    public function __construct(HttpClientInterface $client, string $method, string $url, array $options, callable $passthru = null)
     {
         $this->client = $client;
         $this->shouldBuffer = $options['buffer'] ?? true;
diff --git a/src/Symfony/Component/HttpClient/RetryableHttpClient.php b/src/Symfony/Component/HttpClient/RetryableHttpClient.php
@@ -53,7 +53,7 @@ public function __construct(HttpClientInterface $client, RetryDeciderInterface $
     public function request(string $method, string $url, array $options = []): ResponseInterface
     {
         if ($this->maxRetries <= 0) {
-            return $this->client->request($method, $url, $options);
+            return new AsyncResponse($this->client, $method, $url, $options);
         }
 
         $retryCount = 0;
@@ -97,7 +97,7 @@ public function request(string $method, string $url, array $options = []): Respo
                     if (!$chunk->isLast()) {
                         return;
                     }
-                    $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content, null);
+                    $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)));
                     }
diff --git a/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php
@@ -93,4 +93,24 @@ public function shouldRetry(string $requestMethod, string $requestUrl, array $re
         $this->expectExceptionMessageMatches('/must not return null when called with a body/');
         $response->getHeaders();
     }
+
+    public function testStreamNoRetry()
+    {
+        $client = new RetryableHttpClient(
+            new MockHttpClient([
+                new MockResponse('', ['http_code' => 500]),
+            ]),
+            new HttpStatusCodeDecider([500]),
+            new ExponentialBackOff(0),
+            0
+        );
+
+        $response = $client->request('GET', 'http://example.com/foo-bar');
+
+        foreach ($client->stream($response) as $chunk) {
+            if ($chunk->isFirst()) {
+                self::assertSame(500, $response->getStatusCode());
+            }
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ public function __construct(HttpClientInterface $client, RetryDeciderInterface $`
`53`	`53`	`public function request(string $method, string $url, array $options = []): ResponseInterface`
`54`	`54`	`{`
`55`	`55`	`if ($this->maxRetries <= 0) {`
`56`		`- return $this->client->request($method, $url, $options);`
	`56`	`+ return new AsyncResponse($this->client, $method, $url, $options);`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`$retryCount = 0;`
`@@ -97,7 +97,7 @@ public function request(string $method, string $url, array $options = []): Respo`
`97`	`97`	`if (!$chunk->isLast()) {`
`98`	`98`	`return;`
`99`	`99`	`}`
`100`		`- $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content, null);`
	`100`	`+ $shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content);`
`101`	`101`	`if (null === $shouldRetry) {`
`102`	`102`	`throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with a body.', \get_class($this->decider)));`
`103`	`103`	`}`