Skip to content

Commit 18e36cc

Browse files
dbuNyholm
authored andcommitted
adjust decider to not retry client errors (#125)
1 parent 14a2a39 commit 18e36cc

File tree

3 files changed

+27
-2
lines changed

3 files changed

+27
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 2.0 (unreleased)
44

55
### Changed
6+
- RetryPlugin will no longer retry requests when the response failed with a HTTP code < 500.
67
- Abstract method `HttpClientPool::chooseHttpClient()` has now an explicit return type (`Http\Client\Common\HttpClientPoolItem`)
78
- Interface method `Plugin::handleRequest(...)` has now an explicit return type (`Http\Promise\Promise`)
89

spec/Plugin/RetryPluginSpec.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,28 @@ public function it_throws_exception_on_multiple_exceptions(RequestInterface $req
5959
$promise->shouldThrow($exception2)->duringWait();
6060
}
6161

62+
public function it_does_not_retry_client_errors(RequestInterface $request, ResponseInterface $response)
63+
{
64+
$exception = new Exception\HttpException('Exception', $request->getWrappedObject(), $response->getWrappedObject());
65+
66+
$seen = false;
67+
$next = function (RequestInterface $receivedRequest) use ($request, $exception, &$seen) {
68+
if (!Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) {
69+
throw new \Exception('Unexpected request received');
70+
}
71+
if ($seen) {
72+
throw new \Exception('This should only be called once');
73+
}
74+
$seen = true;
75+
76+
return new HttpRejectedPromise($exception);
77+
};
78+
79+
$promise = $this->handleRequest($request, $next, function () {});
80+
$promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class);
81+
$promise->shouldThrow($exception)->duringWait();
82+
}
83+
6284
public function it_returns_response_on_second_try(RequestInterface $request, ResponseInterface $response)
6385
{
6486
$exception = new Exception\NetworkException('Exception 1', $request->getWrappedObject());

src/Plugin/RetryPlugin.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Http\Client\Common\Plugin;
66
use Http\Client\Exception;
7+
use Http\Client\Exception\HttpException;
78
use Http\Promise\Promise;
89
use Psr\Http\Message\RequestInterface;
910
use Psr\Http\Message\ResponseInterface;
@@ -56,7 +57,8 @@ public function __construct(array $config = [])
5657
$resolver->setDefaults([
5758
'retries' => 1,
5859
'decider' => function (RequestInterface $request, Exception $e) {
59-
return true;
60+
// do not retry client errors
61+
return !$e instanceof HttpException || $e->getCode() >= 500;
6062
},
6163
'delay' => __CLASS__.'::defaultDelay',
6264
]);
@@ -101,7 +103,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl
101103
$time = call_user_func($this->delay, $request, $exception, $this->retryStorage[$chainIdentifier]);
102104
usleep($time);
103105

104-
// Retry in synchrone
106+
// Retry synchronously
105107
++$this->retryStorage[$chainIdentifier];
106108
$promise = $this->handleRequest($request, $next, $first);
107109

0 commit comments

Comments
 (0)