Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ee93bfb

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #43537 [HttpClient] fix RetryableHttpClient when a response is canceled (nicolas-grekas)
This PR was merged into the 5.3 branch. Discussion ---------- [HttpClient] fix RetryableHttpClient when a response is canceled | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #43390, fix #43391 | License | MIT | Doc PR | - Commits ------- 4f827c3 [HttpClient] fix RetryableHttpClient when a response is canceled
2 parents 95f5209 + 4f827c3 commit ee93bfb
Copy full SHA for ee93bfb

File tree

5 files changed

+50
-21
lines changed
Filter options

5 files changed

+50
-21
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/EventSourceHttpClient.php
+1-1Lines 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()) {
81+
if (null !== $chunk->getInformationalStatus() || $context->getInfo('canceled')) {
8282
yield $chunk;
8383

8484
return;

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/AsyncResponse.php
+1-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/MockResponse.php
+5-1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/RetryableHttpClient.php
+24-18Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php
+19Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.