Correct retry waiting behaviour

See #70

Author: dru1x

src/Support/RetriesRequests.php

@@ -24,15 +24,15 @@ trait RetriesRequests
      * @param callable(Throwable, Request): (bool)|null $handleRetry
      * @throws FatalRequestException|RequestException
      */
-    protected function sendAsyncWithRetries(Request $request, ?MockClient $mockClient = null, ?callable $handleRetry = null, int $attempts = 1): PromiseInterface
+    protected function sendAsyncWithRetries(Request $request, ?MockClient $mockClient = null, ?callable $handleRetry = null, int $attempt = 1): PromiseInterface
     {
         // Allow retries by default, unless `$handleRetry` is specified
         if (is_null($handleRetry)) {
             $handleRetry = static fn(Throwable $throwable, Request $request): bool => true;
         }
 
         // Send off the request asynchronously and register a rejection handler
-        return parent::sendAsync($request, $mockClient)->otherwise(function (FatalRequestException|RequestException $exception) use ($request, $mockClient, $handleRetry, $attempts) {
+        return parent::sendAsync($request, $mockClient)->otherwise(function (FatalRequestException|RequestException $exception) use ($request, $mockClient, $handleRetry, $attempt) {
 
             $maxTries = $request->tries ?? $this->tries;
             $retryInterval = $request->retryInterval ?? $this->retryInterval;
@@ -52,7 +52,7 @@ protected function sendAsyncWithRetries(Request $request, ?MockClient $mockClien
             }
 
             // Handle max tries being exhausted
-            if ($attempts === $maxTries) {
+            if ($attempt === $maxTries) {
                 return isset($exceptionResponse) && $throwOnMaxTries === false ? $exceptionResponse : throw $exception;
             }
 
@@ -67,10 +67,10 @@ protected function sendAsyncWithRetries(Request $request, ?MockClient $mockClien
             }
 
             // Wait for the appropriate amount of time based on the retry config
-            $this->waitBeforeRetry($attempts, $retryInterval, $useExponentialBackoff);
+            $this->waitBeforeRetry($attempt, $retryInterval, $useExponentialBackoff);
 
             // Retry the request
-            return $this->sendAsyncWithRetries($request, $mockClient, $handleRetry, $attempts + 1);
+            return $this->sendAsyncWithRetries($request, $mockClient, $handleRetry, $attempt + 1);
         });
     }
 
@@ -90,16 +90,17 @@ public function handleRetry(FatalRequestException|RequestException $exception, R
 
     /**
      * Wait before the next retry, based on the retry config.
+     *
+     * @param int $attempt The index of the attempt that has just taken place
+     * @param int $interval The number of miliseconds to use as the base wait interval
+     * @param bool $useExponentialBackoff If enabled, the interval will be doubled for each subsequent attempt
      */
-    protected function waitBeforeRetry(int $attempts, int $interval, bool $useExponentialBackoff): void
+    protected function waitBeforeRetry(int $attempt, int $interval, bool $useExponentialBackoff): void
     {
-        // From the 2nd attempt onwards, wait before executing the attempt
-        if ($attempts > 0) {
-            $sleepTime = $useExponentialBackoff
-                ? $interval * (2 ** ($attempts - 2)) * 1000
-                : $interval * 1000;
+        $sleepTime = $useExponentialBackoff
+            ? $interval * (2 ** ($attempt - 1)) * 1000
+            : $interval * 1000;
 
-            usleep($sleepTime);
-        }
+        usleep($sleepTime);
     }
 }

tests/Unit/Support/RetriesRequestsTest.php

@@ -0,0 +1,56 @@
+<?php
+
+namespace Dru1x\ExpoPush\Tests\Unit\Support;
+
+use Dru1x\ExpoPush\Support\RetriesRequests;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+
+class RetriesRequestsTest extends TestCase
+{
+    #[Test]
+    public function exponential_backoff_wait_time_doubles_with_each_attempt(): void
+    {
+        $waiter   = new RetryWaiter();
+        $interval = 5;
+
+        foreach ([1, 2, 3, 4] as $attempt) {
+            $expectedMicroseconds = $interval * (2 ** ($attempt - 1)) * 1000;
+
+            $start             = hrtime(true);
+            $waiter->wait($attempt, $interval, true);
+            $elapsedMicroseconds = (hrtime(true) - $start) / 1000;
+
+            $this->assertGreaterThanOrEqual($expectedMicroseconds, $elapsedMicroseconds, "Attempt {$attempt} did not wait long enough.");
+            $this->assertLessThan($expectedMicroseconds + 50_000, $elapsedMicroseconds, "Attempt {$attempt} waited too long.");
+        }
+    }
+
+    #[Test]
+    public function wait_time_stays_constant_across_attempts_when_exponential_backoff_is_disabled(): void
+    {
+        $waiter   = new RetryWaiter();
+        $interval = 5;
+
+        $expectedMicroseconds = $interval * 1000;
+
+        foreach ([1, 2, 3] as $attempt) {
+            $start             = hrtime(true);
+            $waiter->wait($attempt, $interval, false);
+            $elapsedMicroseconds = (hrtime(true) - $start) / 1000;
+
+            $this->assertGreaterThanOrEqual($expectedMicroseconds, $elapsedMicroseconds, "Attempt {$attempt} did not wait long enough.");
+            $this->assertLessThan($expectedMicroseconds + 50_000, $elapsedMicroseconds, "Attempt {$attempt} waited too long.");
+        }
+    }
+}
+
+class RetryWaiter
+{
+    use RetriesRequests;
+
+    public function wait(int $attempt, int $interval, bool $useExponentialBackoff): void
+    {
+        $this->waitBeforeRetry($attempt, $interval, $useExponentialBackoff);
+    }
+}