8000 Parameterize list of retryed Http methods · symfony/symfony@68a37b1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 68a37b1

Browse files
jderussejderusse
committed
Parameterize list of retryed Http methods
1 parent 81833b6 commit 68a37b1

File tree

8 files changed

+128
-50
lines changed

8 files changed

+128
-50
lines changed
Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,8 +1644,8 @@ private function addHttpClientRetrySection()
16441644
if (isset($v['backoff_service']) && (isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']))) {
16451645
throw new \InvalidArgumentException('The "backoff_service" option cannot be used along with the "delay", "multiplier" or "max_delay" options.');
16461646
}
1647-
if (isset($v['decider_service']) && (isset($v['http_codes']))) {
1648-
throw new \InvalidArgumentException('The "decider_service" option cannot be used along with the "http_codes" options.');
1647+
if (isset($v['decider_service']) && (isset($v['http_codes']) || isset($v['idempotent_http_codes']))) {
1648+
throw new \InvalidArgumentException('The "decider_service" option cannot be used along with the "http_codes" or "idempotent_http_codes" options.');
16491649
}
16501650

16511651
return $v;
@@ -1663,8 +1663,32 @@ private function addHttpClientRetrySection()
16631663
})
16641664
->end()
16651665
->prototype('integer')->end()
1666-
->info('A list of HTTP status code that triggers a retry')
1667-
->defaultValue([423, 425, 429, 500, 502, 503, 504, 507, 510])
1666+
->info('A list of HTTP status code that triggers a retry regardless the method')
1667+
->defaultValue([423, 425, 429, 503])
1668+
->end()
1669+
->arrayNode('idempotent_http_codes')
1670+
->performNoDeepMerging()
1671+
->beforeNormalization()
1672+
->ifArray()
1673+
->then(function ($v) {
1674+
return array_filter(array_values($v));
1675+
})
1676+
->end()
1677+
->prototype('integer')->end()
1678+
->info('A list of HTTP status code that triggers a retry on idempotent methods')
1679+
->defaultValue([500, 502, 504, 507, 510])
1680+
->end()
1681+
->arrayNode('methods')
1682+
->performNoDeepMerging()
1683+
->beforeNormalization()
1684+
->ifArray()
1685+
->then(function ($v) {
1686+
return array_filter(array_values($v));
1687+
})
1688+
->end()
1689+
->prototype('scalar')->end()
1690+
->info('A list of HTTP request method code that are taken into account')
1691+
->defaultValue(['*'])
16681692
->end()
16691693
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
16701694
->integerNode('delay')->defaultValue(1000)->min(0)->info('Time in ms to delay (or the initial value when multiplier is used)')->end()

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2086,7 +2086,8 @@ private function registerHttpClientRetry(array $retryOptions, string $name, Cont
20862086
$retryServiceId = $name.'.retry.decider';
20872087
$retryDefinition = new ChildDefinition('http_client.retry.abstract_httpstatuscode_decider');
20882088
$retryDefinition
2089-
->replaceArgument(0, $retryOptions['http_codes']);
2089+
->replaceArgument(0, $retryOptions['http_codes'])
2090+
->replaceArgument(1, $retryOptions['idempotent_http_codes']);
20902091
$container->setDefinition($retryServiceId, $retryDefinition);
20912092

20922093
$deciderReference = new Reference($retryServiceId);

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/http_client.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
->abstract()
6464
->args([
6565
abstract_arg('http codes'),
66+
abstract_arg('idempotent http codes'),
6667
])
6768
;
6869
};

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,36 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
15+
1416
/**
1517
* Decides to retry the request when HTTP status codes belong to the given list of codes.
1618
*
1719
* @author Jérémy Derussé <jeremy@derusse.com>
1820
*/
1921
final class HttpStatusCodeDecider implements RetryDeciderInterface
2022
{
23+
private const IDEMPOTENT_METHODS = ['GET', 'HEAD', 'PUT', 'DELETE', 'OPTIONS', 'TRACE', 'CONNECT'];
24+
2125
private $statusCodes;
26+
private $idempotentStatusCodes;
2227

2328
/**
24-
* @param array $statusCodes List of HTTP status codes that trigger a retry
29+
* @param array $statusCodes List of HTTP status codes that trigger a retry regardless the method
30+
* @param array $idempotentStatusCodes List of HTTP status codes that trigger a retry on idempotent methods
2531
*/
26-
public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503, 504, 507, 510])
32+
public function __construct(array $statusCodes = [423, 425, 429, 503], array $idempotentStatusCodes = [500, 502, 504, 507, 510])
2733
{
2834
$this->statusCodes = $statusCodes;
35+
$this->idempotentStatusCodes = array_unique(array_merge($statusCodes, $idempotentStatusCodes));
2936
}
3037

31-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
38+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
3239
{
33-
return \in_array($responseStatusCode, $this->statusCodes, true);
40+
if (\in_array($responseStatusCode, $this->statusCodes, true)) {
41+
return true;
42+
}
43+
44+
return \in_array(strtoupper($requestMethod), self::IDEMPOTENT_METHODS, true) && (null !== $exception || \in_array($responseStatusCode, $this->idempotentStatusCodes, true));
3445
}
3546
}

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

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

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
15+
1416
/**
1517
* @author Jérémy Derussé <jeremy@derusse.com>
1618
*/
@@ -23,5 +25,5 @@ interface RetryDeciderInterface
2325
*
2426
* @return ?bool Returns null to signal that the body is required to take a decision
2527
*/
26-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
28+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool;
2729
}

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

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,42 +74,59 @@ public function request(string $method, string $url, array $options = []): Respo
7474

7575
$statusCode = $context->getStatusCode();
7676
$headers = $context->getHeaders();
77-
if (null === $exception) {
78-
if ($chunk->isFirst()) {
79-
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null);
77+
if (null !== $exception) {
78+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, null, $exception);
79+
if (null === $shouldRetry) {
80+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with an exception.', \get_class($this->decider)));
81+
}
8082

81-
if (false === $shouldRetry) {
82-
$context->passthru();
83+
if (false === $shouldRetry) {
84+
$context->passthru();
85+
if (null !== $firstChunk) {
86+
yield $firstChunk;
87+
yield $context->createChunk($content);
88+
yield $chunk;
89+
} else {
8390
yield $chunk;
84-
85-
return;
8691
}
92+
$content = '';
8793

88-
// Decider need body to decide
89-
if (null === $shouldRetry) {
90-
$firstChunk = $chunk;
91-
$content = '';
94+
return;
95+
}
96+
} elseif ($chunk->isFirst()) {
97+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, null, null);
9298

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);
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-
}
99+
if (false === $shouldRetry) {
100+
$context->passthru();
101+
yield $chunk;
104102

105-
if (false === $shouldRetry) {
106-
$context->passthru();
107-
yield $firstChunk;
108-
yield $context->createChunk($content);
109-
$content = '';
103+
return;
104+
}
110105

111-
return;
112-
}
106+
// Decider need body to decide
107+
if (null === $shouldRetry) {
108+
$firstChunk = $chunk;
109+
$content = '';
110+
111+
return;
112+
}
113+
} else {
114+
$content .= $chunk->getContent();
115+
if (!$chunk->isLast()) {
116+
return;
117+
}
118+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, $content, null);
119+
if (null === $shouldRetry) {
120+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', \get_class($this->decider)));
121+
}
122+
123+
if (false === $shouldRetry) {
124+
$context->passthru();
125+
yield $firstChunk;
126+
yield $context->createChunk($content);
127+
$content = '';
128+
129+
return;
113130
}
114131
}
115132

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

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

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\HttpClient\Exception\TransportException;
1516
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
17+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1618

1719
class HttpStatusCodeDeciderTest extends TestCase
1820
{
19-
public function testShouldRetryStatusCode()
21+
/**
22+
* @dataProvider provideRetryable
23+
*/
24+
public function testShouldRetry(string $method, int $code, ?TransportExceptionInterface $exception)
2025
{
21-
$decider = new HttpStatusCodeDecider([500]);
26+
$decider = new HttpStatusCodeDecider();
2227

23-
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null));
28+
$this->assertTrue($decider->shouldRetry(1, $method, 'http://example.com/', [], $code, [], null, $exception));
2429
}
2530

26-
public function testIsNotRetryableOk()
31+
public function provideRetryable(): iterable
2732
{
28-
$decider = new HttpStatusCodeDecider([500]);
33+
yield ['GET', 200, new TransportException()];
34+
yield ['GET', 500, null];
35+
yield ['POST', 429, null];
36+
}
37+
38+
/**
39+
* @dataProvider provideNotRetryable
40+
*/
41+
public function testShouldNotRetry(string $method, int $code, ?TransportExceptionInterface $exception)
42+
{
43+
$decider = new HttpStatusCodeDecider();
2944

30-
self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null));
45+
$this->assertFalse($decider->shouldRetry(1, $method, 'http://example.com/', [], $code, [], null, $exception));
46+
}
47+
48+
public function provideNotRetryable(): iterable
49+
{
50+
yield ['POST', 200, new TransportException()];
51+
yield ['POST', 500, null];
3152
}
3253
}

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
1111
use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
1212
use Symfony\Component\HttpClient\RetryableHttpClient;
13+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1314

1415
class RetryableHttpClientTest extends TestCase
1516
{
@@ -20,7 +21,7 @@ public function testRetryOnError()
2021
new MockResponse('', ['http_code' => 500]),
2122
new MockResponse('', ['http_code' => 200]),
2223
]),
23-
new HttpStatusCodeDecider([500]),
24+
new HttpStatusCodeDecider(['*'], [500]),
2425
new ExponentialBackOff(0),
2526
1
2627
);
@@ -38,7 +39,7 @@ public function testRetryRespectStrategy()
3839
new MockResponse('', ['http_code' => 500]),
3940
new MockResponse('', ['http_code' => 200]),
4041
]),
41-
new HttpStatusCodeDecider([500]),
42+
new HttpStatusCodeDecider(['*'], [500]),
4243
new ExponentialBackOff(0),
4344
1
4445
);
@@ -57,7 +58,7 @@ public function testRetryWithBody()
5758
new MockResponse('', ['http_code' => 200]),
5859
]),
5960
new class() implements RetryDeciderInterface {
60-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
61+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
6162
{
6263
return null === $responseContent ? null : 200 !== $responseCode;
6364
}
@@ -79,7 +80,7 @@ public function testRetryWithBodyInvalid()
7980
new MockResponse('', ['http_code' => 200]),
8081
]),
8182
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+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool
8384
{
8485
return null;
8586
}
@@ -100,7 +101,7 @@ public function testStreamNoRetry()
100101
new MockHttpClient([
101102
new MockResponse('', ['http_code' => 500]),
102103
]),
103-
new HttpStatusCodeDecider([500]),
104+
new HttpStatusCod 48DA eDecider(['*'], [500]),
104105
new ExponentialBackOff(0),
105106
0
106107
);

0 commit comments

Comments
 (0)
0