Skip to content

Commit ebd5c2b

Browse files
jderussejderusse
committed
Do not retry n exceptions
1 parent 6dcdfe2 commit ebd5c2b

File tree

7 files changed

+49
-58
lines changed

7 files changed

+49
-58
lines changed

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

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

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14+
use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
1415
use Symfony\Component\Messenger\Exception\InvalidArgumentException;
1516

1617
/**
@@ -56,7 +57,7 @@ public function __construct(int $delayMilliseconds = 1000, float $multiplier = 2
5657
$this->maxDelayMilliseconds = $maxDelayMilliseconds;
5758
}
5859

59-
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, \Throwable $throwable = null): int
60+
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int
6061
{
6162
$delay = $this->delayMilliseconds * $this->multiplier ** $retryCount;
6263

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,8 @@ public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503,
3030
$this->statusCodes = $statusCodes;
3131
}
3232

33-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool
33+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool
3434
{
35-
if ($throwable instanceof TransportExceptionInterface) {
36-
return true;
37-
}
38-
3935
return \in_array($responseStatusCode, $this->statusCodes, true);
4036
}
4137
}

‎src/Symfony/Component/HttpClient/Retry/RetryBackOffInterface.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\Component\Mailer\Exception\TransportExceptionInterface;
15+
1416
/**
1517
* @author Jérémy Derussé <jeremy@derusse.com>
1618
*/
@@ -19,5 +21,5 @@ interface RetryBackOffInterface
1921
/**
2022
* Returns the time to wait in milliseconds.
2123
*/
22-
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, \Throwable $throwable = null): int;
24+
public function getDelay(int $retryCount, string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, ?TransportExceptionInterface $exception): int;
2325
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@ interface RetryDeciderInterface
2525
*
2626
* @return ?bool Returns null to signal that the body is required to take a decision
2727
*/
28-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool;
28+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
2929
}

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

Lines changed: 39 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ public function __construct(HttpClientInterface $client, RetryDeciderInterface $
5454

5555
public function request(string $method, string $url, array $options = []): ResponseInterface
5656
{
57+
if ($this->maxRetries <= 0) {
58+
return $this->client->request($method, $url, $options);
59+
}
60+
5761
$retryCount = 0;
5862
$content = '';
5963
$firstChunk = null;
@@ -70,55 +74,46 @@ public function request(string $method, string $url, array $options = []): Respo
7074
// catch TransportExceptionInterface to send it to strategy.
7175
}
7276

73-
if ($retryCount >= $this->maxRetries) {
74-
$context->passthru();
75-
yield $chunk;
76-
77-
return;
78-
}
79-
8077
$statusCode = $context->getStatusCode();
8178
$headers = $context->getHeaders();
82-
if ($chunk instanceof FirstChunk || null !== $exception) {
83-
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null, $exception);
79+
if (null === $exception) {
80+
if ($chunk instanceof FirstChunk) {
81+
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, null);
8482

85-
if (false === $shouldRetry) {
86-
$context->passthru();
87-
yield $chunk;
88-
89-
return;
90-
}
83+
if (false === $shouldRetry) {
84+
$context->passthru();
85+
yield $chunk;
9186

92-
// Decider need body to decide
93-
if (null === $shouldRetry) {
94-
if (null !== $exception) {
95-
throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with an exception.', \get_class($this->decider)));
87+
return;
9688
}
9789

98-
$firstChunk = $chunk;
99-
$content = '';
100-
// disable exception thrown on destruct
101-
$context->getResponse()->getStatusCode();
90+
// Decider need body to decide
91+
if (null === $shouldRetry) {
92+
$firstChunk = $chunk;
93+
$content = '';
94+
// disable exception thrown on destruct
95+
$context->getResponse()->getStatusCode();
10296

103-
return;
104-
}
105-
} else {
106-
$content .= $chunk->getContent();
107-
if (!$chunk instanceof LastChunk) {
108-
return;
109-
}
110-
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content, null);
111-
if (null === $shouldRetry) {
112-
throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with a body.', \get_class($this->decider)));
113-
}
97+
return;
98+
}
99+
} else {
100+
$content .= $chunk->getContent();
101+
if (!$chunk instanceof LastChunk) {
102+
return;
103+
}
104+
$shouldRetry = $this->decider->shouldRetry($method, $url, $options, $statusCode, $headers, $content, null);
105+
if (null === $shouldRetry) {
106+
throw new \LogicException(sprintf('The "%s::shouldRetry" method must not return null when called with a body.', \get_class($this->decider)));
107+
}
114108

115-
if (false === $shouldRetry) {
116-
$context->passthru();
117-
yield $firstChunk;
118-
yield $context->createChunk($content);
119-
$content = '';
109+
if (false === $shouldRetry) {
110+
$context->passthru();
111+
yield $firstChunk;
112+
yield $context->createChunk($content);
113+
$content = '';
120114

121-
return;
115+
return;
116+
}
122117
}
123118
}
124119

@@ -135,6 +130,10 @@ public function request(string $method, string $url, array $options = []): Respo
135130

136131
$context->replaceRequest($method, $url, $options);
137132
$context->pause($delay / 1000);
133+
134+
if ($retryCount >= $this->maxRetries) {
135+
$context->passthru();
136+
}
138137
});
139138
}
140139

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,17 @@
1717

1818
class HttpStatusCodeDeciderTest extends TestCase
1919
{
20-
public function testShouldRetryException()
21-
{
22-
$decider = new HttpStatusCodeDecider([500]);
23-
24-
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null, new TransportException()));
25-
}
26-
2720
public function testShouldRetryStatusCode()
2821
{
2922
$decider = new HttpStatusCodeDecider([500]);
3023

31-
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null, null));
24+
self::assertTrue($decider->shouldRetry('GET', 'http://example.com/', [], 500, [], null));
3225
}
3326

3427
public function testIsNotRetryableOk()
3528
{
3629
$decider = new HttpStatusCodeDecider([500]);
3730

38-
self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null, null));
31+
self::assertFalse($decider->shouldRetry('GET', 'http://example.com/', [], 200, [], null));
3932
}
4033
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function testRetryWithBody()
5858
new MockResponse('', ['http_code' => 200]),
5959
]),
6060
new class() implements RetryDeciderInterface {
61-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool
61+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
6262
{
6363
return null === $responseContent ? null : 200 !== $responseCode;
6464
}

0 commit comments

Comments
 (0)