From e773da5d175739954b688d14ee0e3caba01dcc20 Mon Sep 17 00:00:00 2001 From: muhammed Date: Sun, 18 Oct 2026 15:23:34 +0300 Subject: [PATCH] fix: cap Retry-After delays at maximum backoff duration --- .changeset/tough-pianos-walk.md | 5 +++++ lib/HttpClient.php | 4 ++-- test/HttpClientTest.php | 38 +++++++++++++++++++++++++++++++++ test/fixtures/usleep_mock.php | 15 +++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 .changeset/tough-pianos-walk.md create mode 100644 test/fixtures/usleep_mock.php diff --git a/.changeset/tough-pianos-walk.md b/.changeset/tough-pianos-walk.md new file mode 100644 index 00000000..309a55bc --- /dev/null +++ b/.changeset/tough-pianos-walk.md @@ -0,0 +1,5 @@ +--- +"posthog-php": patch +--- + +Cap Retry-After delays at maximum_backoff_duration to bound synchronous retry waits. diff --git a/lib/HttpClient.php b/lib/HttpClient.php index 45583f35..e6ea243b 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -162,10 +162,10 @@ public function sendRequest(string $path, ?string $payload, array $extraHeaders if ($shouldRetry === false) { break; } elseif ($this->isRetryableStatus($responseCode)) { - // Retry transient failures. Honor Retry-After when provided, otherwise use + // Retry transient failures. Honor Retry-After up to the maximum backoff, otherwise use // exponential backoff starting at 100ms. $retryAfterMs = $this->retryAfterMilliseconds($responseHeaders); - usleep(($retryAfterMs ?? $backoff) * 1000); + usleep(min($retryAfterMs ?? $backoff, $this->maximumBackoffDurationMs) * 1000); $backoff *= 2; } else { // Do not retry non-transient responses (e.g. 400, 401, 403, 413, diff --git a/test/HttpClientTest.php b/test/HttpClientTest.php index 43c37b18..36b58a5d 100644 --- a/test/HttpClientTest.php +++ b/test/HttpClientTest.php @@ -6,6 +6,8 @@ use PHPUnit\Framework\TestCase; use PostHog\HttpClient; +require_once __DIR__ . '/fixtures/usleep_mock.php'; + class RetryAfterHttpClient extends HttpClient { /** @param array $headers */ @@ -119,6 +121,42 @@ public function testHttpResponsesAndRetryPolicy(int $status, bool $retry, int $a } } + public static function retryDelayCases(): array + { + return [ + 'large rate-limit delay capped' => [429, ['Retry-After' => '60'], [2500000, 2500000]], + 'large service-unavailable delay capped' => [503, ['Retry-After' => '60'], [2500000, 2500000]], + 'small delay honored' => [429, ['Retry-After' => '1'], [1000000, 1000000]], + 'date delay capped' => [503, ['Retry-After' => 'Thu, 01 Jan 2099 00:00:00 GMT'], [2500000, 2500000]], + 'no header uses exponential backoff' => [503, [], [100000, 200000]], + ]; + } + + #[DataProvider('retryDelayCases')] + public function testRetryDelay(int $status, array $headers, array $expectedDelays): void + { + global $httpClientSleepDurations; + $httpClientSleepDurations = []; + $server = new LocalHttpServer([ + ['status' => $status, 'headers' => $headers], + ['status' => $status, 'headers' => $headers], + ['status' => 200, 'body' => 'delivered'], + ]); + + try { + $client = new HttpClient($server->address(), useSsl: false, maximumBackoffDuration: 2500); + $response = $client->sendRequest('/batch/', '{"batch":[]}'); + + $this->assertSame($expectedDelays, $httpClientSleepDurations); + $this->assertSame(200, $response->getResponseCode()); + $this->assertSame('delivered', $response->getResponse()); + $this->assertCount(3, $server->requests()); + } finally { + $httpClientSleepDurations = null; + $server->stop(); + } + } + public function testConditionalGetPreservesEtagAndSeparatesResponseHeaders(): void { $server = new LocalHttpServer([ diff --git a/test/fixtures/usleep_mock.php b/test/fixtures/usleep_mock.php new file mode 100644 index 00000000..39099a70 --- /dev/null +++ b/test/fixtures/usleep_mock.php @@ -0,0 +1,15 @@ +