feat(storage): retry SignBlob call for URL signing (#7862)

.gitignore

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

Storage/src/Connection/RetryTrait.php

@@ -69,6 +69,7 @@ trait RetryTrait
         'objects.get',
         'objects.list',
         'serviceaccount.get',
+        'signBlob.execute'
     ];
 
     /**
@@ -134,7 +135,9 @@ private function getRestRetryFunction($resource, $method, array $args)
             StorageClient::RETRY_IDEMPOTENT;
 
         return function (
-            \Exception $exception
+            \Exception $exception,
+            $currentAttempt = 0,
+            $maxRetries = null
         ) use (
             $isOpIdempotent,
             $preconditionNeeded,
@@ -146,7 +149,9 @@ private function getRestRetryFunction($resource, $method, array $args)
                 $isOpIdempotent,
                 $preconditionNeeded,
                 $preconditionSupplied,
-                $retryStrategy
+                $retryStrategy,
+                $currentAttempt,
+                $maxRetries
             );
         };
     }
@@ -187,16 +192,24 @@ private function isPreConditionSupplied($methodName, array $args)
      * @param bool $isIdempotent
      * @param bool $preconditionNeeded
      * @param bool $preconditionSupplied
-     * @param int $maxRetries
+     * @param int|null $maxRetries The maximum number of retries allowd.
+     * Null for no limit.
      * @return bool
      */
     private function retryDeciderFunction(
         \Exception $exception,
         $isIdempotent,
         $preconditionNeeded,
         $preconditionSupplied,
-        $retryStrategy
+        $retryStrategy,
+        $currentAttempt = 0,
+        $maxRetries = null
     ) {
+        // If maxRetries is specified, ensure we don't exceed it
+        if ($maxRetries !== null && $currentAttempt >= $maxRetries) {
+            return false;
+        }
+
         if ($retryStrategy == StorageClient::RETRY_NEVER) {
             return false;
         }

Storage/src/SigningHelper.php

@@ -23,6 +23,8 @@
 use Google\Cloud\Core\JsonTrait;
 use Google\Cloud\Core\Timestamp;
 use Google\Cloud\Storage\Connection\ConnectionInterface;
+use Google\Cloud\Core\Exception\ServiceException;
+use Google\Cloud\Storage\Connection\RetryTrait;
 
 /**
  * Provides common methods for signing storage URLs.
@@ -33,13 +35,15 @@ class SigningHelper
 {
     use ArrayTrait;
     use JsonTrait;
+    use RetryTrait;
 
     const DEFAULT_URL_SIGNING_VERSION = 'v2';
     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';
+    const MAX_RETRIES = 5;
 
     /**
      * Create or fetch a SigningHelper instance.
@@ -169,7 +173,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 +185,10 @@ public function v2Sign(ConnectionInterface $connection, $expires, $resource, $ge
 
         $stringToSign = $this->createV2CanonicalRequest($toSign);
 
-        $signature = $credentials->signBlob($stringToSign, [
+        // Use exponential backOff
+        $signature = $this->retrySignBlob(fn () => $credentials->signBlob($stringToSign, [
             'forceOpenssl' => $options['forceOpenssl']
-        ]);
+        ]));
 
         // Start with user-provided query params and add required parameters.
         $params = $options['queryParams'];
@@ -337,9 +342,11 @@ 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(
+            fn() => $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.
@@ -882,4 +889,33 @@ private function buildQueryString(array $input)
 
         return implode('&', $q);
     }
+
+    /**
+     * Retry logic for signBlob
+     *
+     * @param callable $signBlobFn  A callable that perform the actual signBlob operation.
+     * @param string $resourceName The resource name for logging or retry strategy determination.
+     * @param array $args Arguments for the operations, include preconditions
+     * @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, string $resourceName = 'signBlob', array $args = [])
+    {
+        $attempt = 0;
+        // Generate a retry decider function using the RetryTrait logic.
+        $retryDecider = $this->getRestRetryFunction($resourceName, 'execute', $args);
+        while (true) {
+            ++$attempt;
+            try {
+                // Attempt the operation
+                return $signBlobFn();
+            } catch (\Exception $exception) {
+                if (!$retryDecider($exception, $attempt, self::MAX_RETRIES)) {
+                    // Non-retryable error
+                    throw $exception;
+                }
+            }
+        }
+    }
 }

Storage/tests/Unit/SigningHelperTest.php

@@ -19,6 +19,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;
@@ -398,7 +399,7 @@ public function testV4SignCanonicalRequestSaveAsName()
 
         $this->helper->createV4CanonicalRequest = function ($request) use ($saveAsName) {
             parse_str($request[2], $query);
-            $expectedDisposition = 'attachment; filename="' . $saveAsName .'"';
+            $expectedDisposition = 'attachment; filename="' . $saveAsName . '"';
             $this->assertEquals($expectedDisposition, $query['response-content-disposition']);
         };
 
@@ -789,6 +790,67 @@ private function mockConnection($credentials = null, $rw = null)
 
         return $conn->reveal();
     }
+
+    public function testRetrySignBlobSuccessFirstAttempt()
+    {
+        $signBlobFn = function () {
+            return 'signature';
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn
+        ]);
+
+        $this->assertEquals('signature', $res);
+    }
+
+    public function testRetrySignBlobSuccessAfterRetries()
+    {
+        $attempt = 0;
+        $signBlobFn = function () use (&$attempt) {
+            if (++$attempt < 3) {
+                throw new ServiceException('Transient error', 503);
+            }
+            return 'signature';
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn
+        ]);
+
+        $this->assertEquals('signature', $res);
+        $this->assertEquals(3, $attempt);
+    }
+
+    public function testRetrySignBlobNonRetryableError()
+    {
+        $this->expectException(ServiceException::class);
+        $this->expectExceptionMessage('Non-retryable error');
+
+        $signBlobFn = function () {
+            throw new ServiceException('Non-retryable error', 400);
+        };
+
+        $res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
+            $signBlobFn
+        ]);
+    }
+
+    public function testRetrySignBlobThrowsExceptionAfterThreeAttempts()
+    {
+        $this->expectException(ServiceException::class);
+        $this->expectExceptionMessage('Transient error (5 attempts)');
+
+        $attempt = 0;
+        $signBlobFn = function () use (&$attempt) {
+            throw new ServiceException(
+                sprintf('Transient error (%s attempts)', ++$attempt),
+                503
+            );
+        };
+
+        $this->helper->proxyPrivateMethodCall('retrySignBlob', [$signBlobFn]);
+    }
 }
 
 //@codingStandardsIgnoreStart