Skip to content

Commit 4a1a919

Browse files
dbudbu
committed
no exception should be thrown on server error, but retrying those would make sense
1 parent af5e870 commit 4a1a919

File tree

3 files changed

+81
-35
lines changed

3 files changed

+81
-35
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
## 2.0 (unreleased)
44

55
### Changed
6-
- RetryPlugin will no longer retry requests when the response failed with a HTTP code < 500.
6+
- RetryPlugin will no longer retry exceptions when there is a response failed with a HTTP code < 500.
7+
- RetryPlugin also retries when no exception is thrown if the responses has HTTP code >= 500. The callbacks
8+
for exception handling have been renamed and callbacks for response handling have been added.
79
- Abstract method `HttpClientPool::chooseHttpClient()` has now an explicit return type (`Http\Client\Common\HttpClientPoolItem`)
810
- Interface method `Plugin::handleRequest(...)` has now an explicit return type (`Http\Promise\Promise`)
911
- Made classes final that are not intended to be extended.
@@ -13,6 +15,7 @@
1315

1416
### Removed
1517
- Deprecated option `debug_plugins` has been removed from `PluginClient`
18+
- Deprecated options `decider` and `delay` have been removed from `RetryPlugin`, use `exception_decider` and `exception_delay` instead.
1619

1720
## 1.9.0 - [unreleased]
1821

‎spec/Plugin/RetryPluginSpec.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,19 @@ public function it_does_not_keep_history_of_old_failure(RequestInterface $reques
150150
$this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf(HttpFulfilledPromise::class);
151151
}
152152

153-
public function it_has_an_exponential_default_delay(RequestInterface $request, Exception\HttpException $exception)
153+
public function it_has_an_exponential_default_error_response_delay(RequestInterface $request, ResponseInterface $response)
154154
{
155-
$this->defaultDelay($request, $exception, 0)->shouldBe(500000);
156-
$this->defaultDelay($request, $exception, 1)->shouldBe(1000000);
157-
$this->defaultDelay($request, $exception, 2)->shouldBe(2000000);
158-
$this->defaultDelay($request, $exception, 3)->shouldBe(4000000);
155+
$this->defaultErrorResponseDelay($request, $response, 0)->shouldBe(500000);
156+
$this->defaultErrorResponseDelay($request, $response, 1)->shouldBe(1000000);
157+
$this->defaultErrorResponseDelay($request, $response, 2)->shouldBe(2000000);
158+
$this->defaultErrorResponseDelay($request, $response, 3)->shouldBe(4000000);
159+
}
160+
161+
public function it_has_an_exponential_default_exception_delay(RequestInterface $request, Exception\HttpException $exception)
162+
{
163+
$this->defaultExceptionDelay($request, $exception, 0)->shouldBe(500000);
164+
$this->defaultExceptionDelay($request, $exception, 1)->shouldBe(1000000);
165+
$this->defaultExceptionDelay($request, $exception, 2)->shouldBe(2000000);
166+
$this->defaultExceptionDelay($request, $exception, 3)->shouldBe(4000000);
159167
}
160168
}

‎src/Plugin/RetryPlugin.php

Lines changed: 64 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,23 @@ final class RetryPlugin implements Plugin
3131
/**
3232
* @var callable
3333
*/
34-
private $exceptionDelay;
34+
private $errorResponseDelay;
35+
36+
/**
37+
* @var callable
38+
*/
39+
private $errorResponseDecider;
3540

3641
/**
3742
* @var callable
3843
*/
3944
private $exceptionDecider;
4045

46+
/**
47+
* @var callable
48+
*/
49+
private $exceptionDelay;
50+
4151
/**
4252
* Store the retry counter for each request.
4353
*
@@ -49,44 +59,39 @@ final class RetryPlugin implements Plugin
4959
* @param array $config {
5060
*
5161
* @var int $retries Number of retries to attempt if an exception occurs before letting the exception bubble up
62+
* @var callable $error_response_decider A callback that gets a request and response to decide whether the request should be retried
5263
* @var callable $exception_decider A callback that gets a request and an exception to decide after a failure whether the request should be retried
64+
* @var callable $error_response_delay A callback that gets a request and response and the number of retries and returns how many microseconds we should wait before trying again
5365
* @var callable $exception_delay A callback that gets a request, an exception and the number of retries and returns how many microseconds we should wait before trying again
5466
* }
5567
*/
5668
public function __construct(array $config = [])
5769
{
58-
if (array_key_exists('decider', $config)) {
59-
if (array_key_exists('exception_decider', $config)) {
60-
throw new \InvalidArgumentException('Do not set both the old "decider" and new "exception_decider" options');
61-
}
62-
trigger_error('The "decider" option has been deprecated in favour of "exception_decider"', E_USER_DEPRECATED);
63-
$config['exception_decider'] = $config['decider'];
64-
unset($config['decider']);
65-
}
66-
if (array_key_exists('delay', $config)) {
67-
if (array_key_exists('exception_delay', $config)) {
68-
throw new \InvalidArgumentException('Do not set both the old "delay" and new "exception_delay" options');
69-
}
70-
trigger_error('The "delay" option has been deprecated in favour of "exception_delay"', E_USER_DEPRECATED);
71-
$config['exception_delay'] = $config['delay'];
72-
unset($config['delay']);
73-
}
74-
7570
$resolver = new OptionsResolver();
7671
$resolver->setDefaults([
7772
'retries' => 1,
73+
'error_response_decider' => function (RequestInterface $request, ResponseInterface $response) {
74+
// do not retry client errors
75+
return $response->getStatusCode() >= 500;
76+
},
7877
'exception_decider' => function (RequestInterface $request, Exception $e) {
7978
// do not retry client errors
8079
return !$e instanceof HttpException || $e->getCode() >= 500;
8180
},
82-
'exception_delay' => __CLASS__.'::defaultDelay',
81+
'error_response_delay' => __CLASS__.'::defaultErrorResponseDelay',
82+
'exception_delay' => __CLASS__.'::defaultExceptionDelay',
8383
]);
84+
8485
$resolver->setAllowedTypes('retries', 'int');
86+
$resolver->setAllowedTypes('error_response_decider', 'callable');
8587
$resolver->setAllowedTypes('exception_decider', 'callable');
88+
$resolver->setAllowedTypes('error_response_delay', 'callable');
8689
$resolver->setAllowedTypes('exception_delay', 'callable');
8790
$options = $resolver->resolve($config);
8891

8992
$this->retry = $options['retries'];
93+
$this->errorResponseDecider = $options['error_response_decider'];
94+
$this->errorResponseDelay = $options['error_response_delay'];
9095
$this->exceptionDecider = $options['exception_decider'];
9196
$this->exceptionDelay = $options['exception_delay'];
9297
}
@@ -98,7 +103,22 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
98103
{
99104
$chainIdentifier = spl_object_hash((object) $first);
100105

101-
return $next($request)->then(function (ResponseInterface $response) use ($request, $chainIdentifier) {
106+
return $next($request)->then(function (ResponseInterface $response) use ($request, $next, $first, $chainIdentifier) {
107+
if (!array_key_exists($chainIdentifier, $this->retryStorage)) {
108+
$this->retryStorage[$chainIdentifier] = 0;
109+
}
110+
111+
if ($this->retryStorage[$chainIdentifier] >= $this->retry) {
112+
unset($this->retryStorage[$chainIdentifier]);
113+
114+
return $response;
115+
}
116+
117+
if (call_user_func($this->errorResponseDecider, $request, $response)) {
118+
$time = call_user_func($this->errorResponseDelay, $request, $response, $this->retryStorage[$chainIdentifier]);
119+
$response = $this->retry($request, $next, $first, $chainIdentifier, $time);
120+
}
121+
102122
if (array_key_exists($chainIdentifier, $this->retryStorage)) {
103123
unset($this->retryStorage[$chainIdentifier]);
104124
}
@@ -120,23 +140,38 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
120140
}
121141

122142
$time = call_user_func($this->exceptionDelay, $request, $exception, $this->retryStorage[$chainIdentifier]);
123-
usleep($time);
124-
125-
// Retry synchronously
126-
++$this->retryStorage[$chainIdentifier];
127-
$promise = $this->handleRequest($request, $next, $first);
128143

129-
return $promise->wait();
144+
return $this->retry($request, $next, $first, $chainIdentifier, $time);
130145
});
131146
}
132147

133148
/**
134149
* @param int $retries The number of retries we made before. First time this get called it will be 0.
135-
*
136-
* @return int
137150
*/
138-
public static function defaultDelay(RequestInterface $request, Exception $e, $retries)
151+
public static function defaultErrorResponseDelay(RequestInterface $request, ResponseInterface $response, int $retries): int
139152
{
140153
return pow(2, $retries) * 500000;
141154
}
155+
156+
/**
157+
* @param int $retries The number of retries we made before. First time this get called it will be 0.
158+
*/
159+
public static function defaultExceptionDelay(RequestInterface $request, Exception $e, int $retries): int
160+
{
161+
return pow(2, $retries) * 500000;
162+
}
163+
164+
/**
165+
* @throws \Exception if retrying returns a failed promise
166+
*/
167+
private function retry(RequestInterface $request, callable $next, callable $first, string $chainIdentifier, int $delay): ResponseInterface
168+
{
169+
usleep($delay);
170+
171+
// Retry synchronously
172+
++$this->retryStorage[$chainIdentifier];
173+
$promise = $this->handleRequest($request, $next, $first);
174+
175+
return $promise->wait();
176+
}
142177
}

0 commit comments

Comments
 (0)