Skip to content

Commit 321be58

Browse files
jderussejderusse
committed
[HttpClient] provide response body to the RetryDecider
1 parent af8ad34 commit 321be58

File tree

9 files changed

+117
-48
lines changed

9 files changed

+117
-48
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ public static function stream(iterable $responses, float $timeout = null, string
219219
continue;
220220
}
221221

222+
if (null === $chunk->getError() && $chunk->isFirst()) {
223+
// Ensure no exception is thrown on destruct for the wrapped response
224+
$r->response->getStatusCode();
225+
}
226+
222227
foreach (self::passthru($r->client, $r, $chunk, $asyncMap) as $chunk) {
223228
yield $r => $chunk;
224229
}

‎src/Symfony/Component/HttpClient/Retry/ExponentialBackOff.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Component\Messenger\Exception\InvalidArgumentException;
15-
use Symfony\Contracts\HttpClient\ResponseInterface;
14+
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
15+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1616

1717
/**
1818
* A retry backOff with a constant or exponential retry delay.
@@ -57,7 +57,7 @@ public function __construct(int $delayMilliseconds = 1000, float $multiplier = 2
5757
$this->maxDelayMilliseconds = $maxDelayMilliseconds;
5858
}
5959

60-
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $partialResponse, \Throwable $throwable = null): int
60+
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int
6161
{
6262
$delay = $this->delayMilliseconds * $this->multiplier ** $retryCount;
6363

‎src/Symfony/Component/HttpClient/Retry/HttpStatusCodeDecider.php

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,6 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
15-
use Symfony\Contracts\HttpClient\ResponseInterface;
16-
1714
/**
1815
* Decides to retry the request when HTTP status codes belong to the given list of codes.
1916
*
@@ -31,12 +28,8 @@ public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503,
3128
$this->statusCodes = $statusCodes;
3229
}
3330

34-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $partialResponse, \Throwable $throwable = null): bool
31+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
3532
{
36-
if ($throwable instanceof TransportExceptionInterface) {
37-
return true;
38-
}
39-
40-
return \in_array($partialResponse->getStatusCode(), $this->statusCodes, true);
33+
return \in_array($responseStatusCode, $this->statusCodes, true);
4134
}
4235
}

‎src/Symfony/Component/HttpClient/Retry/RetryBackOffInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Contracts\HttpClient\ResponseInterface;
14+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1515

1616
/**
1717
* @author Jérémy Derussé <jeremy@derusse.com>
@@ -21,5 +21,5 @@ interface RetryBackOffInterface
2121
/**
2222
* Returns the time to wait in milliseconds.
2323
*/
24-
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $partialResponse, \Throwable $throwable = null): int;
24+
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int;
2525
}

‎src/Symfony/Component/HttpClient/Retry/RetryDeciderInterface.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,17 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Contracts\HttpClient\ResponseInterface;
15-
1614
/**
1715
* @author Jérémy Derussé <jeremy@derusse.com>
1816
*/
1917
interface RetryDeciderInterface
2018
{
2119
/**
2220
* Returns whether the request should be retried.
21+
*
22+
* @param ?string $responseContent Null is passed when the body did not arrive yet
23+
*
24+
* @return ?bool Returns null to signal that the body is required to take a decision
2325
*/
24-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, ResponseInterface $partialResponse, \Throwable $throwable = null): bool;
26+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
2527
}

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

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Psr\Log\NullLogger;
1616
use Symfony\Component\HttpClient\Response\AsyncContext;
1717
use Symfony\Component\HttpClient\Response\AsyncResponse;
18-
use Symfony\Component\HttpClient\Response\MockResponse;
1918
use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
2019
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
2120
use Symfony\Component\HttpClient\Retry\RetryBackOffInterface;
@@ -53,41 +52,71 @@ public function __construct(HttpClientInterface $client, RetryDeciderInterface $
5352

5453
public function request(string $method, string $url, array $options = []): ResponseInterface
5554
{
55+
if ($this->maxRetries <= 0) {
56+
return $this->client->request($method, $url, $options);
57+
}
58+
5659
$retryCount = 0;
60+
$content = '';
61+
$firstChunk = null;
5762

58-
return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, &$retryCount) {
63+
return new AsyncResponse($this->client, $method, $url, $options, function (ChunkInterface $chunk, AsyncContext $context) use ($method, $url, $options, &$retryCount, &$content, &$firstChunk) {
5964
$exception = null;
6065
try {
6166
if ($chunk->isTimeout() || null !== $chunk->getInformationalStatus()) {
6267
yield $chunk;
6368

6469
return;
6570
}
66-
67-
// only retry first chunk
68-
if (!$chunk->isFirst()) {
69-
$context->passthru();
70-
yield $chunk;
71-
72-
return;
73-
}
7471
} catch (TransportExceptionInterface $exception) {
7572
// catch TransportExceptionInterface to send it to strategy.
7673
}
7774

7875
$statusCode = $context->getStatusCode();
7976
$headers = $context->getHeaders();
80-
if ($retryCount >= $this->maxRetries || !$this->decider->shouldRetry($method, $url, $options, $partialResponse = new MockResponse($context->getContent(), ['http_code' => $statusCode, 'headers' => $headers]), $exception)) {
81-
$context->passthru();
82-
yield $chunk;
83-
84-
return;
77+
if (null === $exception) {
78+
if ($chunk->isFirst()) {
79+
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null);
80+
81+
if (false === $shouldRetry) {
82+
$context->passthru();
83+
yield $chunk;
84+
85+
return;
86+
}
87+
88+
// Decider need body to decide
89+
if (null === $shouldRetry) {
90+
$firstChunk = $chunk;
91+
$content = '';
92+
93+
return;
94+
}
95+
} else {
96+
$content .= $chunk->getContent();
97+
if (!$chunk->isLast()) {
98+
return;
99+
}
100+
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content, null);
101+
if (null === $shouldRetry) {
102+
throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with a body.', \get_class($this->decider)));
103+
}
104+
105+
if (false === $shouldRetry) {
106+
$context->passthru();
107+
yield $firstChunk;
108+
yield $context->createChunk($content);
109+
$content = '';
110+
111+
return;
112+
}
113+
}
85114
}
86115

87116
$context->setInfo('retry_count', $retryCount);
88117
$context->getResponse()->cancel();
89118

90-
$delay = $this->getDelayFromHeader($headers) ?? $this->strategy->getDelay($retryCount, $method, $url, $options, $partialResponse, $exception);
119+
$delay = $this->getDelayFromHeader($headers) ?? $this->strategy->getDelay($retryCount, $method, $url, $options, $statusCode, $headers, $chunk instanceof LastChunk ? $content : null, $exception);
91120
++$retryCount;
92121

93122
$this->logger->info('Error returned by the server. Retrying #{retryCount} using {delay} ms delay: '.($exception ? $exception->getMessage() : 'StatusCode: '.$statusCode), [
@@ -97,6 +126,10 @@ public function request(string $method, string $url, array $options = []): Respo
97126

98127
$context->replaceRequest($method, $url, $options);
99128
$context->pause($delay / 1000);
129+
130+
if ($retryCount >= $this->maxRetries) {
131+
$context->passthru();
132+
}
100133
});
101134
}
102135

‎src/Symfony/Component/HttpClient/Tests/Retry/ExponentialBackOffTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
namespace Symfony\Component\HttpClient\Tests\Retry;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\HttpClient\Response\MockResponse;
1615
use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
1716

1817
class ExponentialBackOffTest extends TestCase
@@ -24,7 +23,7 @@ public function testGetDelay(int $delay, int $multiplier, int $maxDelay, int $pr
2423
{
2524
$backOff = new ExponentialBackOff($delay, $multiplier, $maxDelay);
2625

27-
self::assertSame($expectedDelay, $backOff->getDelay($previousRetries, 'GET', 'http://example.com/', [], new MockResponse(), null));
26+
self::assertSame($expectedDelay, $backOff->getDelay($previousRetries, 'GET', 'http://example.com/', [], 200, [], null, null));
2827
}
2928

3029
public function provideDelay(): iterable

‎src/Symfony/Component/HttpClient/Tests/Retry/HttpStatusCodeDeciderTest.php

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,21 @@
1212
namespace Symfony\Component\HttpClient\Tests\Retry;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\HttpClient\Exception\TransportException;
16-
use Symfony\Component\HttpClient\Response\MockResponse;
1715
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
1816

1917
class HttpStatusCodeDeciderTest extends TestCase
2018
{
21-
public function testShouldRetryException()
22-
{
23-
$decider = new HttpStatusCodeDecider([500]);
24-
25-
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], new MockResponse(), new TransportException()));
26-
}
27-
2819
public function testShouldRetryStatusCode()
2920
{
3021
$decider = new HttpStatusCodeDecider([500]);
3122

32-
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], new MockResponse('', ['http_code' => 500]), null));
23+
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null));
3324
}
3425

3526
public function testIsNotRetryableOk()
3627
{
3728
$decider = new HttpStatusCodeDecider([500]);
3829

39-
self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], new MockResponse(''), null));
30+
self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null));
4031
}
4132
}

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

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88
use Symfony\Component\HttpClient\Response\MockResponse;
99
use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
1010
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
11+
use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
1112
use Symfony\Component\HttpClient\RetryableHttpClient;
1213

1314
class RetryableHttpClientTest extends TestCase
1415
{
15-
public function testRetryOnError(): void
16+
public function testRetryOnError()
1617
{
1718
$client = new RetryableHttpClient(
1819
new MockHttpClient([
@@ -29,7 +30,7 @@ public function testRetryOnError(): void
2930
self::assertSame(200, $response->getStatusCode());
3031
}
3132

32-
public function testRetryRespectStrategy(): void
33+
public function testRetryRespectStrategy()
3334
{
3435
$client = new RetryableHttpClient(
3536
new MockHttpClient([
@@ -47,4 +48,49 @@ public function testRetryRespectStrategy(): void
4748
$this->expectException(ServerException::class);
4849
$response->getHeaders();
4950
}
51+
52+
public function testRetryWithBody()
53+
{
54+
$client = new RetryableHttpClient(
55+
new MockHttpClient([
56+
new MockResponse('', ['http_code' => 500]),
57+
new MockResponse('', ['http_code' => 200]),
58+
]),
59+
new class() implements RetryDeciderInterface {
60+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
61+
{
62+
return null === $responseContent ? null : 200 !== $responseCode;
63+
}
64+
},
65+
new ExponentialBackOff(0),
66+
1
67+
);
68+
69+
$response = $client->request('GET', 'http://example.com/foo-bar');
70+
71+
self::assertSame(200, $response->getStatusCode());
72+
}
73+
74+
public function testRetryWithBodyInvalid()
75+
{
76+
$client = new RetryableHttpClient(
77+
new MockHttpClient([
78+
new MockResponse('', ['http_code' => 500]),
79+
new MockResponse('', ['http_code' => 200]),
80+
]),
81+
new class() implements RetryDeciderInterface {
82+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, \Throwable $throwable = null): ?bool
83+
{
84+
return null;
85+
}
86+
},
87+
new ExponentialBackOff(0),
88+
1
89+
);
90+
91+
$response = $client->request('GET', 'http://example.com/foo-bar');
92+
93+
$this->expectExceptionMessageMatches('/must not return null when called with a body/');
94+
$response->getHeaders();
95+
}
5096
}

0 commit comments

Comments
 (0)