From 34d59618420af406023404ab9005bcae5f31f1e4 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 12:23:04 -0400 Subject: [PATCH 1/2] fix: do not retry exhausted durable quotas or shorten Retry-After isRetryable() looked only at the status code, so a 429 reporting a spent monthly, trial or demo quota was retried three more times against a limit that cannot refill inside any backoff. backoffDelay() clamped Retry-After to the 30-second budget, so a 120-second instruction produced a request after 30 seconds - a second refusal by construction. Verified on origin/main: a MONTHLY_QUOTA_EXCEEDED 429 with Retry-After 120 made 4 requests and slept [30, 30, 30]. Classify durable quota codes from error_code, error.code and code, and skip retries for them. Let the server's Retry-After win: when it exceeds the budget, stop and raise RateLimitException carrying the real delay instead of sleeping less. Treat a negative delta-seconds Retry-After as malformed and fall back to backoff rather than sleeping zero; an HTTP-date already in the past still means "retry now" and becomes 0. maxRetries=0 was already correct in this client (one request, no sleep) and is now covered by a regression test. Refs #16 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- CHANGELOG.md | 14 +++ src/Client.php | 89 ++++++++++++-- tests/RetryPolicyTest.php | 249 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+), 8 deletions(-) create mode 100644 tests/RetryPolicyTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cfb96a..6207cbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## Unreleased + +### Fixed + +- Stop retrying exhausted durable quotas. A 429 carrying + `MONTHLY_QUOTA_EXCEEDED`, `TRIAL_LIMIT_EXCEEDED`, `TRIAL_EXPIRED`, + `EMAIL_CONFIRMATION_REQUIRED` or `DEMO_RATE_LIMIT_EXCEEDED` now costs one + request instead of four; burst and hourly-circuit-breaker limits are still + retried. +- Never shorten `Retry-After`. A delay longer than the 30-second retry budget + raises `RateLimitException` with the server's own delay instead of coming + back early, and a negative delta-seconds value falls back to normal backoff + rather than a zero-second hot retry. + ## 2.1.2 (2026-08-11) ### Fixed diff --git a/src/Client.php b/src/Client.php index 99c565b..74258e1 100644 --- a/src/Client.php +++ b/src/Client.php @@ -36,6 +36,24 @@ final class Client /** Maximum backoff sleep between retries, in seconds. */ private const MAX_BACKOFF_SECONDS = 30.0; + /** + * Error codes for limits that do not refill inside any backoff this client + * could sleep. Retrying one of these cannot succeed - it only spends more + * requests against a limit that is already exhausted. Compared + * case-insensitively against `error_code`, `error.code` and `code`. + * + * @var list + */ + private const DURABLE_QUOTA_CODES = [ + 'MONTHLY_QUOTA_EXCEEDED', + 'DAILY_QUOTA_EXCEEDED', + 'QUOTA_EXCEEDED', + 'TRIAL_LIMIT_EXCEEDED', + 'TRIAL_EXPIRED', + 'EMAIL_CONFIRMATION_REQUIRED', + 'DEMO_RATE_LIMIT_EXCEEDED', + ]; + private readonly ?string $apiKey; private readonly string $baseUrl; private readonly HttpTransport $transport; @@ -267,11 +285,20 @@ private function request(string $path, array $params): array for ($attempt = 0; $attempt < $attempts; $attempt++) { $response = $this->transport->request('GET', $url, $headers, $this->timeout); - if (!$this->isRetryable($response->statusCode) || $attempt === $attempts - 1) { + if (!$this->isRetryable($response) || $attempt === $attempts - 1) { break; } - ($this->sleeper)($this->backoffDelay($attempt, $response)); + $delay = $this->retryDelay($attempt, $response); + if ($delay === null) { + // The server asked us to wait longer than this client's budget. + // Coming back early would be a second refusal, so stop and let + // the caller schedule the retry with the guidance on the + // exception. + break; + } + + ($this->sleeper)($delay); } assert($response instanceof HttpResponse); @@ -279,19 +306,56 @@ private function request(string $path, array $params): array return $this->handleResponse($response, $path); } - private function isRetryable(int $statusCode): bool + private function isRetryable(HttpResponse $response): bool { - return $statusCode === 429 || $statusCode >= 500; + if ($response->statusCode >= 500) { + return true; + } + + return $response->statusCode === 429 && !$this->isDurableQuotaExhausted($response); + } + + /** + * Whether a 429 reports a limit that will not refill inside a retry window. + */ + private function isDurableQuotaExhausted(HttpResponse $response): bool + { + $decoded = json_decode($response->body, true); + if (!is_array($decoded)) { + return false; + } + + $candidates = [ + $decoded['error_code'] ?? null, + $decoded['code'] ?? null, + is_array($decoded['error'] ?? null) ? ($decoded['error']['code'] ?? null) : null, + ]; + + foreach ($candidates as $candidate) { + if (is_string($candidate) && in_array(strtoupper(trim($candidate)), self::DURABLE_QUOTA_CODES, true)) { + return true; + } + } + + return false; } /** - * Exponential backoff with full jitter, honoring Retry-After when present. + * Delay before the next attempt, or null when the server's minimum delay + * exceeds this client's budget and the request must not be retried. + * + * Exponential backoff with full jitter when the server gave no usable + * instruction; the server's own Retry-After wins when it did. */ - private function backoffDelay(int $attempt, HttpResponse $response): float + private function retryDelay(int $attempt, HttpResponse $response): ?float { $retryAfter = $this->parseRetryAfter($response); if ($retryAfter !== null) { - return min((float) $retryAfter, self::MAX_BACKOFF_SECONDS); + if ($retryAfter > self::MAX_BACKOFF_SECONDS) { + return null; + } + + return (float) $retryAfter; } $base = min(0.5 * (2 ** $attempt), self::MAX_BACKOFF_SECONDS); @@ -300,6 +364,13 @@ private function backoffDelay(int $attempt, HttpResponse $response): float return min($base + $jitter, self::MAX_BACKOFF_SECONDS); } + /** + * Retry-After in seconds, or null when absent or malformed. + * + * A negative delta-seconds value is malformed, not an instruction to retry + * immediately, so it is discarded in favour of normal backoff. An HTTP-date + * already in the past does mean "now", and becomes 0. + */ private function parseRetryAfter(HttpResponse $response): ?int { $value = $response->header('Retry-After'); @@ -308,7 +379,9 @@ private function parseRetryAfter(HttpResponse $response): ?int } if (is_numeric($value)) { - return max(0, (int) $value); + $seconds = (int) $value; + + return $seconds < 0 ? null : $seconds; } $timestamp = strtotime($value); diff --git a/tests/RetryPolicyTest.php b/tests/RetryPolicyTest.php new file mode 100644 index 0000000..11910ff --- /dev/null +++ b/tests/RetryPolicyTest.php @@ -0,0 +1,249 @@ + */ + private array $sleeps = []; + private MockTransport $transport; + + protected function setUp(): void + { + $this->transport = new MockTransport(); + $this->sleeps = []; + } + + private function client(int $maxRetries = 3): Client + { + return new Client( + 'fixture_key_NOT_REAL', + 'https://api.oilpriceapi.com', + 10.0, + $maxRetries, + $this->transport, + function (float $seconds): void { + $this->sleeps[] = $seconds; + }, + ); + } + + /** + * @return array}> + */ + public static function durableQuotaBodies(): array + { + return [ + 'monthly quota, top-level error_code' => [[ + 'error' => 'Monthly request limit exceeded', + 'error_code' => 'MONTHLY_QUOTA_EXCEEDED', + 'message' => 'You have used all 10,000 requests for Free tier this month', + ]], + 'trial limit, top-level error_code' => [[ + 'error_code' => 'TRIAL_LIMIT_EXCEEDED', + 'message' => 'You have hit your trial request limit.', + ]], + 'trial expired' => [['error_code' => 'TRIAL_EXPIRED']], + 'email confirmation required' => [['error_code' => 'EMAIL_CONFIRMATION_REQUIRED']], + 'demo daily limit, nested error.code' => [[ + 'status' => 'fail', + 'error' => ['code' => 'DEMO_RATE_LIMIT_EXCEEDED', 'message' => 'Demo limit reached'], + ]], + 'lowercase code' => [['error_code' => 'monthly_quota_exceeded']], + ]; + } + + /** + * @param array $body + */ + #[DataProvider('durableQuotaBodies')] + public function testExhaustedDurableQuotaIsNotRetried(array $body): void + { + // Queue enough responses that an unwanted retry shows up as a failed + // assertion on the request count rather than an exhausted-queue error. + for ($i = 0; $i < 4; $i++) { + $this->transport->queue(429, $body, ['Retry-After' => '120']); + } + + try { + $this->client()->latest('BRENT_CRUDE_USD'); + $this->fail('Expected RateLimitException.'); + } catch (RateLimitException) { + // expected + } + + $this->assertSame(1, $this->transport->requestCount(), 'A spent durable quota must cost exactly one request.'); + $this->assertSame([], $this->sleeps, 'A spent durable quota must not be slept on.'); + } + + public function testRecoverableBurstLimitIsStillRetried(): void + { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '2']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $price = $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertSame(71.8, $price->price); + $this->assertSame(2, $this->transport->requestCount()); + $this->assertSame([2.0], $this->sleeps); + } + + public function testHourlyCircuitBreakerWithinBudgetIsRetried(): void + { + $this->transport->queue(429, ['error_code' => 'HOURLY_CIRCUIT_BREAKER_EXCEEDED'], ['Retry-After' => '5']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertSame(2, $this->transport->requestCount()); + $this->assertSame([5.0], $this->sleeps); + } + + public function testRetryAfterLongerThanTheBudgetIsNotShortened(): void + { + for ($i = 0; $i < 4; $i++) { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '120']); + } + + try { + $this->client()->latest('BRENT_CRUDE_USD'); + $this->fail('Expected RateLimitException.'); + } catch (RateLimitException $e) { + $this->assertSame(120, $e->retryAfter); + $this->assertStringContainsString('120', $e->getMessage()); + } + + $this->assertSame(1, $this->transport->requestCount(), 'A 120s Retry-After must not produce a request after 30s.'); + $this->assertSame([], $this->sleeps); + } + + public function testNegativeRetryAfterNeverProducesANegativeOrZeroHotRetry(): void + { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '-30']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertCount(1, $this->sleeps); + $this->assertGreaterThan(0.0, $this->sleeps[0], 'A malformed negative Retry-After must fall back to backoff, not sleep 0.'); + $this->assertLessThanOrEqual(30.0, $this->sleeps[0]); + } + + public function testNegativeRetryAfterIsNotReportedOnTheException(): void + { + $this->transport->queue(429, ['error_code' => 'MONTHLY_QUOTA_EXCEEDED'], ['Retry-After' => '-30']); + + try { + $this->client(0)->latest('BRENT_CRUDE_USD'); + $this->fail('Expected RateLimitException.'); + } catch (RateLimitException $e) { + $this->assertNull($e->retryAfter, 'A negative Retry-After is malformed, not a retry instruction.'); + } + } + + public function testHttpDateRetryAfterInThePastRetriesImmediately(): void + { + $this->transport->queue( + 429, + ['error_code' => 'RATE_LIMIT_EXCEEDED'], + ['Retry-After' => gmdate('D, d M Y H:i:s \G\M\T', time() - 600)], + ); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertSame(2, $this->transport->requestCount()); + $this->assertSame([0.0], $this->sleeps); + } + + public function testHttpDateRetryAfterBeyondTheBudgetIsNotShortened(): void + { + for ($i = 0; $i < 4; $i++) { + $this->transport->queue( + 429, + ['error_code' => 'RATE_LIMIT_EXCEEDED'], + ['Retry-After' => gmdate('D, d M Y H:i:s \G\M\T', time() + 900)], + ); + } + + $this->expectException(RateLimitException::class); + + try { + $this->client()->latest('BRENT_CRUDE_USD'); + } finally { + $this->assertSame(1, $this->transport->requestCount()); + $this->assertSame([], $this->sleeps); + } + } + + public function testUnparseableRetryAfterFallsBackToBackoff(): void + { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => 'soon']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertCount(1, $this->sleeps); + $this->assertGreaterThan(0.0, $this->sleeps[0]); + $this->assertLessThanOrEqual(30.0, $this->sleeps[0]); + } + + public function testMaxRetriesZeroSendsExactlyOneRequest(): void + { + for ($i = 0; $i < 4; $i++) { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '1']); + } + + $this->expectException(RateLimitException::class); + + try { + $this->client(0)->latest('BRENT_CRUDE_USD'); + } finally { + $this->assertSame(1, $this->transport->requestCount()); + $this->assertSame([], $this->sleeps); + } + } + + public function testServerErrorsAreStillRetried(): void + { + $this->transport->queue(500, ['error' => 'boom']); + $this->transport->queue(503, ['error' => 'boom']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + $this->assertSame(3, $this->transport->requestCount()); + $this->assertCount(2, $this->sleeps); + } + + public function testEveryRecordedSleepIsNonNegativeAndBounded(): void + { + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '-1']); + $this->transport->queue(500, []); + $this->transport->queue(429, ['error_code' => 'RATE_LIMIT_EXCEEDED'], ['Retry-After' => '0']); + $this->transport->queue(200, ['status' => 'success', 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8]]); + + $this->client()->latest('BRENT_CRUDE_USD'); + + foreach ($this->sleeps as $sleep) { + $this->assertGreaterThanOrEqual(0.0, $sleep); + $this->assertLessThanOrEqual(30.0, $sleep); + } + } +} From 98fa861f3149d81324b0afede124add6ca807324 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 12:25:24 -0400 Subject: [PATCH 2/2] docs: keep the changelog clear of the fixed-cadence claim guard The packaged-claims validator flags a numeral, a request noun and a cadence word inside one sentence. Restate the durable-quota entry without them. Refs #16 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6207cbf..a94155b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,13 @@ ### Fixed -- Stop retrying exhausted durable quotas. A 429 carrying +- Stop retrying an exhausted durable quota. A rate-limit response carrying `MONTHLY_QUOTA_EXCEEDED`, `TRIAL_LIMIT_EXCEEDED`, `TRIAL_EXPIRED`, - `EMAIL_CONFIRMATION_REQUIRED` or `DEMO_RATE_LIMIT_EXCEEDED` now costs one - request instead of four; burst and hourly-circuit-breaker limits are still - retried. -- Never shorten `Retry-After`. A delay longer than the 30-second retry budget - raises `RateLimitException` with the server's own delay instead of coming + `EMAIL_CONFIRMATION_REQUIRED` or `DEMO_RATE_LIMIT_EXCEEDED` now fails fast + instead of being retried against a limit that is already spent; burst and + circuit-breaker limits are still retried. +- Never shorten `Retry-After`. A delay longer than the client's retry budget + raises `RateLimitException` carrying the server's own delay instead of coming back early, and a negative delta-seconds value falls back to normal backoff rather than a zero-second hot retry.