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

Commit 434d5a3

Browse files
jderussejderusse
committed
Parameterize list of retryed Http methods
1 parent f52f090 commit 434d5a3

File tree

10 files changed

+186
-46
lines changed

10 files changed

+186
-46
lines changed

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

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,15 +1652,58 @@ private function addHttpClientRetrySection()
16521652
->scalarNode('retry_strategy')->defaultNull()->info('service id to override the retry strategy')->end()
16531653
->arrayNode('http_codes')
16541654
->performNoDeepMerging()
1655+
->validate()
1656+
->ifTrue(static function ($v) {
1657+
foreach ($v as $code => &$val) {
1658+
$code = $val['code'] ?? $code;
1659+
if ($code < 100 || $code >= 600) {
1660+
return true;
1661+
}
1662+
}
1663+
1664+
return false;
1665+
})
1666+
->thenInvalid('The keys of the "http_codes" option should be a valid HTTP status code.')
1667+
->end()
16551668
->beforeNormalization()
16561669
->ifArray()
1657-
->then(function ($v) {
1658-
return array_filter(array_values($v));
1670+
->then(static function ($v) {
1671+
$list = [];
1672+
foreach ($v as $key => $val) {
1673+
if (is_numeric($val)) {
1674+
$list[] = ['code' => $val];
1675+
} elseif (\is_array($val)) {
1676+
if (isset($val['code']) || isset($val['methods'])) {
1677+
$list[] = $val;
1678+
} else {
1679+
$list[] = ['code' => $key, 'methods' => $val];
1680+
}
1681+
} elseif (true === $val || null === $val) {
1682+
$list[] = ['code' => $key];
1683+
}
1684+
}
1685+
1686+
return $list;
16591687
})
16601688
->end()
1661-
->prototype('integer')->end()
1689+
->useAttributeAsKey('code')
1690+
->arrayPrototype()
1691+
->fixXmlConfig('method')
1692+
->children()
1693+
->integerNode('code')->end()
1694+
->arrayNode('methods')
1695+
->beforeNormalization()
1696+
->ifArray()
1697+
->then(function ($v) {
1698+
return array_map('strtoupper', $v);
1699+
})
1700+
->end()
1701+
->prototype('scalar')->end()
1702+
->info('A list of HTTP method that triggers a retry for this status code. When null, all method are retried.')
1703+
->end()
1704+
->end()
1705+
->end()
16621706
->info('A list of HTTP status code that triggers a retry')
1663-
->defaultValue([423, 425, 429, 500, 502, 503, 504, 507, 510])
16641707
->end()
16651708
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
16661709
->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: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
use Symfony\Component\Form\FormTypeGuesserInterface;
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;
@@ -666,7 +667,7 @@ private function registerProfilerConfiguration(array $config, ContainerBuilder $
666667
$container->setParameter('profiler_listener.only_master_requests', $config['only_master_requests']);
667668

668669
// Choose storage class based on the DSN
669-
list($class) = explode(':', $config['dsn'], 2);
670+
[$class] = explode(':', $config['dsn'], 2);
670671
if ('file' !== $class) {
671672
throw new \LogicException(sprintf('Driver "%s" is not supported for the profiler.', $class));
672673
}
@@ -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 (empty($codeOptions['methods'])) {
2079+
$codes[] = $code;
2080+
} else {
2081+
$codes[$code] = $codeOptions['methods'];
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,18 @@
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+
423,
28+
425,
29+
429,
30+
500 => self::IDEMPOTENT_METHODS,
31+
502,
32+
503,
33+
504 => self::IDEMPOTENT_METHODS,
34+
507 => self::IDEMPOTENT_METHODS,
35+
510 => self::IDEMPOTENT_METHODS,
36+
];
2537

2638
private $statusCodes;
2739
private $delayMs;
@@ -63,7 +75,26 @@ public function __construct(array $statusCodes = self::DEFAULT_RETRY_STATUS_CODE
6375

6476
public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
6577
{
66-
return \in_array($context->getStatusCode(), $this->statusCodes, true);
78+
$statusCode = $context->getStatusCode();
79+
if (\in_array($statusCode, $this->statusCodes, true)) {
80+
return true;
81+
}
82+
if (isset($this->statusCodes[$statusCode]) && \is_array($this->statusCodes[$statusCode])) {
83+
return \in_array($context->getInfo('http_method'), $this->statusCodes[$statusCode], true);
84+
}
85+
86+
if (null === $exception) {
87+
return false;
88+
}
89+
90+
if (\in_array(500, $this->statusCodes, true)) {
91+
return true;
92+
}
93+
if (isset($this->statusCodes[500]) && \is_array($this->statusCodes[500])) {
94+
return \in_array($context->getInfo('http_method'), $this->statusCodes[500], 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: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -69,43 +69,64 @@ public function request(string $method, string $url, array $options = []): Respo
6969
$context->setInfo('retry_count', $retryCount);
7070
}
7171

72-
if (null === $exception) {
73-
if ($chunk->isFirst()) {
74-
$context->setInfo('retry_count', $retryCount);
72+
if (null !== $exception) {
73+
// check if the request has been sent
74+
$uploaded = $context->getInfo('size_upload');
75+
if (null !== $uploaded && $uploaded > 0) {
76+
$shouldRetry = $this->strategy->shouldRetry($context, null, $exception);
77+
if (null === $shouldRetry) {
78+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with an exception.', \get_class($this->decider)));
79+
}
7580

76-
if (false === $shouldRetry = $this->strategy->shouldRetry($context, null, null)) {
81+
if (false === $shouldRetry) {
7782
$context->passthru();
78-
yield $chunk;
83+
if (null !== $firstChunk) {
84+
yield $firstChunk;
85+
yield $context->createChunk($content);
86+
yield $chunk;
87+
} else {
88+
yield $chunk;
89+
}
90+
$content = '';
7991

8092
return;
8193
}
94+
}
95+
} elseif ($chunk->isFirst()) {
96+
$context->setInfo('retry_count', $retryCount);
8297

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

88-
return;
89-
}
90-
} else {
91-
$content .= $chunk->getContent();
102+
return;
103+
}
92104

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

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-
}
110+
return;
111+
}
112+
} else {
113+
$content .= $chunk->getContent();
100114

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

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

‎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 10000 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