From fdd6afac257fb7a4dc5450f3ce08d8c35269a240 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 1 Jul 2017 19:26:22 +0200 Subject: [PATCH 1/6] Allow a callable parameter to decide if require is going to be retried or not. Also allow retry backoff --- src/Plugin/RetryPlugin.php | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index bbb1ffa..1b3e930 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -24,6 +24,16 @@ final class RetryPlugin implements Plugin */ private $retry; + /** + * @var callable + */ + private $delay; + + /** + * @var callable + */ + private $decider; + /** * Store the retry counter for each request. * @@ -35,6 +45,8 @@ final class RetryPlugin implements Plugin * @param array $config { * * @var int $retries Number of retries to attempt if an exception occurs before letting the exception bubble up. + * @var callable $decider A callback that gets a request and an exception to decide if we should retry this or not. + * @var callable $delay A callback to return how many milliseconds we should wait before trying again. * } */ public function __construct(array $config = []) @@ -42,11 +54,21 @@ public function __construct(array $config = []) $resolver = new OptionsResolver(); $resolver->setDefaults([ 'retries' => 1, + 'decider' => function(RequestInterface $request, Exception $e) { + return true; + }, + 'delay' => function(RequestInterface $request, Exception $e, $retries) { + return ((int) pow(2, $retries - 1)) * 1000; + }, ]); $resolver->setAllowedTypes('retries', 'int'); + $resolver->setAllowedTypes('decider', 'callable'); + $resolver->setAllowedTypes('delay', 'callable'); $options = $resolver->resolve($config); $this->retry = $options['retries']; + $this->decider = $options['decider']; + $this->delay = $options['delay']; } /** @@ -73,7 +95,12 @@ public function handleRequest(RequestInterface $request, callable $next, callabl throw $exception; } - ++$this->retryStorage[$chainIdentifier]; + if (!call_user_func($this->decider, $request, $exception)) { + throw $exception; + } + + $time = call_user_func($this->delay, $request, $exception, ++$this->retryStorage[$chainIdentifier]); + usleep($time); // Retry in synchrone $promise = $this->handleRequest($request, $next, $first); From cb37fad4f78f7b6b11f27fc0ebe1027c23936ce1 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sun, 2 Jul 2017 02:12:26 +0200 Subject: [PATCH 2/6] Applied changes from StyleCI --- src/Plugin/RetryPlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index 1b3e930..1e478f0 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -54,10 +54,10 @@ public function __construct(array $config = []) $resolver = new OptionsResolver(); $resolver->setDefaults([ 'retries' => 1, - 'decider' => function(RequestInterface $request, Exception $e) { + 'decider' => function (RequestInterface $request, Exception $e) { return true; }, - 'delay' => function(RequestInterface $request, Exception $e, $retries) { + 'delay' => function (RequestInterface $request, Exception $e, $retries) { return ((int) pow(2, $retries - 1)) * 1000; }, ]); From 98941f579bbbb28833818719bea8c48cde845aa3 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 3 Jul 2017 08:51:27 +0200 Subject: [PATCH 3/6] Some fixes --- spec/Plugin/RetryPluginSpec.php | 8 ++++++++ src/Plugin/RetryPlugin.php | 23 +++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/spec/Plugin/RetryPluginSpec.php b/spec/Plugin/RetryPluginSpec.php index 981c16d..1f4de0b 100644 --- a/spec/Plugin/RetryPluginSpec.php +++ b/spec/Plugin/RetryPluginSpec.php @@ -101,4 +101,12 @@ function it_does_not_keep_history_of_old_failure(RequestInterface $request, Resp $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); } + + function it_has_a_good_default_delay(RequestInterface $request, Exception\HttpException $exception) + { + $this->defaultDelay($request, $exception, 0)->shouldBe(1000); + $this->defaultDelay($request, $exception, 1)->shouldBe(2000); + $this->defaultDelay($request, $exception, 2)->shouldBe(4000); + $this->defaultDelay($request, $exception, 3)->shouldBe(8000); + } } diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index 1e478f0..d163a1d 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -45,8 +45,8 @@ final class RetryPlugin implements Plugin * @param array $config { * * @var int $retries Number of retries to attempt if an exception occurs before letting the exception bubble up. - * @var callable $decider A callback that gets a request and an exception to decide if we should retry this or not. - * @var callable $delay A callback to return how many milliseconds we should wait before trying again. + * @var callable $decider A callback that gets a request and an exception to decide after a failure whether the request should be retried. + * @var callable $delay A callback that gets a request, an exception and the number of retries and returns how many milliseconds we should wait before trying again. * } */ public function __construct(array $config = []) @@ -57,9 +57,7 @@ public function __construct(array $config = []) 'decider' => function (RequestInterface $request, Exception $e) { return true; }, - 'delay' => function (RequestInterface $request, Exception $e, $retries) { - return ((int) pow(2, $retries - 1)) * 1000; - }, + 'delay' => __CLASS__.'::defaultDelay', ]); $resolver->setAllowedTypes('retries', 'int'); $resolver->setAllowedTypes('decider', 'callable'); @@ -99,13 +97,26 @@ public function handleRequest(RequestInterface $request, callable $next, callabl throw $exception; } - $time = call_user_func($this->delay, $request, $exception, ++$this->retryStorage[$chainIdentifier]); + $time = call_user_func($this->delay, $request, $exception, $this->retryStorage[$chainIdentifier]); usleep($time); // Retry in synchrone + $this->retryStorage[$chainIdentifier]++; $promise = $this->handleRequest($request, $next, $first); return $promise->wait(); }); } + + /** + * @param RequestInterface $request + * @param Exception $e + * @param int $retries The number of retries we made before. First time this get called it will be 0. + * + * @return int + */ + public static function defaultDelay(RequestInterface $request, Exception $e, $retries) + { + return ((int) pow(2, $retries)) * 1000; + } } From 5e80c64341231c6f82c51eca744beafcc29e301e Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 3 Jul 2017 09:00:05 +0200 Subject: [PATCH 4/6] Applied changes from StyleCI --- src/Plugin/RetryPlugin.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index d163a1d..6520366 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -57,7 +57,7 @@ public function __construct(array $config = []) 'decider' => function (RequestInterface $request, Exception $e) { return true; }, - 'delay' => __CLASS__.'::defaultDelay', + 'delay' => __CLASS__.'::defaultDelay', ]); $resolver->setAllowedTypes('retries', 'int'); $resolver->setAllowedTypes('decider', 'callable'); @@ -101,7 +101,7 @@ public function handleRequest(RequestInterface $request, callable $next, callabl usleep($time); // Retry in synchrone - $this->retryStorage[$chainIdentifier]++; + ++$this->retryStorage[$chainIdentifier]; $promise = $this->handleRequest($request, $next, $first); return $promise->wait(); @@ -110,8 +110,8 @@ public function handleRequest(RequestInterface $request, callable $next, callabl /** * @param RequestInterface $request - * @param Exception $e - * @param int $retries The number of retries we made before. First time this get called it will be 0. + * @param Exception $e + * @param int $retries The number of retries we made before. First time this get called it will be 0. * * @return int */ From bd9a5a40670dd75295be0e10006db3bc1d46c177 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 4 Jul 2017 14:57:50 +0200 Subject: [PATCH 5/6] Updated changelog --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a89388..425fb3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## Unreleased +### Added + +- Added new option 'delay' for `RetryPlugin`. +- Added new option 'decider' for `RetryPlugin`. + +### Changed + +- The `RetryPlugin` does now wait between retries. To disable/change this feature you must write something like: + +```php +$plugin = new RetryPlugin(['delay' => function(RequestInterface $request, Exception $e, $retries) { + return 0; +}); +``` + ### Deprecated - The `debug_plugins` option for `PluginClient` is deprecated and will be removed in 2.0. Use the decorator design pattern instead like in [ProfilePlugin](https://github.com/php-http/HttplugBundle/blob/de33f9c14252f22093a5ec7d84f17535ab31a384/Collector/ProfilePlugin.php). From 237561d35c5dd77f17030b9164924e3bed767e63 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 6 Jul 2017 09:27:36 +0200 Subject: [PATCH 6/6] Stuff --- spec/Plugin/RetryPluginSpec.php | 2 +- src/Plugin/RetryPlugin.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/Plugin/RetryPluginSpec.php b/spec/Plugin/RetryPluginSpec.php index 1f4de0b..269e90a 100644 --- a/spec/Plugin/RetryPluginSpec.php +++ b/spec/Plugin/RetryPluginSpec.php @@ -102,7 +102,7 @@ function it_does_not_keep_history_of_old_failure(RequestInterface $request, Resp $this->handleRequest($request, $next, function () {})->shouldReturnAnInstanceOf('Http\Client\Promise\HttpFulfilledPromise'); } - function it_has_a_good_default_delay(RequestInterface $request, Exception\HttpException $exception) + function it_has_an_exponential_default_delay(RequestInterface $request, Exception\HttpException $exception) { $this->defaultDelay($request, $exception, 0)->shouldBe(1000); $this->defaultDelay($request, $exception, 1)->shouldBe(2000); diff --git a/src/Plugin/RetryPlugin.php b/src/Plugin/RetryPlugin.php index 6520366..1a58004 100644 --- a/src/Plugin/RetryPlugin.php +++ b/src/Plugin/RetryPlugin.php @@ -117,6 +117,6 @@ public function handleRequest(RequestInterface $request, callable $next, callabl */ public static function defaultDelay(RequestInterface $request, Exception $e, $retries) { - return ((int) pow(2, $retries)) * 1000; + return pow(2, $retries) * 1000; } }