8000 feature #38532 [HttpClient] simplify retry mechanism around RetryStra… · symfony/symfony@f52f090 · GitHub
[go: up one dir, main page]
Skip to content

Commit f52f090

Browse files
nicolas-grekasnicolas-grekas
committed
feature #38532 [HttpClient] simplify retry mechanism around RetryStrategyInterface (nicolas-grekas)
This PR was merged into the 5.x branch. Discussion ---------- [HttpClient] simplify retry mechanism around RetryStrategyInterface | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Replaces #38466 I feel like the mechanism behind RetryableHttpClient is too bloated for no pragmatic reasons. I propose to merge RetryDeciderInterface and RetryBackoffInterface into one single RetryStrategyInterface. Having two separate interfaces supports no real-world use case. The only implementations we provide are trivial, and they can be reused by extending the provided GenericRetryStrategy implementation if one really doesn't want to duplicate that. The methods are also simplified by accepting an AsyncContext as argument. This makes the signatures shorter and allows accessing the "info" of the request (allowing to decide based on the IP of the host, etc). /cc @jderusse Commits ------- 544c3e8 [HttpClient] simplify retry mechanism around RetryStrategyInterface
2 parents ee3294e + 544c3e8 commit f52f090

17 files changed

+198
-281
lines changed

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,19 +1641,15 @@ private function addHttpClientRetrySection()
16411641
->addDefaultsIfNotSet()
16421642
->beforeNormalization()
16431643
->always(function ($v) {
1644-
if (isset($v['backoff_service']) && (isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
1645-
throw new \InvalidArgumentException('The "backoff_service" option cannot be used along with the "delay", "multiplier", "max_delay" or "jitter" options.');
1646< 10000 /code>-
}
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.');
1644+
if (isset($v['retry_strategy']) && (isset($v['http_codes']) || isset($v['delay']) || isset($v['multiplier']) || isset($v['max_delay']) || isset($v['jitter']))) {
1645+
throw new \InvalidArgumentException('The "retry_strategy" option cannot be used along with the "http_codes", "delay", "multiplier", "max_delay" or "jitter" options.');
16491646
}
16501647

16511648
return $v;
16521649
})
16531650
->end()
16541651
->children()
1655-
->scalarNode('backoff_service')->defaultNull()->info('service id to override the retry backoff')->end()
1656-
->scalarNode('decider_service')->defaultNull()->info('service id to override the retry decider')->end()
1652+
->scalarNode('retry_strategy')->defaultNull()->info('service id to override the retry strategy')->end()
16571653
->arrayNode('http_codes')
16581654
->performNoDeepMerging()
16591655
->beforeNormalization()
@@ -1668,9 +1664,9 @@ private function addHttpClientRetrySection()
16681664
->end()
16691665
->integerNode('max_retries')->defaultValue(3)->min(0)->end()
16701666
->integerNode('delay')->defaultValue(1000)->min(0)->info('Time in ms to delay (or the initial value when multiplier is used)')->end()
1671-
->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: (delay * (multiple ^ retries))')->end()
1667+
->floatNode('multiplier')->defaultValue(2)->min(1)->info('If greater than 1, delay will grow exponentially for each retry: delay * (multiple ^ retries)')->end()
16721668
->integerNode('max_delay')->defaultValue(0)->min(0)->info('Max time in ms that a retry should ever be delayed (0 = infinite)')->end()
1673-
->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1)) to apply to the delay')->end()
1669+
->floatNode('jitter')->defaultValue(0.1)->min(0)->max(1)->info('Randomness in percent (between 0 and 1) to apply to the delay')->end()
16741670
->end()
16751671
;
16761672
}

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

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2011,10 +2011,10 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
20112011
}
20122012

20132013
if ($this->isConfigEnabled($container, $retryOptions)) {
2014-
$this->registerHttpClientRetry($retryOptions, 'http_client', $container);
2014+
$this->registerRetryableHttpClient($retryOptions, 'http_client', $container);
20152015
}
20162016

2017-
$httpClientId = $retryOptions['enabled'] ?? false ? 'http_client.retry.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
2017+
$httpClientId = ($retryOptions['enabled'] ?? false) ? 'http_client.retryable.inner' : ($this->isConfigEnabled($container, $profilerConfig) ? '.debug.http_client.inner' : 'http_client');
20182018
foreach ($config['scoped_clients'] as $name => $scopeConfig) {
20192019
if ('http_client' === $name) {
20202020
throw new InvalidArgumentException(sprintf('Invalid scope name: "%s" is reserved.', $name));
@@ -2042,7 +2042,7 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
20422042
}
20432043

20442044
if ($this->isConfigEnabled($container, $retryOptions)) {
2045-
$this->registerHttpClientRetry($retryOptions, $name, $container);
2045+
$this->registerRetryableHttpClient($retryOptions, $name, $container);
20462046
}
20472047

20482048
$container->registerAliasForArgument($name, HttpClientInterface::class);
@@ -2062,42 +2062,31 @@ private function registerHttpClientConfiguration(array $config, ContainerBuilder
20622062
}
20632063
}
20642064

2065-
private function registerHttpClientRetry(array $retryOptions, string $name, ContainerBuilder $container)
2065+
private function registerRetryableHttpClient(array $options, string $name, ContainerBuilder $container)
20662066
{
20672067
if (!class_exists(RetryableHttpClient::class)) {
2068-
throw new LogicException('Retry failed request support cannot be enabled as version 5.2+ of the HTTP Client component is required.');
2068+
throw new Logi F913 cException('Support for retrying failed requests requires symfony/http-client 5.2 or higher, try upgrading.');
20692069
}
20702070

2071-
if (null !== $retryOptions['backoff_service']) {
2072-
$backoffReference = new Reference($retryOptions['backoff_service']);
2071+
if (null !== $options['retry_strategy']) {
2072+
$retryStrategy = new Reference($options['retry_strategy']);
20732073
} else {
2074-
$retryServiceId = $name.'.retry.exponential_backoff';
2075-
$retryDefinition = new ChildDefinition('http_client.retry.abstract_exponential_backoff');
2076-
$retryDefinition
2077-
->replaceArgument(0, $retryOptions['delay'])
2078-
->replaceArgument(1, $retryOptions['multiplier'])
2079-
->replaceArgument(2, $retryOptions['max_delay'])
2080-
->replaceArgument(3, $retryOptions['jitter']);
2081-
$container->setDefinition($retryServiceId, $retryDefinition);
2082-
2083-
$backoffReference = new Reference($retryServiceId);
2084-
}
2085-
if (null !== $retryOptions['decider_service']) {
2086-
$deciderReference = new Reference($retryOptions['decider_service']);
2087-
} else {
2088-
$retryServiceId = $name.'.retry.decider';
2089-
$retryDefinition = new ChildDefinition('http_client.retry.abstract_httpstatuscode_decider');
2090-
$retryDefinition
2091-
->replaceArgument(0, $retryOptions['http_codes']);
2092-
$container->setDefinition($retryServiceId, $retryDefinition);
2074+
$retryStrategy = new ChildDefinition('http_client.abstract_retry_strategy');
2075+
$retryStrategy
2076+
->replaceArgument(0, $options['http_codes'])
2077+
->replaceArgument(1, $options['delay'])
2078+
->replaceArgument(2, $options['multiplier'])
2079+
->replaceArgument(3, $options['max_delay'])
2080+
->replaceArgument(4, $options['jitter']);
2081+
$container->setDefinition($name.'.retry_strategy', $retryStrategy);
20932082

2094-
$deciderReference = new Reference($retryServiceId);
2083+
$retryStrategy = new Reference($name.'.retry_strategy');
20952084
}
20962085

20972086
$container
2098-
->register($name.'.retry', RetryableHttpClient::class)
2087+
->register($name.'.retryable', RetryableHttpClient::class)
20992088
->setDecoratedService($name, null, -10) // lower priority than TraceableHttpClient
2100-
->setArguments([new Reference($name.'.retry.inner'), $deciderReference, $backoffReference, $retryOptions['max_retries'], new Reference('logger')])
2089+
->setArguments([new Reference($name.'.retryable.inner'), $retryStrategy, $options['max_retries'], new Reference('logger')])
21012090
->addTag('monolog.logger', ['channel' => 'http_client']);
21022091
}
21032092

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@
1717
use Symfony\Component\HttpClient\HttpClient;
1818
use Symfony\Component\HttpClient\HttplugClient;
1919
use Symfony\Component\HttpClient\Psr18Client;
20-
use Symfony\Component\HttpClient\Retry\ExponentialBackOff;
21-
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
20+
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
2221
use Symfony\Contracts\HttpClient\HttpClientInterface;
2322

2423
return static function (ContainerConfigurator $container) {
@@ -51,19 +50,14 @@
5150
service(StreamFactoryInterface::class)->ignoreOnInvalid(),
5251
])
5352

54-
// retry
55-
->set('http_client.retry.abstract_exponential_backoff', ExponentialBackOff::class)
53+
->set('http_client.abstract_retry_strategy', GenericRetryStrategy::class)
5654
->abstract()
5755
->args([
56+
abstract_arg('http codes'),
5857
abstract_arg('delay ms'),
5958
abstract_arg('multiplier'),
6059
abstract_arg('max delay ms'),
6160
abstract_arg('jitter'),
6261
])
63-
->set('http_client.retry.abstract_httpstatuscode_decider', HttpStatusCodeDecider::class)
64-
->abstract()
65-
->args([
66-
abstract_arg('http codes'),
67-
])
6862
;
6963
};

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,7 @@
581581
<xsd:element name="http-code" type="xsd:integer" minOccurs="0" maxOccurs="unbounded" />
582582
</xsd:sequence>
583583
<xsd:attribute name="enabled" type="xsd:boolean" />
584-
<xsd:attribute name="backoff-service" type="xsd:string" />
585-
<xsd:attribute name="decider-service" type="xsd:string" />
584+
<xsd:attribute name="retry-strategy" type="xsd:string" />
586585
<xsd:attribute name="max-retries" type="xsd:integer" />
587586
<xsd:attribute name="delay" type="xsd:integer" />
588587
<xsd:attribute name="multiplier" type="xsd:float" />

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
'http_client' => [
55
'default_options' => [
66
'retry_failed' => [
7-
'backoff_service' => null,
8-
'decider_service' => null,
7+
'retry_strategy' => null,
98
'http_codes' => [429, 500],
109
'max_retries' => 2,
1110
'delay' => 100,

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ framework:
22
http_client:
33
default_options:
44
retry_failed:
5-
backoff_service: null
6-
decider_service: null
5+
retry_strategy: null
76
http_codes: [429, 500]
87
max_retries: 2
98
delay: 100

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,15 +1500,15 @@ 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));
1504-
$this->assertSame(100, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(0));
1505-
$this->assertSame(2, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(1));
1506-
$this->assertSame(0, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(2));
1507-
$this->assertSame(0.3, $container->getDefinition('http_client.retry.exponential_backoff')->getArgument(3));
1508-
$this->assertSame(2, $container->getDefinition('http_client.retry')->getArgument(3));
1509-
1510-
$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retry')->getClass());
1511-
$this->assertSame(4, $container->getDefinition('foo.retry.exponential_backoff')->getArgument(1));
1503+
$this->assertSame([429, 500], $container->getDefinition('http_client.retry_strategy')->getArgument(0));
1504+
$this->assertSame(100, $container->getDefinition('http_client.retry_strategy')->getArgument(1));
1505+
$this->assertSame(2, $container->getDefinition('http_client.retry_strategy')->getArgument(2));
1506+
$this->assertSame(0, $container->getDefinition('http_client.retry_strategy')->getArgument(3));
1507+
$this->assertSame(0.3, $container->getDefinition('http_client.retry_strategy')->getArgument(4));
1508+
$this->assertSame(2, $container->getDefinition('http_client.retryable')->getArgument(2));
1509+
1510+
$this->assertSame(RetryableHttpClient::class, $container->getDefinition('foo.retryable')->getClass());
1511+
$this->assertSame(4, $container->getDefinition('foo.retry_strategy')->getArgument(2));
15121512
}
15131513

15141514
public function testHttpClientWithQueryParameterKey()

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
use Symfony\Component\HttpClient\Exception\TransportException;
2929
use Symfony\Component\HttpClient\HttpClientTrait;
3030
use Symfony\Component\HttpClient\Internal\AmpBody;
31-
use Symfony\Component\HttpClient\Internal\AmpCanary;
3231
use Symfony\Component\HttpClient\Internal\AmpClientState;
3332
use Symfony\Component\HttpClient\Internal\Canary;
3433
use Symfony\Component\HttpClient\Internal\ClientState;

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

Lines changed: 0 additions & 81 deletions
This file was deleted.

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpClient\Retry;
13+
14+
use Symfony\Component\HttpClient\Response\AsyncContext;
15+
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
16+
17+
/**
18+
* Decides to retry the request when HTTP status codes belong to the given list of codes.
19+
*
20+
* @author Jérémy Derussé <jeremy@derusse.com>
21+
*/
22+
class GenericRetryStrategy implements RetryStrategyInterface
23+
{
24+
public const DEFAULT_RETRY_STATUS_CODES = [423, 425, 429, 500, 502, 503, 504, 507, 510];
25+
26+
private $statusCodes;
27+
private $delayMs;
28+
private $multiplier;
29+
private $maxDelayMs;
30+
private $jitter;
31+
32+
/**
33+
* @param array $statusCodes List of HTTP status codes that trigger a retry
34+
* @param int $delayMs Amount of time to delay (or the initial value when multiplier is used)
35+
* @param float $multiplier Multiplier to apply to the delay each time a retry occurs
36+
* @param int $maxDelayMs Maximum delay to allow (0 means no maximum)
37+
* @param float $jitter Probability of randomness int delay (0 = none, 1 = 100% random)
38+
*/
39+
public function __construct(array $statusCodes = self::DEFAULT_RETRY_STATUS_CODES, int $delayMs = 1000, float $multiplier = 2.0, int $maxDelayMs = 0, float $jitter = 0.1)
40+
{
41+
$this->statusCodes = $statusCodes;
42+
43+
if ($delayMs < 0) {
44+
throw new InvalidArgumentException(sprintf('Delay must be greater than or equal to zero: "%s" given.', $delayMs));
45+
}
46+
$this->delayMs = $delayMs;
47+
48+
if ($multiplier < 1) {
49+
throw new InvalidArgumentException(sprintf('Multiplier must be greater than or equal to one: "%s" given.', $multiplier));
50+
}
51+
$this->multiplier = $multiplier;
52+
53+
if ($maxDelayMs < 0) {
54+
throw new InvalidArgumentException(sprintf('Max delay must be greater than or equal to zero: "%s" given.', $maxDelayMs));
55+
}
56+
$this->maxDelayMs = $maxDelayMs;
57+
58+
if ($jitter < 0 || $jitter > 1) {
59+
throw new InvalidArgumentException(sprintf('Jitter must be between 0 and 1: "%s" given.', $jitter));
60+
}
61+
$this->jitter = $jitter;
62+
}
63+
64+
public function shouldRetry(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): ?bool
65+
{
66+
return \in_array($context->getStatusCode(), $this->statusCodes, true);
67+
}
68+
69+
public function getDelay(AsyncContext $context, ?string $responseContent, ?TransportExceptionInterface $exception): int
70+
{
71+
$delay = $this->delayMs * $this->multiplier ** $context->getInfo('retry_count');
72+
73+
if ($this->jitter > 0) {
74+
$randomness = $delay * $this->jitter;
75+
$delay = $delay + random_int(-$randomness, +$randomness);
76+
}
77+
78+
if ($delay > $this->maxDelayMs && 0 !== $this->maxDelayMs) {
79+
return $this->maxDelayMs;
80+
}
81+
82+
return (int) $delay;
83+
}
84+
}

0 commit comments

Comments
 (0)
0