diff --git a/README.md b/README.md index 336ea634..02fc5aaf 100755 --- a/README.md +++ b/README.md @@ -131,6 +131,23 @@ The allowed options are * page * sort_order +### Retrying Requests + +Add the `RetryHandler` middleware on the `HandlerStack` of your `GuzzleHttp\Client` instance. By default `Zendesk\Api\HttpClient` +retries: +* timeout requests +* those that throw `Psr\Http\Message\RequestInterface\ConnectException:class` +* and those that throw `Psr\Http\Message\RequestInterface\RequestException:class` that are identified as ssl issue. + +#### Available options +Options are passed on `RetryHandler` as an array of values. + +* max = 2 _limit of retries_ +* interval = 300 _base delay between retries in milliseconds_ +* max_interval = 20000 _maximum delay value_ +* backoff_factor = 1 _backoff factor_ +* exceptions = [ConnectException::class] _Exceptions to retry without checking retry_if_ +* retry_if = null _callable function that can decide whether to retry the request or not_ ## Copyright and license diff --git a/src/Zendesk/API/HttpClient.php b/src/Zendesk/API/HttpClient.php index a3aff509..e550e258 100644 --- a/src/Zendesk/API/HttpClient.php +++ b/src/Zendesk/API/HttpClient.php @@ -7,7 +7,10 @@ * spl_autoload_register(function($c){@include 'src/'.preg_replace('#\\\|_(?!.+\\\)#','/',$c).'.php';}); */ +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\HandlerStack; use Zendesk\API\Exceptions\AuthException; +use Zendesk\API\Middleware\RetryHandler; use Zendesk\API\Resources\Core\Activities; use Zendesk\API\Resources\Core\AppInstallations; use Zendesk\API\Resources\Core\Apps; @@ -186,7 +189,11 @@ public function __construct( $guzzle = null ) { if (is_null($guzzle)) { - $this->guzzle = new \GuzzleHttp\Client(); + $handler = HandlerStack::create(); + $handler->push(new RetryHandler(['retry_if' => function ($retries, $request, $response, $e) { + return $e instanceof RequestException && strpos($e->getMessage(), 'ssl') !== false; + }]), 'retry_handler'); + $this->guzzle = new \GuzzleHttp\Client(compact('handler')); } else { $this->guzzle = $guzzle; } diff --git a/src/Zendesk/API/Middleware/RetryHandler.php b/src/Zendesk/API/Middleware/RetryHandler.php new file mode 100644 index 00000000..d81a0956 --- /dev/null +++ b/src/Zendesk/API/Middleware/RetryHandler.php @@ -0,0 +1,105 @@ + 2, // limit of retries + 'interval' => 300, // base delay between retries, unit is in milliseconds + 'max_interval' => 20000, // maximum delay value + 'backoff_factor' => 1, // backoff factor + 'exceptions' => [ConnectException::class], // Exceptions to retry without checking retry_if + 'retry_if' => null, // callable function that can decide whether to retry the request or not + ]; + + /** + * RetryHandler constructor. + * + * @param array $config + */ + public function __construct(array $config = []) + { + $this->options = array_merge($this->options, $config); + } + + /** + * Returns the function that will decide whether to retry the request or not. + * + * @return callable + */ + public function shouldRetryRequest() + { + return function ($retries, Request $request, $response, $exception) { + if ($retries >= $this->options['max']) { + return false; + } elseif ($this->isRetryableException($exception)) { + return true; + } elseif (is_callable($this->options['retry_if'])) { + return call_user_func($this->options['retry_if'], $retries, $request, $response, $exception); + } + + return $response && in_array($response->getStatusCode(), $this->timeoutCodes); + }; + } + + /** + * Returns the function that computes the delay before the next retry + * + * @return callable + */ + public function delay() + { + return function ($retries) { + $current_interval = $this->options['interval'] * pow($this->options['backoff_factor'], $retries); + $current_interval = min([$current_interval, $this->options['max_interval']]); + + return $current_interval; + }; + } + + /** + * Called when the middleware is handled by the client. + * + * @param callable $handler + * + * @return RetryMiddleware + */ + public function __invoke(callable $handler) + { + $retryMiddleware = new RetryMiddleware($this->shouldRetryRequest(), $handler, $this->delay()); + + return $retryMiddleware; + } + + /** + * Checks if the exception thrown warrants a retry + * + * @param $exception + * + * @return bool + */ + private function isRetryableException($exception) + { + if (!$this->options['exceptions']) { + return true; + } + + foreach ($this->options['exceptions'] as $expectedException) { + if ($exception instanceof $expectedException) { + return true; + } + } + + return false; + } +} diff --git a/tests/Zendesk/API/UnitTests/BasicTest.php b/tests/Zendesk/API/UnitTests/BasicTest.php index 206eca66..6568a0cb 100644 --- a/tests/Zendesk/API/UnitTests/BasicTest.php +++ b/tests/Zendesk/API/UnitTests/BasicTest.php @@ -85,8 +85,10 @@ protected function setUp() * * @param array $responses * An array of GuzzleHttp\Psr7\Response objects + * @param array $config + * config for the GuzzleHttp\Client */ - protected function mockApiResponses($responses = []) + protected function mockApiResponses($responses = [], array $config = []) { if (empty($responses)) { return; @@ -96,11 +98,16 @@ protected function mockApiResponses($responses = []) $history = Middleware::history($this->mockedTransactionsContainer); $mock = new MockHandler($responses); - $handler = HandlerStack::create($mock); - $handler->push($history); - - $this->client->guzzle = new Client(['handler' => $handler]); + $handlerStack = HandlerStack::create($mock); + $handlerStack->push($history); + if (isset($config['handlers'])) { + foreach ($config['handlers'] as $handler) { + $handlerStack->push($handler); + } + } + $config['handler'] = $handlerStack; + return $this->client->guzzle = new Client($config); } /** diff --git a/tests/Zendesk/API/UnitTests/Middleware/RetryHandlerTest.php b/tests/Zendesk/API/UnitTests/Middleware/RetryHandlerTest.php new file mode 100644 index 00000000..4aca6a1d --- /dev/null +++ b/tests/Zendesk/API/UnitTests/Middleware/RetryHandlerTest.php @@ -0,0 +1,248 @@ + 1, + 'exceptions' => [ServerException::class, ClientException::class], + 'retry_if' => function () { + return false; + } + ]; + $client = $this->mockApiResponses([ + new $exception('', new Request('GET', '')), + new Response(200), + ], ['handlers' => [ + new RetryHandler($config) + ]]); + + $this->checkRequest($client, $success, $exception); + } + + /** + * Samples for testExceptionsRetry + * + * @return array + */ + public function requestExceptionsProvider() + { + return [ + [ServerException::class, true], + [ClientException::class, true], + [ConnectException::class, false], + [TooManyRedirectsException::class, false] + ]; + } + + /** + * Checks that the max number of retries behaves properly + * + * @param int $limit + * @param bool $success + * + * @dataProvider retryLimitProvider + */ + public function testRetryLimit($limit, $success) + { + $config = [ + 'max' => $limit + ]; + + $responses = []; + do { + $responses[] = new ConnectException('', new Request('GET', '')); + } while (count($responses) < 10); + $responses[] = new Response(200); + $client = $this->mockApiResponses($responses, ['handlers' => [ + new RetryHandler($config) + ]]); + + $this->checkRequest($client, $success); + } + + /** + * Samples for testRetryLimit + * + * @return array + */ + public function retryLimitProvider() + { + return [ + [-10, false], // negative value should not retry requests + [0, false], // zero value should not retry requests + [5, false], // value lesser than the number of errors should fail + [10, true], // value equal to the number of errors should eventually succeed on the request + [12, true] // value greater than the number of errors should eventually succeed on the request + ]; + } + + /** + * Checks that the retry_if is used to decide the retry + * + * @param callable $retryIf + * @param bool $success + * + * @dataProvider retryIfProvider + */ + public function testRetryIf($retryIf, $success) + { + $config = [ + 'max' => 1, + 'retry_if' => $retryIf + ]; + + $client = $this->mockApiResponses([ + new Response(500), + new Response(200) + ], ['handlers' => [ + new RetryHandler($config) + ]]); + + $this->checkRequest($client, $success, ServerException::class); + } + + /** + * Samples for testRetryIf + * + * @return array + */ + public function retryIfProvider() + { + return [ + // check if retry_if is called with appropriate parameters + [function ($retries, $request, $response, $exception) { + return $request instanceof Request && + $response instanceof Response && + $response->getStatusCode() == 500 && + is_null($exception); + }, true], + + // check if retry_if is really used to decide the retry + [function () { + return false; + }, false] + ]; + } + + /** + * Checks that the delay between retries is correctly computed + * + * @param int $maxInterval maximum interval + * @param int $backoffFactor backoff factor + * @param int $shouldConsume expected consumed time + * @param string $message + * + * @dataProvider retryDelayProvider + */ + public function testRetryDelay($maxInterval, $backoffFactor, $shouldConsume, $message) + { + $config = [ + 'max' => 3, + 'interval' => 100, + 'max_interval' => $maxInterval, + 'backoff_factor' => $backoffFactor, + ]; + $i = 0; + $responses = []; + do { + $responses[] = new ConnectException($i++, new Request('GET', '')); + } while (count($responses) < 3); + $responses[] = new Response(200); + $client = $this->mockApiResponses($responses, ['handlers' => [ + new RetryHandler($config) + ]]); + + $start = microtime(true); + $client->get('/'); + $timeConsumed = round(microtime(true) - $start, 3) * 1000; + // round to the nearest 100 to remove noise from executing other statements + $this->assertEquals($shouldConsume, round($timeConsumed, -2), $message); + } + + /** + * Samples for testRetryDelay + * + * @return array + */ + public function retryDelayProvider() + { + return [ + [20000, 1, 300, 'for each request delays should be 100ms'], // all delays are 100ms + [20000, 2, 1400, 'delay should have an exponential growth'], // for each retry delays are 200, 400, 800 ms + [1000, 3, 2200, 'delay should not exceed max interval'] // for each retry delays are 300, 900, 1000 ms + ]; + } + + /** + * Tests that by default the Zendesk\API\HttpClient retries + * requests that failed because of ssl issue + */ + public function testHttpClientRetry() + { + $this->setUp(); + $config = $this->client->guzzle->getConfig(); + $sslException = new RequestException( + 'ssl', + $this->getMockBuilder(Request::class) + ->disableOriginalConstructor() + ->getMock() + ); + + $mock = new MockHandler([ + $sslException, + $sslException, + new Response() + ]); + + $config['handler']->setHandler(HandlerStack::create($mock)); + $client = new Client($config); + $response = $client->get('/'); + $this->assertEquals(200, $response->getStatusCode()); + } + + /** + * Checks if the request on the client will be successful or not + * + * @param Client $client + * @param bool $success + * @param \Exception $exception + */ + private function checkRequest(Client $client, $success, $exception = ConnectException::class) + { + if (!$success) { + $this->setExpectedException($exception); + } + + $response = $client->get('/'); + + if ($success) { + $this->assertEquals(200, $response->getStatusCode()); + } + } +}