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

Commit 2c1ee99

Browse files
jderussejderusse
committed
Parameterize list of retryed Http methods
1 parent f52f090 commit 2c1ee99

File tree

10 files changed

+171
-46
lines changed

10 files changed

+171
-46
lines changed

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

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,13 +1654,43 @@ private function addHttpClientRetrySection()
16541654
->performNoDeepMerging()
16551655
->beforeNormalization()
16561656
->ifArray()
1657-
->then(function ($v) {
1658-
return array_filter(array_values($v));
1657+
->then(static function ($v) {
1658+
$list = [];
1659+
foreach ($v as $key => $val) {
1660+
if (is_numeric($val)) {
1661+
$list[] = ['code' => $val];
1662+
} elseif (\is_array($val)) {
1663+
if (isset($val['code']) || isset($val['methods'])) {
1664+
$list[] = $val;
1665+
} else {
1666+
$list[] = ['code' => $key, 'methods' => $val];
1667+
}
1668+
} elseif (true === $val || null === $val) {
1669+
$list[] = ['code' => $key];
1670+
}
1671+
}
1672+
1673+
return $list;
16591674
})
16601675
->end()
1661-
->prototype('integer')->end()
1676+
->useAttributeAsKey('code')
1677+
->arrayPrototype()
1678+
->fixXmlConfig('method')
1679+
->children()
1680+
->integerNode('code')->end()
1681+
->arrayNode('methods')
1682+
->beforeNormalization()
1683+
->ifArray()
1684+
->then(function ($v) {
1685+
return array_map('strtoupper', $v);
1686+
})
1687+
->end()
1688+
->prototype('scalar')->end()
1689+
->info('A list of HTTP method that triggers a retry for this status code. When empty, all method are retried.')
1690+
->end()
1691+
->end()
1692+
->end()
16621693
->info('A list of HTTP status code that triggers a retry')
1663-
->defaultValue([423, 425, 429, 500, 502, 503, 504, 507, 510])
16641694
->end()
16651695
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
16661696
->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: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
use Symfony\Component\Form\FormTypeGuesserInterface 10000 ;
6565
use Symfony\Component\Form\FormTypeInterface;
6666
use Symfony\Component\HttpClient\MockHttpClient;
67+
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
6768
use Symfony\Component\HttpClient\RetryableHttpClient;
6869
use Symfony\Component\HttpClient\ScopingHttpClient;
6970
use Symfony\Component\HttpFoundation\Request;
@@ -2072,8 +2073,17 @@ private function registerRetryableHttpClient(array $options, string $name, Conta
20722073
$retryStrategy = new Reference($options['retry_strategy']);
20732074
} else {
20742075
$retryStrategy = new ChildDefinition('http_client.abstract_retry_strategy');
2076+
$codes = [];
2077+
foreach ($options['http_codes'] as $code => $codeOptions) {
2078+
if ($codeOptions['methods']) {
2079+
$codes[$code] = $codeOptions['methods'];
2080+
} else {
2081+
$codes[] = $code;
2082+
}
2083+
}
2084+
20752085
$retryStrategy
2076-
->replaceArgument(0, $options['http_codes'])
2086+
->replaceArgument(0, $codes ?: GenericRetryStrategy::DEFAULT_RETRY_STATUS_CODES)
20772087
->replaceArgument(1, $options['delay'])
20782088
->replaceArgument(2, $options['multiplier'])
20792089
->replaceArgument(3, $options['max_delay'])

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@
578578

579579
<xsd:complexType name="http_client_retry_failed">
580580
<xsd:sequence>
581-
<xsd:element name="http-code" type="xsd:integer" minOccurs="0" maxOccurs="unbounded" />
581+
<xsd:element name="http-code" type="http_client_retry_code" minOccurs="0" maxOccurs="unbounded" />
582582
</xsd:sequence>
583583
<xsd:attribute name="enabled" type="xsd:boolean" />
584584
<xsd:attribute name="retry-strategy" type="xsd:string" />
@@ -590,6 +590,14 @@
590590
<xsd:attribute name="response_header" type="xsd:boolean" />
591591
</xsd:complexType>
592592

593+
<xsd:complexType name="http_client_retry_code">
594+
<xsd:sequence>
595+
<xsd:element name="method" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
596+
</xsd:sequence>
597+
<xsd:attribute name="code" type="xsd:integer" />
598+
<xsd:attribute name="retry" type="xsd:boolean" />
599+
</xsd:complexType>
600+
593601
<xsd:complexType name="http_query" mixed="true">
594602
<xsd:attribute name="key" type="xsd:string" />
595603
</xsd:complexType>

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/http_client_retry.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
'default_options' => [
66
'retry_failed' => [
77
'retry_strategy' => null,
8-
'http_codes' => [429, 500],
8+
'http_codes' => [429, 500 => ['GET', 'HEAD']],
99
'max_retries' => 2,
1010
'delay' => 100,
1111
'multiplier' => 2,

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/http_client_retry.xml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414
max-retries="2"
1515
multiplier="2"
1616
jitter="0.3">
17-
<framework:http-code>429</framework:http-code>
18-
<framework:http-code>500</framework:http-code>
17+
<framework:http-code code="429"/>
18+
<framework:http-code code="500">
19+
<framework:method>GET</framework:method>
20+
<framework:method>HEAD</framework:method>
21+
</framework:http-code>
1922
</framework:retry-failed>
2023
</framework:default-options>
2124
<framework:scoped-client name="foo" base-uri="http://example.com">

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/http_client_retry.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ framework:
33
default_options:
44
retry_failed:
55
retry_strategy: null
6-
http_codes: [429, 500]
6+
http_codes:
7+
429: true
8+
500: ['GET', 'HEAD']
79
max_retries: 2
810
delay: 100
911
multiplier: 2

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1500,7 +1500,7 @@ public function testHttpClientRetry()
15001500
}
15011501
$container = $this->createContainerFromFile('http_client_retry');
15021502

1503-
$this->assertSame([429, 500], $container->getDefinition('http_client.retry_strategy')->getArgument(0));
1503+
$this->assertSame([429, 500 => ['GET', 'HEAD']], $container->getDefinition('http_client.retry_strategy')->getArgument(0));
15041504
$this->assertSame(100, $container->getDefinition('http_client.retry_strategy')->getArgument(1));
15051505
$this->assertSame(2, $container->getDefinition('http_client.retry_strategy')->getArgument(2));
15061506
$this->assertSame(0, $container->getDefinition('http_client.retry_strategy')->getArgument(3));

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

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

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14+
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1415
use Symfony\Component\HttpClient\Response\AsyncContext;
1516
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1617

@@ -21,7 +22,19 @@
2122
*/
2223
class GenericRetryStrategy implements RetryStrategyInterface
2324
{
24-
public const DEFAULT_RETRY_STATUS_CODES = [423, 425, 429, 500, 502, 503, 504, 507, 510];
25+
public const IDEMPOTENT_METHODS = ['GET', 'HEAD', 'PUT', 'DELETE', 'OPTIONS', 'TRACE'];
26+
public const DEFAULT_RETRY_STATUS_CODES = [
27+
0 => self::IDEMPOTENT_METHODS,
28+
423,
29+
425,
30+
429,
31+
500 => self::IDEMPOTENT_METHODS,
32+
502,
33+
503,
34+
504 => self::IDEMPOTENT_METHODS,
35+
507 => self::IDEMPOTENT_METHODS,
36+
510 => self::IDEMPOTENT_METHODS,
37+
];
2538

2639
private $statusCodes;
2740
private $delayMs;
@@ -63,7 +76,25 @@ public function __construct(array $statusCodes = self::DEFAULT_RETRY_STATUS_CODE
6376

6477
public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
6578
{
66-
return \in_array($context->getStatusCode(), $this->statusCodes, true);
79+
$statusCode = $context->getStatusCode();
80+
if (\in_array($statusCode, $this->statusCodes, true)) {
81+
return true;
82+
}
83+
if (isset($this->statusCodes[$statusCode]) && \is_array($this->statusCodes[$statusCode])) {
84+
return \in_array($context->getInfo('http_method'), $this->statusCodes[$statusCode], true);
85+
}
86+
if (null === $exception) {
87+
return false;
88+
}
89+
90+
if (\in_array(0, $this->statusCodes, true)) {
91+
return true;
92+
}
93+
if (isset($this->statusCodes[0]) && \is_array($this->statusCodes[0])) {
94+
return \in_array($context->getInfo('http_method'), $this->statusCodes[0], true);
95+
}
96+
97+
return false;
6798
}
6899

69100
public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int

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

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -68,44 +68,63 @@ public function request(string $method, string $url, array $options = []): Respo
6868
// catch TransportExceptionInterface to send it to the strategy
6969
$context->setInfo('retry_count', $retryCount);
7070
}
71+
if (null !== $exception) {
72+
// always retry request that fail to resolve DNS
73+
if ('' !== $context->getInfo('primary_ip')) {
74+
$shouldRetry = $this->strategy->shouldRetry($context, null, $exception);
75+
if (null === $shouldRetry) {
76+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with an exception.', \get_class($this->decider)));
77+
}
7178

72-
if (null === $exception) {
73-
if ($chunk->isFirst()) {
74-
$context->setInfo('retry_count', $retryCount);
75-
76-
if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
79+
if (false === $shouldRetry) {
7780
$context->passthru();
78-
yield $chunk;
81+
if (null !== $firstChunk) {
82+
yield $firstChunk;
83+
yield $context->createChunk($content);
84+
yield $chunk;
85+
} else {
86+
yield $chunk;
87+
}
88+
$content = '';
7989

8090
return;
8191
}
92+
}
93+
} elseif ($chunk->isFirst()) {
94+
$context->setInfo('retry_count', $retryCount);
8295

83-
// Body is needed to decide
84-
if (null === $shouldRetry) {
85-
$firstChunk = $chunk;
86-
$content = '';
96+
if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
97+
$context->passthru();
98+
yield $chunk;
8799

88-
return;
89-
}
90-
} else {
91-
$content .= $chunk->getContent();
100+
return;
101+
}
92102

93-
if (!$chunk->isLast()) {
94-
return;
95-
}
103+
// Body is needed to decide
104+
if (null === $shouldRetry) {
105+
$firstChunk = $chunk;
106+
$content = '';
96107

97-
if (null === $shouldRetry = $this->strategy->shouldRetry($context, $content, null)) {
98-
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', \get_class($this->strategy)));
99-
}
108+
return;
109+
}
110+
} else {
111+
$content .= $chunk->getContent();
100112

101-
if (false === $shouldRetry) {
102-
$context->passthru();
103-
yield $firstChunk;
104-
yield $context->createChunk($content);
105-
$content = '';
113+
if (!$chunk->isLast()) {
114+
return;
115+
}
106116

107-
return;
108-
}
117+
if (null === $shouldRetry = $this->strategy->shouldRetry($context, $content, null)) {
118+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', \get_class($this->strategy)));
119+
}
120+
121+
if (false === $shouldRetry) {
122+
$context->passthru();
123+
yield $firstChunk;
124+
yield $context->createChunk($content);
125+
$content = '';
126+
127+
return;
109128
}
110129
}
111130

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

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,47 @@
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\MockHttpClient;
1617
use Symfony\Component\HttpClient\Response\AsyncContext;
1718
use Symfony\Component\HttpClient\Response\MockResponse;
1819
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
20+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1921

2022
class GenericRetryStrategyTest extends TestCase
2123
{
22-
public function testShouldRetryStatusCode()
24+
/**
25+
* @dataProvider provideRetryable
26+
*/
27+
public function testShouldRetry(string $method, int $code, ?TransportExceptionInterface $exception)
28+
{
29+
$strategy = new GenericRetryStrategy();
30+
31+
self::assertTrue($strategy->shouldRetry($this->getContext(0, $method, 'http://example.com/', $code), null, $exception));
32+
}
33+
34+
/**
35+
* @dataProvider provideNotRetryable
36+
*/
37+
public function testShouldNotRetry(string $method, int $code, ?TransportExceptionInterface $exception)
2338
{
24-
$strategy = new GenericRetryStrategy([500]);
39+
$strategy = new GenericRetryStrategy();
2540

26-
self::assertTrue($strategy->shouldRetry($this->getContext(0, 'GET', 'http://example.com/', 500), null, null));
41+
self::assertFalse($strategy->shouldRetry($this->getContext(0, $method, 'http://example.com/', $code), null, $exception));
2742
}
2843

29-
public function testIsNotRetryableOk()
44+
public function provideRetryable(): iterable
3045
{
31-
$strategy = new GenericRetryStrategy([500]);
46+
yield ['GET', 200, new TransportException()];
47+
yield ['GET', 500, null];
48+
yield ['POST', 429, null];
49+
}
3250

33-
self::assertFalse($strategy->shouldRetry($this->getContext(0, 'GET', 'http://example.com/', 200), null, null));
51+
public function provideNotRetryable(): iterable
52+
{
53+
yield ['POST', 200, null];
54+
yield ['POST', 200, new TransportException()];
55+
yield ['POST', 500, null];
3456
}
3557

3658
/**

0 commit comments

Comments
 (0)
0