Skip to content

Commit c85bbb0

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

File tree

7 files changed

+49
-64
lines changed

7 files changed

+49
-64
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 & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
15-
1614
/**
1715
* Decides to retry the request when HTTP status codes belong to the given list of codes.
1816
*
@@ -30,12 +28,8 @@ public function __construct(array $statusCodes = [423, 425, 429, 500, 502, 503,
3028
$this->statusCodes = $statusCodes;
3129
}
3230

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

‎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 & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111

1212
namespace Symfony\Component\HttpClient\Retry;
1313

14-
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
15-
1614
/**
1715
* @author Jérémy Derussé <jeremy@derusse.com>
1816
*/
@@ -25,5 +23,5 @@ interface RetryDeciderInterface
2523
*
2624
* @return ?bool Returns null to signal that the body is required to take a decision
2725
*/
28-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool;
26+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseStatusCode, array $responseHeaders, ?string $responseContent): ?bool;
2927
}

‎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 & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,21 @@
1212
namespace Symfony\Component\HttpClient\Tests\Retry;
1313

1414
use PHPUnit\Framework\TestCase;
15-
use Symfony\Component\HttpClient\Exception\TransportException;
1615
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
1716

1817
class HttpStatusCodeDeciderTest extends TestCase
1918
{
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-
2719
public function testShouldRetryStatusCode()
2820
{
2921
$decider = new HttpStatusCodeDecider([500]);
3022

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

3426
public function testIsNotRetryableOk()
3527
{
3628
$decider = new HttpStatusCodeDecider([500]);
3729

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

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
use Symfony\Component\HttpClient\Retry\HttpStatusCodeDecider;
1111
use Symfony\Component\HttpClient\Retry\RetryDeciderInterface;
1212
use Symfony\Component\HttpClient\RetryableHttpClient;
13-
use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
1413

1514
class RetryableHttpClientTest extends TestCase
1615
{
@@ -58,7 +57,7 @@ public function testRetryWithBody()
5857
new MockResponse('', ['http_code' => 200]),
5958
]),
6059
new class() implements RetryDeciderInterface {
61-
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent, TransportExceptionInterface $throwable = null): ?bool
60+
public function shouldRetry(string $requestMethod, string $requestUrl, array $requestOptions, int $responseCode, array $responseHeaders, ?string $responseContent): ?bool
6261
{
6362
return null === $responseContent ? null : 200 !== $responseCode;
6463
}

0 commit comments

Comments
 (0)