[HttpClient] fix RetryableHttpClient when a response is canceled

Author: nicolas-grekas

EventSourceHttpClient.php

@@ -78,7 +78,7 @@ public function request(string $method, string $url, array $options = []): Respo
             try {
                 $isTimeout = $chunk->isTimeout();
 
-                if (null !== $chunk->getInformationalStatus()) {
+                if (null !== $chunk->getInformationalStatus() || $context->getInfo('canceled')) {
                     yield $chunk;
 
                     return;

Response/AsyncResponse.php

@@ -247,7 +247,7 @@ public static function stream(iterable $responses, float $timeout = null, string
                 }
             }
 
-            if (!$client) {
+            if (!$client || !$wrappedResponses) {
                 return;
             }
 

Response/MockResponse.php

@@ -105,7 +105,11 @@ public function cancel(): void
     {
         $this->info['canceled'] = true;
         $this->info['error'] = 'Response has been canceled.';
-        $this->body = null;
+        try {
+            $this->body = null;
+        } catch (TransportException $e) {
+            // ignore errors when canceling
+        }
     }
 
     /**

RetryableHttpClient.php

@@ -59,7 +59,7 @@ 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, &$content, &$firstChunk) {
             $exception = null;
             try {
-                if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus()) {
+                if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus() || $context->getInfo('canceled')) {
                     yield $chunk;
 
                     return;
@@ -76,23 +76,14 @@ public function request(string $method, string $url, array $options = []): Respo
                     }
 
                     if (false === $shouldRetry) {
-                        $context->passthru();
-                        if (null !== $firstChunk) {
-                            yield $firstChunk;
-                            yield $context->createChunk($content);
-                            yield $chunk;
-                        } else {
-                            yield $chunk;
-                        }
-                        $content = '';
+                        yield from $this->passthru($context, $firstChunk, $content, $chunk);
 
                         return;
                     }
                 }
             } elseif ($chunk->isFirst()) {
                 if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
-                    $context->passthru();
-                    yield $chunk;
+                    yield from $this->passthru($context, $firstChunk, $content, $chunk);
 
                     return;
                 }
@@ -105,9 +96,9 @@ public function request(string $method, string $url, array $options = []): Respo
                     return;
                 }
             } else {
-                $content .= $chunk->getContent();
-
                 if (!$chunk->isLast()) {
+                    $content .= $chunk->getContent();
+
                     return;
                 }
 
@@ -116,10 +107,7 @@ public function request(string $method, string $url, array $options = []): Respo
                 }
 
                 if (false === $shouldRetry) {
-                    $context->passthru();
-                    yield $firstChunk;
-                    yield $context->createChunk($content);
-                    $content = '';
+                    yield from $this->passthru($context, $firstChunk, $content, $chunk);
 
                     return;
                 }
@@ -159,4 +147,22 @@ private function getDelayFromHeader(array $headers): ?int
 
         return null;
     }
+
+    private function passthru(AsyncContext $context, ?ChunkInterface $firstChunk, string &$content, ChunkInterface $lastChunk): \Generator
+    {
+        $context->passthru();
+
+        if (null !== $firstChunk) {
+            yield $firstChunk;
+        }
+
+        if ('' !== $content) {
+            $chunk = $context->createChunk($content);
+            $content = '';
+
+            yield $chunk;
+        }
+
+        yield $lastChunk;
+    }
 }

Tests/RetryableHttpClientTest.php

@@ -4,6 +4,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpClient\Exception\ServerException;
+use Symfony\Component\HttpClient\HttpClient;
 use Symfony\Component\HttpClient\MockHttpClient;
 use Symfony\Component\HttpClient\NativeHttpClient;
 use Symfony\Component\HttpClient\Response\AsyncContext;
@@ -159,4 +160,22 @@ public function shouldRetry(AsyncContext $context, ?string $responseContent, ?Tr
         $this->assertCount(2, $logger->logs);
         $this->assertSame('Try #{count} after {delay}ms: Could not resolve host "does.not.exists".', $logger->logs[0]);
     }
+
+    public function testCancelOnTimeout()
+    {
+        $client = HttpClient::create();
+
+        if ($client instanceof NativeHttpClient) {
+            $this->markTestSkipped('NativeHttpClient cannot timeout before receiving headers');
+        }
+
+        $client = new RetryableHttpClient($client);
+
+        $response = $client->request('GET', 'https://example.com/');
+
+        foreach ($client->stream($response, 0) as $chunk) {
+            $this->assertTrue($chunk->isTimeout());
+            $response->cancel();
+        }
+    }
 }