From 10f7229a5f55462a473cce55f1eb91432c33acf0 Mon Sep 17 00:00:00 2001
From: Anthony Bachour <anthonybachour@gmail.com>
Date: Wed, 18 Jun 2025 17:02:06 -0400
Subject: [PATCH 1/4] fix: don't throw while retrying

---
 src/ApiCall.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ApiCall.php b/src/ApiCall.php
index 523369ce..501f6f75 100644
--- a/src/ApiCall.php
+++ b/src/ApiCall.php
@@ -273,12 +273,12 @@ private function makeRequest(string $method, string $endPoint, bool $asJson, arr
                     continue;
                 }
                 $this->setNodeHealthCheck($node, false);
-                throw $this->getException($exception->getResponse()
+                $lastException = $this->getException($exception->getResponse()
                     ->getStatusCode())
                     ->setMessage($exception->getMessage());
             } catch (TypesenseClientError | HttpClientException $exception) {
                 $this->setNodeHealthCheck($node, false);
-                throw $exception;
+                $lastException = $exception;
             } catch (Exception $exception) {
                 $this->setNodeHealthCheck($node, false);
                 $lastException = $exception;

From 591455cb3c79e444ad4baa76bad3408a89b4e136 Mon Sep 17 00:00:00 2001
From: Fanis Tharropoulos <ftharropoulos@gmail.com>
Date: Tue, 24 Jun 2025 10:45:16 +0300
Subject: [PATCH 2/4] fix(api): throw on first case of 400s

- remove duplicate `$statusCode` variable assignment in `ApiCall.php`
- clean up redundant conditional check for status code 408
- simplify exception handling flow for better code readability
---
 src/ApiCall.php | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/ApiCall.php b/src/ApiCall.php
index 501f6f75..b2d73e98 100644
--- a/src/ApiCall.php
+++ b/src/ApiCall.php
@@ -266,15 +266,20 @@ private function makeRequest(string $method, string $endPoint, bool $asJson, arr
                     ->getContents(), true, 512, JSON_THROW_ON_ERROR) : $response->getBody()
                     ->getContents();
             } catch (HttpException $exception) {
-                if (
-                    $exception->getResponse()
-                    ->getStatusCode() === 408
-                ) {
+                $statusCode = $exception->getResponse()->getStatusCode();
+                
+                if ($statusCode === 408) {
                     continue;
                 }
+                
+                if (400 <= $statusCode && $statusCode < 500) {
+                    $this->setNodeHealthCheck($node, false);
+                    throw $this->getException($statusCode)
+                        ->setMessage($exception->getMessage());
+                }
+                
                 $this->setNodeHealthCheck($node, false);
-                $lastException = $this->getException($exception->getResponse()
-                    ->getStatusCode())
+                $lastException = $this->getException($statusCode)
                     ->setMessage($exception->getMessage());
             } catch (TypesenseClientError | HttpClientException $exception) {
                 $this->setNodeHealthCheck($node, false);

From 8efbdd6fa46522348ca9c66478a364782b0c2e44 Mon Sep 17 00:00:00 2001
From: Fanis Tharropoulos <ftharropoulos@gmail.com>
Date: Tue, 24 Jun 2025 10:46:30 +0300
Subject: [PATCH 3/4] test(api): add retry mechanism tests for `ApiCall` class

- Add `ApiCallRetryTest.php` with tests for exception handling scenarios
- Test retry logic for server errors, client errors, and timeout responses
- Verify 4xx errors are not retried while 5xx errors trigger retries
- Test node health check updates after failed requests
- Ensure 408 timeout errors are skipped and retrying continues
---
 tests/Feature/ApiCallRetryTest.php | 409 +++++++++++++++++++++++++++++
 1 file changed, 409 insertions(+)
 create mode 100644 tests/Feature/ApiCallRetryTest.php

diff --git a/tests/Feature/ApiCallRetryTest.php b/tests/Feature/ApiCallRetryTest.php
new file mode 100644
index 00000000..bc62e63b
--- /dev/null
+++ b/tests/Feature/ApiCallRetryTest.php
@@ -0,0 +1,409 @@
+<?php
+
+namespace Feature;
+
+use Tests\TestCase;
+use Typesense\ApiCall;
+use Typesense\Lib\Configuration;
+use Typesense\Exceptions\ServerError;
+use Typesense\Exceptions\RequestMalformed;
+use Http\Client\Exception\HttpException;
+use Http\Client\Exception\TransferException;
+use Psr\Http\Client\ClientInterface;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\StreamInterface;
+use Psr\Http\Message\RequestInterface;
+
+class ApiCallRetryTest extends TestCase
+{
+    public function testRetriesOnHttpExceptionWithNon408Status(): void
+    {
+        $callCount = 0;
+        $expectedCalls = 3;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount, $expectedCalls) {
+                $callCount++;
+                
+                if ($callCount < $expectedCalls) {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(500);
+                    throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
+                } else {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(200);
+                    $stream = $this->createMock(StreamInterface::class);
+                    $stream->method('getContents')->willReturn('{"success": true}');
+                    $response->method('getBody')->willReturn($stream);
+                    return $response;
+                }
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node3', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $result = $apiCall->get('/test', []);
+        
+        $this->assertEquals(['success' => true], $result);
+        $this->assertEquals($expectedCalls, $callCount);
+    }
+
+    public function testRetriesExhaustedThrowsLastException(): void
+    {
+        $callCount = 0;
+        $expectedCalls = 3;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(500);
+                throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $this->expectException(ServerError::class);
+        $this->expectExceptionMessage('Server error');
+        
+        $apiCall->get('/test', []);
+        
+        $this->assertEquals($expectedCalls, $callCount);
+    }
+
+    public function testRetriesOnTypesenseClientError(): void
+    {
+        $callCount = 0;
+        $expectedCalls = 3;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount, $expectedCalls) {
+                $callCount++;
+                
+                if ($callCount < $expectedCalls) {
+                    throw new RequestMalformed('Bad request');
+                } else {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(200);
+                    $stream = $this->createMock(StreamInterface::class);
+                    $stream->method('getContents')->willReturn('{"success": true}');
+                    $response->method('getBody')->willReturn($stream);
+                    return $response;
+                }
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node3', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $result = $apiCall->get('/test', []);
+        
+        $this->assertEquals(['success' => true], $result);
+        $this->assertEquals($expectedCalls, $callCount);
+    }
+
+    public function testRetriesOnHttpClientException(): void
+    {
+        $callCount = 0;
+        $expectedCalls = 3;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount, $expectedCalls) {
+                $callCount++;
+                
+                if ($callCount < $expectedCalls) {
+                    throw new TransferException('Connection error');
+                } else {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(200);
+                    $stream = $this->createMock(StreamInterface::class);
+                    $stream->method('getContents')->willReturn('{"success": true}');
+                    $response->method('getBody')->willReturn($stream);
+                    return $response;
+                }
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node3', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $result = $apiCall->get('/test', []);
+        
+        $this->assertEquals(['success' => true], $result);
+        $this->assertEquals($expectedCalls, $callCount);
+    }
+
+    public function testSkips408TimeoutErrorsAndContinuesRetrying(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                if ($callCount === 1) {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(408);
+                    throw new HttpException('Request timeout', $this->createMock(RequestInterface::class), $response);
+                } elseif ($callCount === 2) {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(500);
+                    throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
+                } else {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(200);
+                    $stream = $this->createMock(StreamInterface::class);
+                    $stream->method('getContents')->willReturn('{"success": true}');
+                    $response->method('getBody')->willReturn($stream);
+                    return $response;
+                }
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node3', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $result = $apiCall->get('/test', []);
+        
+        $this->assertEquals(['success' => true], $result);
+        $this->assertEquals(3, $callCount);
+    }
+
+    public function testNodeHealthCheckAfterExceptions(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(500);
+                throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $node1 = $config->getNodes()[0];
+        $node2 = $config->getNodes()[1];
+        
+        $this->assertTrue($node1->isHealthy());
+        $this->assertTrue($node2->isHealthy());
+        
+        try {
+            $apiCall->get('/test', []);
+        } catch (ServerError $e) {
+            $this->assertFalse($node1->isHealthy());
+            $this->assertFalse($node2->isHealthy());
+        }
+        
+        $this->assertEquals(3, $callCount);
+    }
+
+    public function test400ErrorsAreNotRetried(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(400);
+                throw new HttpException('Bad Request', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $this->expectException(\Typesense\Exceptions\RequestMalformed::class);
+        $this->expectExceptionMessage('Bad Request');
+        
+        $apiCall->get('/test', []);
+        
+        $this->assertEquals(1, $callCount);
+    }
+
+    public function test401ErrorsAreNotRetried(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(401);
+                throw new HttpException('Unauthorized', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $this->expectException(\Typesense\Exceptions\RequestUnauthorized::class);
+        $this->expectExceptionMessage('Unauthorized');
+        
+        $apiCall->get('/test', []);
+        
+        $this->assertEquals(1, $callCount);
+    }
+
+    public function test404ErrorsAreNotRetried(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(404);
+                throw new HttpException('Not Found', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $this->expectException(\Typesense\Exceptions\ObjectNotFound::class);
+        $this->expectExceptionMessage('Not Found');
+        
+        $apiCall->get('/test', []);
+        
+        $this->assertEquals(1, $callCount);
+    }
+
+    public function test408ErrorsAreSkippedAndRetryingContinues(): void
+    {
+        $callCount = 0;
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                if ($callCount === 1) {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(408);
+                    throw new HttpException('Request timeout', $this->createMock(RequestInterface::class), $response);
+                } else {
+                    $response = $this->createMock(ResponseInterface::class);
+                    $response->method('getStatusCode')->willReturn(200);
+                    $stream = $this->createMock(StreamInterface::class);
+                    $stream->method('getContents')->willReturn('{"success": true}');
+                    $response->method('getBody')->willReturn($stream);
+                    return $response;
+                }
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http'],
+                ['host' => 'node2', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $result = $apiCall->get('/test', []);
+        
+        $this->assertEquals(['success' => true], $result);
+        $this->assertEquals(2, $callCount);
+    }
+} 
\ No newline at end of file

From 33d30dce2b4fa1c2c7eecbc0bc7cbf3763c5cd4b Mon Sep 17 00:00:00 2001
From: Fanis Tharropoulos <ftharropoulos@gmail.com>
Date: Wed, 16 Jul 2025 16:52:16 +0300
Subject: [PATCH 4/4] fix: prevent sleep on final retry attempt in api call

- refactor exception handling to separate typesense client errors from network errors
- fix sleep logic to only execute between retry attempts, not after final attempt
- add test to verify no sleep occurs on final retry attempt with timing validation
---
 src/ApiCall.php                    | 15 ++++++-
 tests/Feature/ApiCallRetryTest.php | 64 ++++++++++++++++++++++++++++--
 2 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/src/ApiCall.php b/src/ApiCall.php
index b2d73e98..a0c44459 100644
--- a/src/ApiCall.php
+++ b/src/ApiCall.php
@@ -268,27 +268,38 @@ private function makeRequest(string $method, string $endPoint, bool $asJson, arr
             } catch (HttpException $exception) {
                 $statusCode = $exception->getResponse()->getStatusCode();
                 
+                // Skip 408 timeouts and continue to next iteration
                 if ($statusCode === 408) {
                     continue;
                 }
                 
+                // For 4xx errors, don't retry - throw immediately
                 if (400 <= $statusCode && $statusCode < 500) {
                     $this->setNodeHealthCheck($node, false);
                     throw $this->getException($statusCode)
                         ->setMessage($exception->getMessage());
                 }
                 
+                // For 5xx errors, set exception and continue to retry logic
                 $this->setNodeHealthCheck($node, false);
                 $lastException = $this->getException($statusCode)
                     ->setMessage($exception->getMessage());
-            } catch (TypesenseClientError | HttpClientException $exception) {
+            } catch (HttpClientException $exception) {
+                // For network errors, set exception and continue to retry logic
                 $this->setNodeHealthCheck($node, false);
                 $lastException = $exception;
             } catch (Exception $exception) {
+                if ($exception instanceof TypesenseClientError) {
+                    throw $exception;
+                }
+                
                 $this->setNodeHealthCheck($node, false);
                 $lastException = $exception;
-                sleep($this->config->getRetryIntervalSeconds());
             }
+            
+            if ($numRetries < $this->config->getNumRetries() + 1) {
+                usleep((int) ($this->config->getRetryIntervalSeconds() * 10**6));
+            } 
         }
 
         if ($lastException) {
diff --git a/tests/Feature/ApiCallRetryTest.php b/tests/Feature/ApiCallRetryTest.php
index bc62e63b..8d553831 100644
--- a/tests/Feature/ApiCallRetryTest.php
+++ b/tests/Feature/ApiCallRetryTest.php
@@ -97,7 +97,7 @@ public function testRetriesExhaustedThrowsLastException(): void
     public function testRetriesOnTypesenseClientError(): void
     {
         $callCount = 0;
-        $expectedCalls = 3;
+        $expectedCalls = 1;
         
         $httpClient = $this->createMock(ClientInterface::class);
         $httpClient->method('sendRequest')
@@ -129,9 +129,11 @@ public function testRetriesOnTypesenseClientError(): void
 
         $apiCall = new ApiCall($config);
         
-        $result = $apiCall->get('/test', []);
+        try {
+            $apiCall->get('/test', []);
+        } catch (ServerError $e) {
+        }
         
-        $this->assertEquals(['success' => true], $result);
         $this->assertEquals($expectedCalls, $callCount);
     }
 
@@ -406,4 +408,60 @@ public function test408ErrorsAreSkippedAndRetryingContinues(): void
         $this->assertEquals(['success' => true], $result);
         $this->assertEquals(2, $callCount);
     }
+
+    public function testDoesNotSleepOnFinalRetryAttempt(): void
+    {
+        $callCount = 0;
+        $retryIntervalSeconds = 0.1; 
+        
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount) {
+                $callCount++;
+                
+                $response = $this->createMock(ResponseInterface::class);
+                $response->method('getStatusCode')->willReturn(500);
+                throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'num_retries' => 2, 
+            'retry_interval_seconds' => $retryIntervalSeconds,
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+        
+        $startTime = microtime(true);
+        
+        try {
+            $apiCall->get('/test', []);
+        } catch (ServerError $e) {
+        }
+        
+        $endTime = microtime(true);
+        $actualDuration = $endTime - $startTime;
+        
+        //  2 sleep intervals (between 1st->2nd and 2nd->3rd attempts)
+        //  no sleep after the final (3rd) attempt
+        $expectedDuration = $retryIntervalSeconds * 2; 
+        
+        $tolerance = 0.05;
+        
+        $this->assertEquals(3, $callCount, 'Should make exactly 3 attempts');
+        $this->assertLessThan(
+            $expectedDuration + $tolerance, 
+            $actualDuration,
+            "Execution took too long ({$actualDuration}s), suggesting sleep was called on final attempt"
+        );
+        $this->assertGreaterThan(
+            $expectedDuration - $tolerance, 
+            $actualDuration,
+            "Execution was too fast ({$actualDuration}s), suggesting sleep intervals were skipped"
+        );
+    }
 } 
\ No newline at end of file