diff --git a/CHANGELOG.md b/CHANGELOG.md index c6a01d9..ac296f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## 2.0 (unreleased) ### Changed +- RetryPlugin will no longer retry requests when the response failed with a HTTP code < 500. - Abstract method `HttpClientPool::chooseHttpClient()` has now an explicit return type (`Http\Client\Common\HttpClientPoolItem`) - Interface method `Plugin::handleRequest(...)` has now an explicit return type (`Http\Promise\Promise`) diff --git a/spec/Plugin/RetryPluginSpec.php b/spec/Plugin/RetryPluginSpec.php index 682b892..1ffb70a 100644 --- a/spec/Plugin/RetryPluginSpec.php +++ b/spec/Plugin/RetryPluginSpec.php @@ -59,6 +59,28 @@ public function it_throws_exception_on_multiple_exceptions(RequestInterface $req $promise->shouldThrow($exception2)->duringWait(); } + public function it_does_not_retry_client_errors(RequestInterface $request, ResponseInterface $response) + { + $exception = new Exception\HttpException('Exception', $request->getWrappedObject(), $response->getWrappedObject()); + + $seen = false; + $next = function (RequestInterface $receivedRequest) use ($request, $exception, &$seen) { + if (!Argument::is($request->getWrappedObject())->scoreArgument($receivedRequest)) { + throw new \Exception('Unexpected request received'); + } + if ($seen) { + throw new \Exception('This should only be called once'); + } + $seen = true; + + return new HttpRejectedPromise($exception); + }; + + $promise = $this->handleRequest($request, $next, function () {}); + $promise->shouldReturnAnInstanceOf(HttpRejectedPromise::class); + $promise->shouldThrow($exception)->duringWait(); + } + public function it_returns_response_on_second_try(RequestInterface $request, ResponseInterface $response) { $exception = new Exception\NetworkException('Exception 1', $request->getWrappedObject()); diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index c10bce4..a0da75c 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -4,6 +4,7 @@ use Http\Client\Common\Plugin; use Http\Client\Exception; +use Http\Client\Exception\HttpException; use Http\Promise\Promise; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -56,7 +57,8 @@ public function __construct(array $config = []) $resolver->setDefaults([ 'retries' => 1, 'decider' => function (RequestInterface $request, Exception $e) { - return true; + // do not retry client errors + return !$e instanceof HttpException || $e->getCode() >= 500; }, 'delay' => __CLASS__.'::defaultDelay', ]); @@ -101,7 +103,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl $time = call_user_func($this->delay, $request, $exception, $this->retryStorage[$chainIdentifier]); usleep($time); - // Retry in synchrone + // Retry synchronously ++$this->retryStorage[$chainIdentifier]; $promise = $this->handleRequest($request, $next, $first);