fix: ensure timeout set by RetryMiddleware is int not float (#462)

googleapis · May 12, 2023 · 9d4c7fa · 9d4c7fa
1 parent 501cc47
commit 9d4c7fa
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/src/Middleware/RetryMiddleware.php b/src/Middleware/RetryMiddleware.php
@@ -128,7 +128,7 @@ private function retry(Call $call, array $options, string $status)
         }
 
         $delayMs = min($delayMs * $delayMult, $maxDelayMs);
-        $timeoutMs = min(
+        $timeoutMs = (int) min(
             $timeoutMs * $timeoutMult,
             $maxTimeoutMs,
             $deadlineMs - $this->getCurrentTimeMs()

diff --git a/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php b/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php
@@ -40,6 +40,7 @@
 use Google\Rpc\Code;
 use GuzzleHttp\Promise\Promise;
 use PHPUnit\Framework\TestCase;
+use function usleep;
 
 class RetryMiddlewareTest extends TestCase
 {
@@ -150,6 +151,38 @@ public function testRetryTimeoutExceedsRealTime()
         $middleware($call, [])->wait();
     }
 
+    public function testRetryTimeoutIsInteger()
+    {
+        $call = $this->getMockBuilder(Call::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $retrySettings = RetrySettings::constructDefault()
+            ->with([
+                'retriesEnabled' => true,
+                'retryableCodes' => [ApiStatus::CANCELLED],
+                'initialRpcTimeoutMillis' => 10000,
+                'totalTimeoutMillis' => 10000,
+            ]);
+        $callCount = 0;
+        $observedTimeouts = [];
+        $handler = function(Call $call, $options) use (&$callCount, &$observedTimeouts) {
+            $observedTimeouts[] = $options['timeoutMillis'];
+            $callCount += 1;
+            return $promise = new Promise(function () use (&$promise, $callCount) {
+                if ($callCount < 2) {
+                    throw new ApiException('Cancelled!', Code::CANCELLED, ApiStatus::CANCELLED);
+                }
+                $promise->resolve('Ok!');
+            });
+        };
+        $middleware = new RetryMiddleware($handler, $retrySettings);
+        $middleware($call, [])->wait();
+
+        $this->assertCount(2, $observedTimeouts, 'Expect 2 attempts');
+        $this->assertSame(10000, $observedTimeouts[0], 'First timeout matches config');
+        $this->assertIsInt($observedTimeouts[1], 'Second timeout is an int');
+    }
+
     public function testTimeoutMillisCallSettingsOverwrite()
     {
         $handlerCalled = false;
@@ -194,6 +227,9 @@ public function testRetryLogicalTimeout()
             $callCount += 1;
             $observedTimeouts[] = $options['timeoutMillis'];
             return $promise = new Promise(function () use (&$promise, $callCount) {
+                // each call needs to take at least 1 millisecond otherwise the rounded timeout will not decrease
+                // with each step of the test.
+                usleep(1000);
                 if ($callCount < 3) {
                     throw new ApiException('Cancelled!', Code::CANCELLED, ApiStatus::CANCELLED);
                 }