8000 Parameterize list of retryed Http methods · symfony/symfony@a7f7ec5 · GitHub
[go: up one dir, main page]

Skip to content

Commit a7f7ec5

Browse files
committed
Parameterize list of retryed Http methods
1 parent 414a5ab commit a7f7ec5

File tree

14 files changed

+212
-58
lines changed

14 files changed

+212
-58
lines changed

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

Lines changed: 61 additions & 5 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']) || isset($v['jitter']))) {
16451645
throw new \InvalidArgumentException('The "backoff_service" option cannot be used along with the "delay", "multiplier", "max_delay" or "jitter" 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;
@@ -1655,16 +1655,72 @@ private function addHttpClientRetrySection()
16551655
->scalarNode('backoff_service')->defaultNull()->info('service id to override the retry backoff')->end()
16561656
->scalarNode('decider_service')->defaultNull()->info('service id to override the retry decider')->end()
16571657
->arrayNode('http_codes')
1658+
->performNoDeepMerging()
1659+
->validate()
1660+
->ifTrue(static function ($v) {
1661+
foreach ($v as $code => &$val) {
1662+
$code = $val['code'] ?? $code;
1663+
if ($code < 100 || $code >= 600) {
1664+
return true;
1665+
}
1666+
}
1667+
1668+
return false;
1669+
})
1670+
->thenInvalid('The keys of the "http_codes" option should be a valid HTTP status code.')
1671+
->end()
1672+
->beforeNormalization()
1673+
->ifArray()
1674+
->then(static function ($v) {
1675+
foreach ($v as $code => &$val) {
1676+
if (\is_bool($val)) {
1677+
$val = ['retry' => $val];
1678+
continue;
1679+
}
1680+
if (!\is_array($val)) {
1681+
continue;
1682+
}
1683+
if (isset($val['retry']) || isset($val['methods'])) {
1684+
continue;
1685+
}
1686+
$val = ['retry' => true, 'methods' => array_filter(array_values($val))];
1687+
}
1688+
1689+
return $v;
1690+
})
1691+
->end()
1692+
->useAttributeAsKey('code')
1693+
->arrayPrototype()
1694+
->fixXmlConfig('method')
1695+
->children()
1696+
->booleanNode('retry')
1697+
->defaultTrue()
1698+
->end()
1699+
->arrayNode('methods')
1700+
->beforeNormalization()
1701+
->ifArray()
1702+
->then(function ($v) {
1703+
return array_map('strtoupper', $v);
1704+
})
1705+
->end()
1706+
->prototype('scalar')->end()
1707+
->info('A list of HTTP method that triggers a retry for this status code. When null, all method are retried.')
1708+
->end()
1709+
->end()
1710+
->end()
1711+
->info('A list of HTTP status code that triggers a retry')
1712+
->end()
1713+
->arrayNode('methods')
16581714
->performNoDeepMerging()
16591715
->beforeNormalization()
16601716
->ifArray()
16611717
->then(function ($v) {
16621718
return array_filter(array_values($v));
16631719
})
16641720
->end()
1665-
->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])
1721+
->prototype('scalar')->end()
1722+
->info('A list of HTTP request method code that are taken into account')
1723+
->defaultValue(['*'])
16681724
->end()
16691725
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
16701726
->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 & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2086,8 +2086,17 @@ private function registerHttpClientRetry(array $retryOptions, string $name, Cont
20862086
} else {
20872087
$retryServiceId = $name.'.retry.decider';
20882088
$retryDefinition = new ChildDefinition('http_client.retry.abstract_httpstatuscode_decider');
2089-
$retryDefinition
2090-
->replaceArgument(0, $retryOptions['http_codes']);
2089+
if (!empty($retryOptions['http_codes'])) {
2090+
$statusCodes = array_map(static function ($value) {
2091+
if (empty($value['methods'])) {
2092+
return $value['retry'];
2093+
}
2094+
2095+
return array_fill_keys($value['methods'], $value['retry']);
2096+
}, $retryOptions['http_codes']);
2097+
$retryDefinition
2098+
->setArgument(0, $statusCodes);
2099+
}
20912100
$container->setDefinition($retryServiceId, $retryDefinition);
20922101

20932102
$deciderReference = new Reference($retryServiceId);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,5 @@
6161
])
6262
->set('http_client.retry.abstract_httpstatuscode_decider', HttpStatusCodeDecider::class)
6363
->abstract()
64-
->args([
65-
abstract_arg('http codes'),
66-
])
6764
;
6865
};

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="backoff-service" 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
@@ -6,7 +6,7 @@
66
'retry_failed' => [
77
'backoff_service' => null,
88
'decider_service' => null,
9-
'http_codes' => [429, 500],
9+
'http_codes' => [429 => true, 500 => ['GET', 'HEAD']],
1010
'max_retries' => 2,
1111
'delay' => 100,
1212
'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
@@ -13,8 +13,11 @@
1313
max-delay="0"
1414
max-retries="2"
1515
multiplier="2">
16-
<framework:http-code>429</framework:http-code>
17-
<framework:http-code>500</framework:http-code>
16+
<framework:http-code code="429" retry="true"/>
17+
<framework:http-code code="500" retry="true">
18+
<framework:method>GET</framework:method>
19+
<framework:method>HEAD</framework:method>
20+
</framework:http-code>
1821
</framework:retry-failed>
1922
</framework:default-options>
2023
<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
@@ -4,7 +4,9 @@ framework:
44
retry_failed:
55
backoff_service: null
66
decider_service: null
7-
http_codes: [429, 500]
7+
http_codes:
8+
429: true
9+
500: ['GET', 'HEAD']
810
max_retries: 2
911
delay: 100
1012
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.decider')->getArgument(0));
1503+
$this->assertSame([429 => true, 500 => ['GET' => true, 'HEAD' => true]], $container->getDefinition('http_client.retry.decider')->getArgument(0));
15041504
$this->assertSame(100, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(0));
15051505
$this->assertSame(2, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(1));
15061506
$this->assertSame(0, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(2));

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,56 @@
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+
public const METHOD_DECISION = [
24+
'GET' => true,
25+
'HEAD' => true,
26+
'PUT' => true,
27+
'DELETE' => true,
28+
'OPTIONS' => true,
29+
'TRACE' => true,
30+
];
31+
private const STATUS_CODES_DECISION = [
32+
423 => true,
33+
425 => true,
34+
429 => true,
35+
500 => self::METHOD_DECISION,
36+
502 => true,
37+
503 => true,
38+
504 => self::METHOD_DECISION,
39+
507 => self::METHOD_DECISION,
40+
510 => self::METHOD_DECISION,
41+
];
42+
2143
private $statusCodes;
2244

2345
/**
24-
* @param array $statusCodes List of HTTP status codes that trigger a retry
46+
* @param array $statusCodes List of HTTP status codes that trigger a retry. Either a boolean to retry/not-retry or a list of request method to take into account
2547
*/
26-
public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503, 504, 507, 510])
48+
public function __construct(array $statusCodes = self::STATUS_CODES_DECISION)
2749
{
2850
$this->statusCodes = $statusCodes;
2951
}
3052

31-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
53+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
3254
{
33-
return \in_array($responseStatusCode, $this->statusCodes, true);
55+
if (null !== $exception && !isset($this->statusCodes[$responseStatusCode])) {
56+
$responseStatusCode = 500;
57+
}
58+
59+
$decision = $this->statusCodes[$responseStatusCode] ?? false;
60+
if (!\is_array($decision)) {
61+
return $decision;
62+
}
63+
64+
return $decision[strtoupper($requestMethod)] ?? false;
3465
}
3566
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ interface RetryBackOffInterface
2020
{
2121
/**
2222
* Returns the time to wait in milliseconds.
23+
*
24+
* @param int $retryCount Number of time the request has already been retried. Will be 0 the first time the request fails.
2325
*/
2426
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int;
2527
}

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

Lines changed: 4 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
*/
@@ -19,9 +21,10 @@ interface RetryDeciderInterface
1921
/**
2022
* Returns whether the request should be retried.
2123
*
24+
* @param int $retryCount Number of time the request has already been retried. Will be 0 the first time the request fails.
2225
* @param ?string $responseContent Null is passed when the body did not arrive yet
2326
*
2427
* @return ?bool Returns null to signal that the body is required to take a decision
2528
*/
26-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
29+
public function shouldRetry(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool;
2730
}

src/Symfony/Component/HttpClient/RetryableHttpClient.php

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -74,42 +74,63 @@ 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+
// If the request has not been sent
79+
$uploaded = $context->getInfo('size_upload');
80+
if ($uploaded !== null && $uploaded > 0) {
81+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, null, $exception);
82+
if (null === $shouldRetry) {
83+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with an exception.', \get_class($this->decider)));
84+
}
8085

8186
if (false === $shouldRetry) {
8287
$context->passthru();
83-
yield $chunk;
88+
if (null !== $firstChunk) {
89+
yield $firstChunk;
90+
yield $context->createChunk($content);
91+
yield $chunk;
92+
} else {
93+
yield $chunk;
94+
}
95+
$content = '';
8496

8597
return;
8698
}
99+
}
100+
} elseif ($chunk->isFirst()) {
101+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, null, null);
87102

88-
// Decider need body to decide
89-
if (null === $shouldRetry) {
90-
$firstChunk = $chunk;
91-
$content = '';
103+
if (false === $shouldRetry) {
104+
$context->passthru();
105+
yield $chunk;
92106

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-
}
107+
return;
108+
}
104109

105-
if (false === $shouldRetry) {
106-
$context->passthru();
107-
yield $firstChunk;
108-
yield $context->createChunk($content);
109-
$content = '';
110+
// Decider need body to decide
111+
if (null === $shouldRetry) {
112+
$firstChunk = $chunk;
113+
$content = '';
110114

111-
return;
112-
}
115+
return;
116+
}
117+
} else {
118+
$content .= $chunk->getContent();
119+
if (!$chunk->isLast()) {
120+
return;
121+
}
122+
$shouldRetry = $this->decider->shouldRetry($retryCount, $method, $url, $options, $statusCode, $headers, $content, null);
123+
if (null === $shouldRetry) {
124+
throw new \LogicException(sprintf('The "%s::shouldRetry()" method must not return null when called with a body.', \get_class($this->decider)));
125+
}
126+
127+
if (false === $shouldRetry) {
128+
$context->passthru();
129+
yield $firstChunk;
130+
yield $context->createChunk($content);
131+
$content = '';
132+
133+
return;
113134
}
114135
}
115136

0 commit comments

Comments
 (0)
0