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 0ffaa0a

Browse filesBrowse files
bug #53889 [HttpClient] Make retry strategy work again (Nyholm)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [HttpClient] Make retry strategy work again | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #53886 | License | MIT PR #53506 accidentally disabled the retry functionality. I reverted that PR and added a small test to make sure this does not happen again. Thank you `@ldebrouwer` for reporting this. FYI `@nicolas`-grekas `@rmikalkenas`, I will try to find an other solution to fix #52587. But I'll do that in a separate PR to get a quick merge on this one. Commits ------- 9a5797d [HttpClient] Make retry strategy work again
2 parents 5e773a8 + 9a5797d commit 0ffaa0a
Copy full SHA for 0ffaa0a

File tree

2 files changed

+25
-19
lines changed
Filter options

2 files changed

+25
-19
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/AsyncResponse.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ public function __construct(HttpClientInterface $client, string $method, string
6565
while (true) {
6666
foreach (self::stream([$response], $timeout) as $chunk) {
6767
if ($chunk->isTimeout() && $response->passthru) {
68-
foreach (self::passthru($response->client, $response, new ErrorChunk($response->offset, $chunk->getError())) as $chunk) {
68+
// Timeouts thrown during initialization are transport errors
69+
foreach (self::passthru($response->client, $response, new ErrorChunk($response->offset, new TransportException($chunk->getError()))) as $chunk) {
6970
if ($chunk->isFirst()) {
7071
return false;
7172
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php
+23-18Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\HttpClient\Exception\ServerException;
16-
use Symfony\Component\HttpClient\Exception\TimeoutException;
16+
use Symfony\Component\HttpClient\Exception\TransportException;
1717
use Symfony\Component\HttpClient\HttpClient;
1818
use Symfony\Component\HttpClient\MockHttpClient;
1919
use Symfony\Component\HttpClient\NativeHttpClient;
2020
use Symfony\Component\HttpClient\Response\AsyncContext;
2121
use Symfony\Component\HttpClient\Response\MockResponse;
2222
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
23+
use Symfony\Component\HttpClient\Retry\RetryStrategyInterface;
2324
use Symfony\Component\HttpClient\RetryableHttpClient;
2425
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
2526
use Symfony\Contracts\HttpClient\Test\TestHttpServer;
@@ -247,32 +248,36 @@ public function testRetryOnErrorAssertContent()
247248
self::assertSame('Test out content', $response->getContent(), 'Content should be buffered');
248249
}
249250

250-
/**
251-
* @testWith ["GET"]
252-
* ["POST"]
253-
* ["PUT"]
254-
* ["PATCH"]
255-
* ["DELETE"]
256-
*/
257-
public function testRetryOnHeaderTimeout(string $method)
251+
public function testRetryOnTimeout()
258252
{
259253
$client = HttpClient::create();
260254

261-
if ($client instanceof NativeHttpClient) {
262-
$this->markTestSkipped('NativeHttpClient cannot timeout before receiving headers');
263-
}
264-
265255
TestHttpServer::start();
266256

267-
$client = new RetryableHttpClient($client);
268-
$response = $client->request($method, 'http://localhost:8057/timeout-header', ['timeout' => 0.1]);
257+
$strategy = new class() implements RetryStrategyInterface {
258+
public $isCalled = false;
259+
260+
public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
261+
{
262+
$this->isCalled = true;
263+
264+
return false;
265+
}
266+
267+
public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int
268+
{
269+
return 0;
270+
}
271+
};
272+
$client = new RetryableHttpClient($client, $strategy);
273+
$response = $client->request('GET', 'http://localhost:8057/timeout-header', ['timeout' => 0.1]);
269274

270275
try {
271276
$response->getStatusCode();
272-
$this->fail(TimeoutException::class.' expected');
273-
} catch (TimeoutException $e) {
277+
$this->fail(TransportException::class.' expected');
278+
} catch (TransportException $e) {
274279
}
275280

276-
$this->assertSame('Idle timeout reached for "http://localhost:8057/timeout-header".', $response->getInfo('error'));
281+
$this->assertTrue($strategy->isCalled, 'The HTTP retry strategy should be called');
277282
}
278283
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.