feat(storage): retry SignBlob call for URL signing

.gitignore

@@ -17,3 +17,4 @@ keys/
 .testing
 .split
 __pycache__
+.vscode/

Storage/src/SigningHelper.php

@@ -23,6 +23,7 @@
 use Google\Cloud\Core\JsonTrait;
 use Google\Cloud\Core\Timestamp;
 use Google\Cloud\Storage\Connection\ConnectionInterface;
+use Google\Cloud\Core\Exception\ServiceException;
 
 /**
  * Provides common methods for signing storage URLs.
@@ -34,12 +35,23 @@ class SigningHelper
     use ArrayTrait;
     use JsonTrait;
 
-    const DEFAULT_URL_SIGNING_VERSION = 'v2';
-    const DEFAULT_DOWNLOAD_HOST = 'storage.googleapis.com';
+    public const DEFAULT_URL_SIGNING_VERSION = 'v2';
+    public const DEFAULT_DOWNLOAD_HOST = 'storage.googleapis.com';
 
-    const V4_ALGO_NAME = 'GOOG4-RSA-SHA256';
-    const V4_TIMESTAMP_FORMAT = 'Ymd\THis\Z';
-    const V4_DATESTAMP_FORMAT = 'Ymd';
+    public const V4_ALGO_NAME = 'GOOG4-RSA-SHA256';
+    public const V4_TIMESTAMP_FORMAT = 'Ymd\THis\Z';
+    public const V4_DATESTAMP_FORMAT = 'Ymd';
+
+    /**
+     * The HTTP codes that will be retried by our custom retry function.
+     * @var array
+     */
+    private static $httpRetryCodes = [
+        500,
+        502,
+        503,
+        504
+    ];
 
     /**
      * Create or fetch a SigningHelper instance.
@@ -50,7 +62,7 @@ public static function getHelper()
     {
         static $helper;
         if (!$helper) {
-            $helper = new static;
+            $helper = new static();
         }
 
         return $helper;
@@ -169,7 +181,7 @@ public function v2Sign(ConnectionInterface $connection, $expires, $resource, $ge
 
         $signedHeaders = [];
         foreach ($headers as $name => $value) {
-            $signedHeaders[] = $name .':'. $value;
+            $signedHeaders[] = $name . ':' . $value;
         }
 
         // Push the headers onto the end of the signing string.
@@ -181,9 +193,12 @@ public function v2Sign(ConnectionInterface $connection, $expires, $resource, $ge
 
         $stringToSign = $this->createV2CanonicalRequest($toSign);
 
-        $signature = $credentials->signBlob($stringToSign, [
-            'forceOpenssl' => $options['forceOpenssl']
-        ]);
+        // Use exponential backOff
+        $signature = $this->retrySignBlob(function () use ($credentials, $stringToSign, $options) {
+            return $credentials->signBlob($stringToSign, [
+                'forceOpenssl' => $options['forceOpenssl']
+            ]);
+        });
 
         // Start with user-provided query params and add required parameters.
         $params = $options['queryParams'];
@@ -337,9 +352,15 @@ public function v4Sign(ConnectionInterface $connection, $expires, $resource, $ge
             $requestHash
         ]);
 
-        $signature = bin2hex(base64_decode($credentials->signBlob($stringToSign, [
-            'forceOpenssl' => $options['forceOpenssl']
-        ])));
+        $signature = bin2hex(base64_decode($this->retrySignBlob(function () use (
+            $credentials,
+            $stringToSign,
+            $options
+        ) {
+            return $credentials->signBlob($stringToSign, [
+                'forceOpenssl' => $options['forceOpenssl']
+            ]);
+        })));
 
         // Construct the modified resource name. If a custom hostname is provided,
         // this will remove the bucket name from the resource.
@@ -677,8 +698,8 @@ private function normalizeOptions(array $options)
                 if (!$options['timestamp']) {
                     throw new \InvalidArgumentException(
                         'Given timestamp string is in an invalid format. Provide timestamp formatted as follows: `' .
-                        \DateTime::RFC3339 .
-                        '`. Note that timestamps MUST be in UTC.'
+                            \DateTime::RFC3339 .
+                            '`. Note that timestamps MUST be in UTC.'
                     );
                 }
             }
@@ -882,4 +903,46 @@ private function buildQueryString(array $input)
 
         return implode('&', $q);
     }
+
+    /**
+     *   Retry logic for signBlob
+     *
+     * @param callable $signBlobFn  A callable that perform the actual signBlob operation.
+     * @param int $maxRetries Maximum number of retry attempts.
+     * @param int $initialDelay Initial delay between retries in milliseconds.
+     * @return string The signature genarated by signBlob.
+     * @throws ServiceException If non-retryable error occur.
+     * @throws \RuntimeException If retries are exhausted.
+     */
+    private function retrySignBlob(callable $signBlobFn, int $maxRetries = 5, int $initialDelay = 100)
+    {
+        $attempts = 0;
+        $delay = $initialDelay;
+
+        while ($attempts < $maxRetries) {
+            try {
+                return $signBlobFn();
+            } catch (ServiceException $exception) {
+                $attempts++;
+                $statusCode = $exception->getCode();
+
+                // Retry if the exception status code matches
+                // with one of the retriable status code
+                if (in_array($statusCode, self::$httpRetryCodes)) {
+                    usleep($delay * 1000);
+
+                    $delay *= 2;    // Exponential backoff
+                    continue;
+                }
+
+                // Non-retryable error
+                throw $exception;
+            }
+        }
+
+        throw new \RuntimeException(sprintf(
+            'Failed to sign message after `%s` attempts.',
+            $attempts
+        ));
+    }
 }

Storage/tests/Unit/SigningHelperTest.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Copyright 2019 Google LLC
  *
@@ -19,6 +20,7 @@
 
 use Google\Auth\Credentials\ServiceAccountCredentials;
 use Google\Auth\SignBlobInterface;
+use Google\Cloud\Core\Exception\ServiceException;
 use Google\Cloud\Core\RequestWrapper;
 use Google\Cloud\Core\Testing\TestHelpers;
 use Google\Cloud\Core\Timestamp;
@@ -37,10 +39,10 @@ class SigningHelperTest extends TestCase
 {
     use ProphecyTrait;
 
-    const CLIENT_EMAIL = '[email protected]';
-    const BUCKET = 'test-bucket';
-    const OBJECT = 'test-object';
-    const GENERATION = 11111;
+    public const CLIENT_EMAIL = '[email protected]';
+    public const BUCKET = 'test-bucket';
+    public const OBJECT = 'test-object';
+    public const GENERATION = 11111;
 
     private $helper;
 
@@ -417,7 +419,7 @@ public function testV4SignInvalidExpiration()
     {
         $this->expectException(\InvalidArgumentException::class);
 
-        $expires = (new \DateTime)->modify('+20 days');
+        $expires = (new \DateTime())->modify('+20 days');
         $this->helper->v4Sign(
             $this->mockConnection($this->createCredentialsMock()->reveal()),
             $expires,
@@ -453,7 +455,7 @@ public function testExpirations($expiration, $expected)
 
     public function expirations()
     {
-        $tenMins = (new \DateTimeImmutable)->modify('+10 minutes');
+        $tenMins = (new \DateTimeImmutable())->modify('+10 minutes');
 
         return [
             [
@@ -789,6 +791,104 @@ private function mockConnection($credentials = null, $rw = null)
 
         return $conn->reveal();
     }
+
+    public function testRetrySignBlobSuccessFirstAttempt()
+    {
+        $signBlobFn = function () {
+            return 'signature';
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn,
+            5,
+            10
+        ]);
+
+        $this->assertEquals('signature', $res);
+    }
+
+    public function testRetrySignBlobSuccessAfterRetries()
+    {
+        $attempts = 0;
+        $signBlobFn = function () use (&$attempts) {
+            $attempts++;
+            if ($attempts < 5) {
+                throw new ServiceException('Transient error', 503);
+            }
+            return 'signature';
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn,
+            5,
+            10
+        ]);
+
+        $this->assertEquals('signature', $res);
+        $this->assertEquals(5, $attempts);
+    }
+
+    public function testRetrySignBlobNonRetryableError()
+    {
+        $this->expectException(ServiceException::class);
+        $this->getExpectedExceptionMessage('Non-retryable error');
+
+        $signBlobFn = function () use (&$attempt) {
+            throw new ServiceException('Non-retryable error', 400);
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn,
+            5,
+            10
+        ]);
+    }
+
+    public function testRetrySignBlobRetriesExhausted()
+    {
+        $this->expectException(\RuntimeException::class);
+        $this->getExpectedExceptionMessage('Failed to sign message after maximum attempts.');
+
+        $signBlobFn = function () use (&$attempt) {
+            throw new ServiceException('Transient error', 503);
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn,
+            5,
+            10
+        ]);
+    }
+
+    public function testRetrySignBlobExponentialBackoff()
+    {
+        $attempts = 0;
+        $delays = [];
+
+        $signBlobFn = function () use (&$attempts, &$delays) {
+            $attempts++;
+            if ($attempts < 5) {
+                // Mock usleep to record delays
+                $delays[] = $attempts * 1000;
+                throw new ServiceException('Transient error', 503);
+            }
+            return 'signature';
+        };
+
+        $originalUsleep = \Closure::fromCallable('usleep');
+        $mockUsleep = function ($microseconds) use (&$delays, $originalUsleep) {
+            $delays[] = $microseconds;
+        };
+
+        $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn,
+            5,
+            10
+        ]);
+
+        $expectedDelays = [1000, 2000, 3000, 4000];
+        $this->assertEquals($expectedDelays, $delays);
+    }
 }
 
 //@codingStandardsIgnoreStart
@@ -803,20 +903,20 @@ private function createV4CanonicalRequest(array $request)
         $callPrivate = $this->callPrivate('createV4CanonicalRequest', [$request]);
         return $this->createV4CanonicalRequest
             ? call_user_func($this->createV4CanonicalRequest, $request)
-            : \Closure::bind($callPrivate, null, new SigningHelper);
+            : \Closure::bind($callPrivate, null, new SigningHelper());
     }
 
     private function createV2CanonicalRequest(array $request)
     {
         $callPrivate = $this->callPrivate('createV2CanonicalRequest', [$request]);
         return $this->createV2CanonicalRequest
             ? call_user_func($this->createV2CanonicalRequest, $request)
-            : \Closure::bind($callPrivate, null, new SigningHelper);
+            : \Closure::bind($callPrivate, null, new SigningHelper());
     }
 
     public function proxyPrivateMethodCall($method, array $args)
     {
-        $parent = new SigningHelper;
+        $parent = new SigningHelper();
         $cb = function () use ($method) {
             return call_user_func_array([$this, $method], func_get_args()[0]);
         };