8000 [HttpClient] fix RetryableHttpClient when a response is canceled · symfony/symfony@4f827c3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4f827c3

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpClient] fix RetryableHttpClient when a response is canceled
1 parent 166ca8d commit 4f827c3

File tree

5 files changed

+50
-21
lines changed

5 files changed

+50
-21
lines changed

‎src/Symfony/Component/HttpClient/EventSourceHttpClient.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function request(string $method, string $url, array $options = []): Respo
7878
try {
7979
$isTimeout = $chunk->isTimeout();
8080

81-
if (null !== $chunk->getInformationalStatus()) {
< 8000 /code>
81+
if (null !== $chunk->getInformationalStatus() || $context->getInfo('canceled')) {
8282
yield $chunk;
8383

8484
return;

‎src/Symfony/Component/HttpClient/Response/AsyncResponse.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ public static function stream(iterable $responses, float $timeout = null, string
247247
}
248248
}
249249

250-
if (!$client) {
250+
if (!$client || !$wrappedResponses) {
251251
return;
252252
}
253253

‎src/Symfony/Component/HttpClient/Response/MockResponse.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,11 @@ public function cancel(): void
105105
{
106106
$this->info['canceled'] = true;
107107
$this->info['error'] = 'Response has been canceled.';
108-
$this->body = null;
108+
try {
109+
$this->body = null;
110+
} catch (TransportException $e) {
111+
// ignore errors when canceling
112+
}
109113
}
110114

111115
/**

‎src/Symfony/Component/HttpClient/RetryableHttpClient.php

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function request(string $method, string $url, array $options = []): Respo
5959
return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, &$retryCount, &$content, &$firstChunk) {
6060
$exception = null;
6161
try {
62-
if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus()) {
62+
if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus() || $context->getInfo('canceled')) {
6363
yield $chunk;
6464

6565
return;
@@ -76,23 +76,14 @@ public function request(string $method, string $url, array $options = []): Respo
7676
}
7777

7878
if (false === $shouldRetry) {
79-
$context->passthru();
80-
if (null !== $firstChunk) {
81-
yield $firstChunk;
82-
yield $context->createChunk($content);
83-
yield $chunk;
84-
} else {
85-
yield $chunk;
86-
}
87-
$content = '';
79+
yield from $this->passthru($context, $firstChunk, $content, $chunk);
8880

8981
return;
9082
}
9183
}
9284
} elseif ($chunk->isFirst()) {
9385
if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
94-
$context->passthru();
95-
yield $chunk;
86+
yield from $this->passthru($context, $firstChunk, $content, $chunk);
9687

9788
return;
9889
}
@@ -105,9 +96,9 @@ public function request(string $method, string $url, array $options = []): Respo
10596
return;
10697
}
10798
} else {
108-
$content .= $chunk->getContent();
109-
11099
if (!$chunk->isLast()) {
100+
$content .= $chunk->getContent();
101+
111102
return;
112103
}
113104

@@ -116,10 +107,7 @@ public function request(string $method, string $url, array $options = []): Respo
116107
}
117108

118109
if (false === $shouldRetry) {
119-
$context->passthru();
120-
yield $firstChunk;
121-
yield $context->createChunk($content);
122-
$content = '';
110+
yield from $this->passthru($context, $firstChunk, $content, $chunk);
123111

124112
return;
125113
}
@@ -159,4 +147,22 @@ private function getDelayFromHeader(array $headers): ?int
159147

160148
return null;
161149
}
150+
151+
private function passthru(AsyncContext $context, ?ChunkInterface $firstChunk, string &$content, ChunkInterface $lastChunk): \Generator
152+
{
153+
$context->passthru();
154+
155+
if (null !== $firstChunk) {
156+
yield $firstChunk;
157+
}
158+
159+
if ('' !== $content) {
160+
$chunk = $context->createChunk($content);
161+
$content = '';
162+
163+
yield $chunk;
164+
}
165+
166+
yield $lastChunk;
167+
}
162168
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPUnit\Framework\TestCase;
66
use Symfony\Component\HttpClient\Exception\ServerException;
7+
use Symfony\Component\HttpClient\HttpClient;
78
use Symfony\Component\HttpClient\MockHttpClient;
89
use Symfony\Component\HttpClient\NativeHttpClient;
910
use Symfony\Component\HttpClient\Response\AsyncContext;
@@ -159,4 +160,22 @@ public function shouldRetry(AsyncContext $context, ?string $responseContent, ?Tr
159160
$this->assertCount(2, $logger->logs);
160161
$this->assertSame('Try #{count} after {delay}ms: Could not resolve host "does.not.exists".', $logger->logs[0]);
161162
}
163+
164+
public function testCancelOnTimeout()
165+
{
166+
$client = HttpClient::create();
167+
168+
if ($client instanceof NativeHttpClient) {
169+
$this->markTestSkipped('NativeHttpClient cannot timeout before receiving headers');
170+
}
171+
172+
$client = new RetryableHttpClient($client);
173+
174+
$response = $client->request('GET', 'https://example.com/');
175+
176+
foreach ($client->stream($response, 0) as $chunk) {
177+
$this->assertTrue($chunk->isTimeout());
178+
$response->cancel();
179+
}
180+
}
162181
}

0 commit comments

Comments
 (0)
0