8000 bug #53889 [HttpClient] Make retry strategy work again (Nyholm) · symfony/symfony@0ffaa0a · GitHub
[go: up one dir, main page]

Skip to content

Commit 0ffaa0a

Browse 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

File tree

2 files changed

+25
-19
lines changed

2 files changed

+25
-19
lines changed

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

+2-1
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

+23-18
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