diff --git a/BigQuery/src/BigQueryClient.php b/BigQuery/src/BigQueryClient.php index 5b4760d3ee0d..e38b1f723970 100644 --- a/BigQuery/src/BigQueryClient.php +++ b/BigQuery/src/BigQueryClient.php @@ -46,8 +46,10 @@ class BigQueryClient { use ArrayTrait; - use ClientTrait; - use RetryDeciderTrait; + use ClientTrait, RetryDeciderTrait { + ClientTrait::jsonEncode insteadof RetryDeciderTrait; + ClientTrait::jsonDecode insteadof RetryDeciderTrait; + } const VERSION = '1.5.0'; @@ -124,7 +126,7 @@ public function __construct(array $config = []) 'projectIdRequired' => true, 'returnInt64AsObject' => false, 'restRetryFunction' => $this->getRetryFunction(), - 'restDelayFunction' => function ($attempt) { + 'restCalcDelayFunction' => function ($attempt) { return min( mt_rand(0, 1000000) + (pow(2, $attempt) * 1000000), self::MAX_DELAY_MICROSECONDS diff --git a/Core/src/ExponentialBackoff.php b/Core/src/ExponentialBackoff.php index f0d2c829b383..0b41262c51af 100644 --- a/Core/src/ExponentialBackoff.php +++ b/Core/src/ExponentialBackoff.php @@ -30,7 +30,7 @@ class ExponentialBackoff private $retries; /** - * @var callable + * @var callable|null */ private $retryFunction; @@ -39,6 +39,11 @@ class ExponentialBackoff */ private $delayFunction; + /** + * @var callable|null + */ + private $calcDelayFunction; + /** * @param int $retries [optional] Number of retries for a failed request. * @param callable $retryFunction [optional] returns bool for whether or not to retry @@ -66,6 +71,7 @@ public function __construct($retries = null, callable $retryFunction = null) public function execute(callable $function, array $arguments = []) { $delayFunction = $this->delayFunction; + $calcDelayFunction = $this->calcDelayFunction ?: [$this, 'calculateDelay']; $retryAttempt = 0; $exception = null; @@ -83,7 +89,7 @@ public function execute(callable $function, array $arguments = []) break; } - $delayFunction($this->calculateDelay($retryAttempt)); + $delayFunction($calcDelayFunction($retryAttempt)); $retryAttempt++; } } @@ -92,6 +98,8 @@ public function execute(callable $function, array $arguments = []) } /** + * If not set, defaults to using `usleep`. + * * @param callable $delayFunction * @return void */ @@ -100,6 +108,18 @@ public function setDelayFunction(callable $delayFunction) $this->delayFunction = $delayFunction; } + /** + * If not set, defaults to using + * {@see Google\Cloud\Core\ExponentialBackoff::calculateDelay()}. + * + * @param callable $calcDelayFunction + * @return void + */ + public function setCalcDelayFunction(callable $calcDelayFunction) + { + $this->calcDelayFunction = $calcDelayFunction; + } + /** * Calculates exponential delay. * diff --git a/Core/src/Logger/AppEngineFlexFormatter.php b/Core/src/Logger/AppEngineFlexFormatter.php index f3697e75102b..707584b18c2e 100644 --- a/Core/src/Logger/AppEngineFlexFormatter.php +++ b/Core/src/Logger/AppEngineFlexFormatter.php @@ -24,8 +24,6 @@ */ class AppEngineFlexFormatter extends LineFormatter { - use JsonTrait; - /** * @param string $format [optional] The format of the message * @param string $dateFormat [optional] The format of the timestamp @@ -62,6 +60,6 @@ public function format(array $record) $_SERVER['HTTP_X_CLOUD_TRACE_CONTEXT'] )[0]; } - return "\n" . $this->jsonEncode($payload); + return "\n" . json_encode($payload); } } diff --git a/Core/src/RequestWrapper.php b/Core/src/RequestWrapper.php index 3e96aaae3366..0aa937d02626 100644 --- a/Core/src/RequestWrapper.php +++ b/Core/src/RequestWrapper.php @@ -34,7 +34,6 @@ */ class RequestWrapper { - use JsonTrait; use RequestWrapperTrait; use RetryDeciderTrait; @@ -82,16 +81,15 @@ class RequestWrapper private $retryFunction; /** - * @var callable|null Sets the conditions for determining how long to wait - * between attempts to retry. + * @var callable Executes a delay. */ - private $restDelayFunction; + private $delayFunction; /** - * @var callable Sets the conditions for determining how long to wait + * @var callable|null Sets the conditions for determining how long to wait * between attempts to retry. */ - private $delayFunction; + private $calcDelayFunction; /** * @param array $config [optional] { @@ -115,9 +113,14 @@ class RequestWrapper * @type array $restOptions HTTP client specific configuration options. * @type bool $shouldSignRequest Whether to enable request signing. * @type callable $restRetryFunction Sets the conditions for whether or - * not a request should attempt to retry. - * @type callable $restDelayFunction Sets the conditions for determining - * how long to wait between attempts to retry. + * not a request should attempt to retry. Function signature should + * match: `function (\Exception $ex) : bool`. + * @type callable $restDelayFunction Executes a delay, defaults to + * utilizing `usleep`. Function signature should match: + * `function (int $delay) : void`. + * @type callable $restCalcDelayFunction Sets the conditions for + * determining how long to wait between attempts to retry. Function + * signature should match: `function (int $attempt) : int`. * } */ public function __construct(array $config = []) @@ -132,7 +135,8 @@ public function __construct(array $config = []) 'shouldSignRequest' => true, 'componentVersion' => null, 'restRetryFunction' => null, - 'restDelayFunction' => null + 'restDelayFunction' => null, + 'restCalcDelayFunction' => null ]; $this->componentVersion = $config['componentVersion']; @@ -143,6 +147,7 @@ public function __construct(array $config = []) $this->delayFunction = $config['restDelayFunction'] ?: function ($delay) { usleep($delay); }; + $this->calcDelayFunction = $config['restCalcDelayFunction']; $this->httpHandler = $config['httpHandler'] ?: HttpHandlerFactory::build(); $this->authHttpHandler = $config['authHttpHandler'] ?: $this->httpHandler; $this->asyncHttpHandler = $config['asyncHttpHandler'] ?: $this->buildDefaultAsyncHandler(); @@ -164,9 +169,14 @@ public function __construct(array $config = []) * @type int $retries Number of retries for a failed request. * **Defaults to** `3`. * @type callable $restRetryFunction Sets the conditions for whether or - * not a request should attempt to retry. - * @type callable $restDelayFunction Sets the conditions for determining - * how long to wait between attempts to retry. + * not a request should attempt to retry. Function signature should + * match: `function (\Exception $ex) : bool`. + * @type callable $restDelayFunction Executes a delay, defaults to + * utilizing `usleep`. Function signature should match: + * `function (int $delay) : void`. + * @type callable $restCalcDelayFunction Sets the conditions for + * determining how long to wait between attempts to retry. Function + * signature should match: `function (int $attempt) : int`. * @type array $restOptions HTTP client specific configuration options. * } * @return ResponseInterface @@ -174,12 +184,19 @@ public function __construct(array $config = []) public function send(RequestInterface $request, array $options = []) { $retryOptions = $this->getRetryOptions($options); - $backoff = $this->configureBackoff( + $backoff = new ExponentialBackoff( $retryOptions['retries'], - $retryOptions['retryFunction'], - $retryOptions['delayFunction'] + $retryOptions['retryFunction'] ); + if ($retryOptions['delayFunction']) { + $backoff->setDelayFunction($retryOptions['delayFunction']); + } + + if ($retryOptions['calcDelayFunction']) { + $backoff->setCalcDelayFunction($retryOptions['calcDelayFunction']); + } + try { return $backoff->execute($this->httpHandler, [ $this->applyHeaders($request), @@ -202,9 +219,14 @@ public function send(RequestInterface $request, array $options = []) * @type int $retries Number of retries for a failed request. * **Defaults to** `3`. * @type callable $restRetryFunction Sets the conditions for whether or - * not a request should attempt to retry. - * @type callable $restDelayFunction Sets the conditions for determining - * how long to wait between attempts to retry. + * not a request should attempt to retry. Function signature should + * match: `function (\Exception $ex) : bool`. + * @type callable $restDelayFunction Executes a delay, defaults to + * utilizing `usleep`. Function signature should match: + * `function (int $delay) : void`. + * @type callable $restCalcDelayFunction Sets the conditions for + * determining how long to wait between attempts to retry. Function + * signature should match: `function (int $attempt) : int`. * @type array $restOptions HTTP client specific configuration options. * } * @return PromiseInterface @@ -221,6 +243,9 @@ public function sendAsync(RequestInterface $request, array $options = []) $fn = function ($retryAttempt) use (&$fn, $request, $options) { $asyncHttpHandler = $this->asyncHttpHandler; $retryOptions = $this->getRetryOptions($options); + if (!$retryOptions['calcDelayFunction']) { + $retryOptions['calcDelayFunction'] = [ExponentialBackoff::class, 'calculateDelay']; + } return $asyncHttpHandler( $this->applyHeaders($request), @@ -232,7 +257,8 @@ public function sendAsync(RequestInterface $request, array $options = []) throw $this->convertToGoogleException($ex); } - $retryOptions['delayFunction'](ExponentialBackoff::calculateDelay($retryAttempt)); + $delay = $retryOptions['calcDelayFunction']($retryAttempt); + $retryOptions['delayFunction']($delay); $retryAttempt++; return $fn($retryAttempt); @@ -339,13 +365,14 @@ private function convertToGoogleException(\Exception $ex) /** * Gets the exception message. * + * @access private * @param \Exception $ex * @return string */ private function getExceptionMessage(\Exception $ex) { if ($ex instanceof RequestException && $ex->hasResponse()) { - $res = (string) $ex->getResponse()->getBody(); + return (string) $ex->getResponse()->getBody(); try { $this->jsonDecode($res); @@ -358,25 +385,6 @@ private function getExceptionMessage(\Exception $ex) return $ex->getMessage(); } - /** - * Configures an exponential backoff implementation. - * - * @param int $retries - * @param callable $retryFunction - * @param callable $delayFunction - * @return ExponentialBackoff - */ - private function configureBackoff($retries, callable $retryFunction, callable $delayFunction) - { - $backoff = new ExponentialBackoff($retries, $retryFunction); - - if ($delayFunction) { - $backoff->setDelayFunction($delayFunction); - } - - return $backoff; - } - /** * Gets a set of request options. * @@ -416,7 +424,10 @@ private function getRetryOptions(array $options) : $this->retryFunction, 'delayFunction' => isset($options['restDelayFunction']) ? $options['restDelayFunction'] - : $this->delayFunction + : $this->delayFunction, + 'calcDelayFunction' => isset($options['restCalcDelayFunction']) + ? $options['restCalcDelayFunction'] + : $this->calcDelayFunction ]; } diff --git a/Core/src/RetryDeciderTrait.php b/Core/src/RetryDeciderTrait.php index 87accace8bc9..d0d9fbde22b3 100644 --- a/Core/src/RetryDeciderTrait.php +++ b/Core/src/RetryDeciderTrait.php @@ -17,11 +17,15 @@ namespace Google\Cloud\Core; +use GuzzleHttp\Exception\RequestException; + /** * Provides methods for deciding if a request should be retried. */ trait RetryDeciderTrait { + use JsonTrait; + /** * @var array */ @@ -62,7 +66,18 @@ private function getRetryFunction($shouldRetryMessages = true) return false; } - $message = json_decode($ex->getMessage(), true); + $message = ($ex instanceof RequestException && $ex->hasResponse()) + ? (string) $ex->getResponse()->getBody() + : $ex->getMessage(); + + try { + $message = $this->jsonDecode( + $message, + true + ); + } catch (\InvalidArgumentException $ex) { + return false; + } if (!isset($message['error']['errors'])) { return false; diff --git a/Core/tests/Unit/ExponentialBackoffTest.php b/Core/tests/Unit/ExponentialBackoffTest.php index b71876661edf..eff5a4f7bdcb 100644 --- a/Core/tests/Unit/ExponentialBackoffTest.php +++ b/Core/tests/Unit/ExponentialBackoffTest.php @@ -112,6 +112,31 @@ public function testSuccessWithNoRetries() $this->assertEquals(1, $actualAttempts); } + public function testSetsCalculateDelayFunction() + { + $backoff = new ExponentialBackoff(); + $hasTriggeredException = false; + $actualDelayAmount = 0; + $expectedDelayAmount = 100; + $backoff->setDelayFunction(function ($delay) use (&$actualDelayAmount) { + $actualDelayAmount = $delay; + }); + $backoff->setCalcDelayFunction(function () use ($expectedDelayAmount) { + return $expectedDelayAmount; + }); + + try { + $backoff->execute(function () { + throw new \Exception(); + }); + } catch (\Exception $ex) { + $hasTriggeredException = true; + } + + $this->assertTrue($hasTriggeredException); + $this->assertEquals($expectedDelayAmount, $actualDelayAmount); + } + /** * @dataProvider delayProvider */ diff --git a/Core/tests/Unit/RequestWrapperTest.php b/Core/tests/Unit/RequestWrapperTest.php index 8a049d47ce43..0d38a6e1def1 100644 --- a/Core/tests/Unit/RequestWrapperTest.php +++ b/Core/tests/Unit/RequestWrapperTest.php @@ -430,6 +430,40 @@ public function testThrowsExceptionWithNonRetryableError() $this->assertEquals(1, $actualAttempts); } + public function testRestCalcDelayFunctionIsProperlySet() + { + $hasTriggeredException = false; + $actualDelay = 0; + $expectedDelay = 100; + $requestWrapper = new RequestWrapper([ + 'retries' => 1, + 'httpHandler' => function () { + throw new \Exception; + }, + 'restRetryFunction' => function () { + return true; + }, + 'restDelayFunction' => function ($delay) use (&$actualDelay) { + $actualDelay = $delay; + }, + 'restCalcDelayFunction' => function () use ($expectedDelay) { + return $expectedDelay; + }, + 'shouldSignRequest' => false + ]); + + try { + $requestWrapper->send( + new Request('GET', 'http://www.example.com') + ); + } catch (\Exception $ex) { + $hasTriggeredException = true; + } + + $this->assertTrue($hasTriggeredException); + $this->assertEquals($expectedDelay, $actualDelay); + } + public function testDisablesRequestSigningWithAnonymousCredentials() { $headers = []; diff --git a/Core/tests/Unit/RetryDeciderTraitTest.php b/Core/tests/Unit/RetryDeciderTraitTest.php index 6a86939cfee1..f25fee01e1f2 100644 --- a/Core/tests/Unit/RetryDeciderTraitTest.php +++ b/Core/tests/Unit/RetryDeciderTraitTest.php @@ -19,6 +19,9 @@ use Google\Cloud\Core\RetryDeciderTrait; use Google\Cloud\Core\Testing\TestHelpers; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; /** @@ -60,8 +63,21 @@ public function retryProvider() $rateLimitExceededMessage = '{"error": {"errors": [{"reason": "rateLimitExceeded"}]}}'; $userRateLimitExceededMessage = '{"error": {"errors": [{"reason": "userRateLimitExceeded"}]}}'; $notAGoodMessage = '{"error": {"errors": [{"reason": "notAGoodReason"}]}}'; + $rateLimitExceededResponse = new Response(400, [], $rateLimitExceededMessage); + $notAGoodMessageResponse = new Response(400, [], $notAGoodMessage); + $request = new Request('GET', 'https://www.example.com'); return [ + [ + RequestException::create($request, $rateLimitExceededResponse), + true, + true + ], + [ + RequestException::create($request, $notAGoodMessageResponse), + true, + false + ], [new \Exception('', 400), true, false], [new \Exception('', 500), true, true], [new \Exception('', 502), true, true], diff --git a/Language/src/LanguageClient.php b/Language/src/LanguageClient.php index bfdd7de79198..0c9c17946b0a 100644 --- a/Language/src/LanguageClient.php +++ b/Language/src/LanguageClient.php @@ -41,8 +41,10 @@ */ class LanguageClient { - use ClientTrait; - use RetryDeciderTrait; + use ClientTrait, RetryDeciderTrait { + ClientTrait::jsonEncode insteadof RetryDeciderTrait; + ClientTrait::jsonDecode insteadof RetryDeciderTrait; + } const VERSION = '0.18.0'; diff --git a/Storage/src/Connection/Rest.php b/Storage/src/Connection/Rest.php index 18e9bdc4882d..e5e7ff9dc4b6 100644 --- a/Storage/src/Connection/Rest.php +++ b/Storage/src/Connection/Rest.php @@ -421,6 +421,7 @@ private function buildDownloadObjectParams(array $args) 'restOptions' => null, 'retries' => null, 'restRetryFunction' => null, + 'restCalcDelayFunction' => null, 'restDelayFunction' => null ]);