From 9a5797d9624d8db372e207ce924a0bf2e811d62e Mon Sep 17 00:00:00 2001 From: Nyholm Date: Fri, 9 Feb 2024 19:38:24 -0800 Subject: [PATCH] [HttpClient] Make retry strategy work again --- .../HttpClient/Response/AsyncResponse.php | 3 +- .../Tests/RetryableHttpClientTest.php | 41 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php index ae0d004f7651b..890e2e96743c8 100644 --- a/src/Symfony/Component/HttpClient/Response/AsyncResponse.php +++ b/src/Symfony/Component/HttpClient/Response/AsyncResponse.php @@ -65,7 +65,8 @@ public function __construct(HttpClientInterface $client, string $method, string while (true) { foreach (self::stream([$response], $timeout) as $chunk) { if ($chunk->isTimeout() && $response->passthru) { - foreach (self::passthru($response->client, $response, new ErrorChunk($response->offset, $chunk->getError())) as $chunk) { + // Timeouts thrown during initialization are transport errors + foreach (self::passthru($response->client, $response, new ErrorChunk($response->offset, new TransportException($chunk->getError()))) as $chunk) { if ($chunk->isFirst()) { return false; } diff --git a/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php index 0e4befafcf4fb..9edf41318555e 100644 --- a/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php +++ b/src/Symfony/Component/HttpClient/Tests/RetryableHttpClientTest.php @@ -13,13 +13,14 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpClient\Exception\ServerException; -use Symfony\Component\HttpClient\Exception\TimeoutException; +use Symfony\Component\HttpClient\Exception\TransportException; use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\NativeHttpClient; use Symfony\Component\HttpClient\Response\AsyncContext; use Symfony\Component\HttpClient\Response\MockResponse; use Symfony\Component\HttpClient\Retry\GenericRetryStrategy; +use Symfony\Component\HttpClient\Retry\RetryStrategyInterface; use Symfony\Component\HttpClient\RetryableHttpClient; use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; use Symfony\Contracts\HttpClient\Test\TestHttpServer; @@ -247,32 +248,36 @@ public function testRetryOnErrorAssertContent() self::assertSame('Test out content', $response->getContent(), 'Content should be buffered'); } - /** - * @testWith ["GET"] - * ["POST"] - * ["PUT"] - * ["PATCH"] - * ["DELETE"] - */ - public function testRetryOnHeaderTimeout(string $method) + public function testRetryOnTimeout() { $client = HttpClient::create(); - if ($client instanceof NativeHttpClient) { - $this->markTestSkipped('NativeHttpClient cannot timeout before receiving headers'); - } - TestHttpServer::start(); - $client = new RetryableHttpClient($client); - $response = $client->request($method, 'http://localhost:8057/timeout-header', ['timeout' => 0.1]); + $strategy = new class() implements RetryStrategyInterface { + public $isCalled = false; + + public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool + { + $this->isCalled = true; + + return false; + } + + public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int + { + return 0; + } + }; + $client = new RetryableHttpClient($client, $strategy); + $response = $client->request('GET', 'http://localhost:8057/timeout-header', ['timeout' => 0.1]); try { $response->getStatusCode(); - $this->fail(TimeoutException::class.' expected'); - } catch (TimeoutException $e) { + $this->fail(TransportException::class.' expected'); + } catch (TransportException $e) { } - $this->assertSame('Idle timeout reached for "http://localhost:8057/timeout-header".', $response->getInfo('error')); + $this->assertTrue($strategy->isCalled, 'The HTTP retry strategy should be called'); } }