Retry the test case for maximum attempts.

Ensure proper handling of maxRetries to prevent infinite loop errors.
googleapis · Dec 11, 2024 · 9b67a34 · 9b67a34
1 parent a0fb38d
commit 9b67a34
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 5 deletions.
diff --git a/Storage/src/Connection/RetryTrait.php b/Storage/src/Connection/RetryTrait.php
@@ -135,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,
@@ -147,7 +149,9 @@ private function getRestRetryFunction($resource, $method, array $args)
                 $isOpIdempotent,
                 $preconditionNeeded,
                 $preconditionSupplied,
-                $retryStrategy
+                $retryStrategy,
+                $currentAttempt,
+                $maxRetries
             );
         };
     }
@@ -188,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;
         }

diff --git a/Storage/src/SigningHelper.php b/Storage/src/SigningHelper.php
@@ -43,6 +43,7 @@ class SigningHelper
     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.
@@ -901,14 +902,16 @@ private function buildQueryString(array $input)
      */
     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)) {
+                if (!$retryDecider($exception, $attempt, self::MAX_RETRIES)) {
                     // Non-retryable error
                     throw $exception;
                 }

diff --git a/Storage/tests/Unit/SigningHelperTest.php b/Storage/tests/Unit/SigningHelperTest.php
@@ -835,6 +835,22 @@ public function testRetrySignBlobNonRetryableError()
             $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