Skip to content

Commit 33d30dc

Browse files
committed
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
1 parent 8efbdd6 commit 33d30dc

File tree

2 files changed

+74
-5
lines changed

2 files changed

+74
-5
lines changed

src/ApiCall.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,27 +268,38 @@ private function makeRequest(string $method, string $endPoint, bool $asJson, arr
268268
} catch (HttpException $exception) {
269269
$statusCode = $exception->getResponse()->getStatusCode();
270270

271+
// Skip 408 timeouts and continue to next iteration
271272
if ($statusCode === 408) {
272273
continue;
273274
}
274275

276+
// For 4xx errors, don't retry - throw immediately
275277
if (400 <= $statusCode && $statusCode < 500) {
276278
$this->setNodeHealthCheck($node, false);
277279
throw $this->getException($statusCode)
278280
->setMessage($exception->getMessage());
279281
}
280282

283+
// For 5xx errors, set exception and continue to retry logic
281284
$this->setNodeHealthCheck($node, false);
282285
$lastException = $this->getException($statusCode)
283286
->setMessage($exception->getMessage());
284-
} catch (TypesenseClientError | HttpClientException $exception) {
287+
} catch (HttpClientException $exception) {
288+
// For network errors, set exception and continue to retry logic
285289
$this->setNodeHealthCheck($node, false);
286290
$lastException = $exception;
287291
} catch (Exception $exception) {
292+
if ($exception instanceof TypesenseClientError) {
293+
throw $exception;
294+
}
295+
288296
$this->setNodeHealthCheck($node, false);
289297
$lastException = $exception;
290-
sleep($this->config->getRetryIntervalSeconds());
291298
}
299+
300+
if ($numRetries < $this->config->getNumRetries() + 1) {
301+
usleep((int) ($this->config->getRetryIntervalSeconds() * 10**6));
302+
}
292303
}
293304

294305
if ($lastException) {

tests/Feature/ApiCallRetryTest.php

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public function testRetriesExhaustedThrowsLastException(): void
9797
public function testRetriesOnTypesenseClientError(): void
9898
{
9999
$callCount = 0;
100-
$expectedCalls = 3;
100+
$expectedCalls = 1;
101101

102102
$httpClient = $this->createMock(ClientInterface::class);
103103
$httpClient->method('sendRequest')
@@ -129,9 +129,11 @@ public function testRetriesOnTypesenseClientError(): void
129129

130130
$apiCall = new ApiCall($config);
131131

132-
$result = $apiCall->get('/test', []);
132+
try {
133+
$apiCall->get('/test', []);
134+
} catch (ServerError $e) {
135+
}
133136

134-
$this->assertEquals(['success' => true], $result);
135137
$this->assertEquals($expectedCalls, $callCount);
136138
}
137139

@@ -406,4 +408,60 @@ public function test408ErrorsAreSkippedAndRetryingContinues(): void
406408
$this->assertEquals(['success' => true], $result);
407409
$this->assertEquals(2, $callCount);
408410
}
411+
412+
public function testDoesNotSleepOnFinalRetryAttempt(): void
413+
{
414+
$callCount = 0;
415+
$retryIntervalSeconds = 0.1;
416+
417+
$httpClient = $this->createMock(ClientInterface::class);
418+
$httpClient->method('sendRequest')
419+
->willReturnCallback(function() use (&$callCount) {
420+
$callCount++;
421+
422+
$response = $this->createMock(ResponseInterface::class);
423+
$response->method('getStatusCode')->willReturn(500);
424+
throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
425+
});
426+
427+
$config = new Configuration([
428+
'api_key' => 'test-key',
429+
'nodes' => [
430+
['host' => 'node1', 'port' => 8108, 'protocol' => 'http']
431+
],
432+
'num_retries' => 2,
433+
'retry_interval_seconds' => $retryIntervalSeconds,
434+
'client' => $httpClient
435+
]);
436+
437+
$apiCall = new ApiCall($config);
438+
439+
$startTime = microtime(true);
440+
441+
try {
442+
$apiCall->get('/test', []);
443+
} catch (ServerError $e) {
444+
}
445+
446+
$endTime = microtime(true);
447+
$actualDuration = $endTime - $startTime;
448+
449+
// 2 sleep intervals (between 1st->2nd and 2nd->3rd attempts)
450+
// no sleep after the final (3rd) attempt
451+
$expectedDuration = $retryIntervalSeconds * 2;
452+
453+
$tolerance = 0.05;
454+
455+
$this->assertEquals(3, $callCount, 'Should make exactly 3 attempts');
456+
$this->assertLessThan(
457+
$expectedDuration + $tolerance,
458+
$actualDuration,
459+
"Execution took too long ({$actualDuration}s), suggesting sleep was called on final attempt"
460+
);
461+
$this->assertGreaterThan(
462+
$expectedDuration - $tolerance,
463+
$actualDuration,
464+
"Execution was too fast ({$actualDuration}s), suggesting sleep intervals were skipped"
465+
);
466+
}
409467
}

0 commit comments

Comments
 (0)