Skip to content

Commit 69b19dc

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 69b19dc

File tree

2 files changed

+69
-2
lines changed

2 files changed

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

0 commit comments

Comments
 (0)