From ed699ade219c9b0c308404f4103c56d1a71f9436 Mon Sep 17 00:00:00 2001 From: Mathias Grimm Date: Mon, 20 Jul 2026 12:21:28 -0300 Subject: [PATCH 1/3] Add typed RateLimitException and ForbiddenException 429 responses now throw RateLimitException carrying the Retry-After delay in seconds, and 403 responses throw ForbiddenException with the API message, so the CLI can retry rate limits with backoff and explain ability rejections (the public CI token hitting a non-analyze endpoint) instead of collapsing both into the generic ApiException. Co-Authored-By: Claude Fable 5 --- src/Client.php | 17 +++++++++++++++++ src/ForbiddenException.php | 5 +++++ src/RateLimitException.php | 14 ++++++++++++++ tests/ClientTest.php | 39 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100644 src/ForbiddenException.php create mode 100644 src/RateLimitException.php diff --git a/src/Client.php b/src/Client.php index dbb1710..b0a3bee 100644 --- a/src/Client.php +++ b/src/Client.php @@ -138,6 +138,14 @@ private function guard(Response $response): Response throw new AuthException('Invalid or missing token.'); } + if ($response->status() === 403) { + $message = $response->json('message'); + + throw new ForbiddenException( + is_string($message) && $message !== '' ? $message : 'This token may not call this endpoint.', + ); + } + if ($response->status() === 422) { $message = $response->json('message'); $errors = $response->json('errors'); @@ -148,6 +156,15 @@ private function guard(Response $response): Response ); } + if ($response->status() === 429) { + $retryAfter = $response->header('Retry-After'); + + throw new RateLimitException( + 'The API rate limit was reached.', + is_numeric($retryAfter) ? (int) $retryAfter : null, + ); + } + if ($response->failed()) { $message = $response->json('message'); diff --git a/src/ForbiddenException.php b/src/ForbiddenException.php new file mode 100644 index 0000000..1585667 --- /dev/null +++ b/src/ForbiddenException.php @@ -0,0 +1,5 @@ + Factory::response(['message' => 'Invalid ability provided.'], 403)]); + + expect(fn () => client($http)->convert(Images::png(), ImageFormat::Jpg)) + ->toThrow(ForbiddenException::class, 'Invalid ability provided.'); +}); + +test('a 403 response without a message gets the fallback text', function () { + $http = fakeHttp(['*/v1/convert' => Factory::response([], 403)]); + + expect(fn () => client($http)->convert(Images::png(), ImageFormat::Jpg)) + ->toThrow(ForbiddenException::class, 'This token may not call this endpoint.'); +}); + +test('a 429 response maps to RateLimitException carrying Retry-After', function () { + $http = fakeHttp(['*/v1/analyze' => Factory::response(['message' => 'Too Many Requests'], 429, ['Retry-After' => '17'])]); + + try { + client($http)->analyze(ImageFormat::Jpg, 2_500_000); + $this->fail('Expected a RateLimitException.'); + } catch (RateLimitException $e) { + expect($e->getMessage())->toBe('The API rate limit was reached.') + ->and($e->retryAfterSeconds)->toBe(17); + } +}); + +test('a 429 response without a Retry-After header yields a null delay', function () { + $http = fakeHttp(['*/v1/analyze' => Factory::response(['message' => 'Too Many Requests'], 429)]); + + try { + client($http)->analyze(ImageFormat::Jpg, 2_500_000); + $this->fail('Expected a RateLimitException.'); + } catch (RateLimitException $e) { + expect($e->retryAfterSeconds)->toBeNull(); + } +}); + test('other failures map to ApiException with the status code', function () { $http = fakeHttp(['*/v1/optimize' => Factory::response(['message' => 'Server Error'], 500)]); From 97c2081dd586c42f6485cbd775707dd5b7154ffb Mon Sep 17 00:00:00 2001 From: Mathias Grimm Date: Mon, 20 Jul 2026 12:25:33 -0300 Subject: [PATCH 2/3] Harden Retry-After parsing and keep the API's 429 message Apply the review findings from codex and claude: parse the HTTP-date form of Retry-After, clamp negative and round up fractional delays so callers can sleep the value as-is, prefer the API's message over the hardcoded fallback, and lock the new exceptions' ApiException catchability in tests. Co-Authored-By: Claude Fable 5 --- src/Client.php | 29 +++++++++++++++++++++--- src/RateLimitException.php | 2 +- tests/ClientTest.php | 46 +++++++++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/Client.php b/src/Client.php index b0a3bee..9ba8603 100644 --- a/src/Client.php +++ b/src/Client.php @@ -157,11 +157,11 @@ private function guard(Response $response): Response } if ($response->status() === 429) { - $retryAfter = $response->header('Retry-After'); + $message = $response->json('message'); throw new RateLimitException( - 'The API rate limit was reached.', - is_numeric($retryAfter) ? (int) $retryAfter : null, + is_string($message) && $message !== '' ? $message : 'The API rate limit was reached.', + $this->retryAfterSeconds($response), ); } @@ -178,6 +178,29 @@ private function guard(Response $response): Response return $response; } + /** + * Parse the Retry-After header, which RFC 9110 allows as either + * delay-seconds or an HTTP date. Negative and fractional delays are + * clamped so a caller can sleep the value as-is; an absent or + * unparseable header yields null. + */ + private function retryAfterSeconds(Response $response): ?int + { + $header = $response->header('Retry-After'); + + if ($header === '') { + return null; + } + + if (is_numeric($header)) { + return max(0, (int) ceil((float) $header)); + } + + $timestamp = strtotime($header); + + return $timestamp === false ? null : max(0, $timestamp - time()); + } + private function requireToken(): string { $token = $this->token instanceof Closure ? ($this->token)() : $this->token; diff --git a/src/RateLimitException.php b/src/RateLimitException.php index 0e2e719..41cb14c 100644 --- a/src/RateLimitException.php +++ b/src/RateLimitException.php @@ -5,7 +5,7 @@ class RateLimitException extends ApiException { /** - * @param ?int $retryAfterSeconds Seconds from the Retry-After header, or null when the API sent none + * @param ?int $retryAfterSeconds Seconds to wait from the Retry-After header (never negative), or null when the header is missing or unparseable */ public function __construct(string $message, public readonly ?int $retryAfterSeconds = null) { diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 7726dd7..f02fac9 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -387,7 +387,7 @@ function client(Factory $http, Closure|string|null $token = 'test-token', string client($http)->analyze(ImageFormat::Jpg, 2_500_000); $this->fail('Expected a RateLimitException.'); } catch (RateLimitException $e) { - expect($e->getMessage())->toBe('The API rate limit was reached.') + expect($e->getMessage())->toBe('Too Many Requests') ->and($e->retryAfterSeconds)->toBe(17); } }); @@ -403,6 +403,50 @@ function client(Factory $http, Closure|string|null $token = 'test-token', string } }); +test('a 429 response prefers the API message over the fallback', function () { + $http = fakeHttp(['*/v1/analyze' => Factory::response(['message' => 'Shared token limit reached.'], 429)]); + + expect(fn () => client($http)->analyze(ImageFormat::Jpg, 2_500_000)) + ->toThrow(RateLimitException::class, 'Shared token limit reached.'); +}); + +test('Retry-After parsing clamps and rounds odd delay values', function (string $header, ?int $expected) { + $http = fakeHttp(['*/v1/analyze' => Factory::response([], 429, ['Retry-After' => $header])]); + + try { + client($http)->analyze(ImageFormat::Jpg, 2_500_000); + $this->fail('Expected a RateLimitException.'); + } catch (RateLimitException $e) { + expect($e->retryAfterSeconds)->toBe($expected); + } +})->with([ + 'negative clamps to zero' => ['-5', 0], + 'fractional rounds up' => ['2.5', 3], + 'garbage yields null' => ['soon', null], +]); + +test('an HTTP-date Retry-After resolves to the remaining seconds', function () { + $http = fakeHttp(['*/v1/analyze' => Factory::response([], 429, [ + 'Retry-After' => gmdate('D, d M Y H:i:s \G\M\T', time() + 30), + ])]); + + try { + client($http)->analyze(ImageFormat::Jpg, 2_500_000); + $this->fail('Expected a RateLimitException.'); + } catch (RateLimitException $e) { + expect($e->retryAfterSeconds)->toBeGreaterThanOrEqual(28) + ->and($e->retryAfterSeconds)->toBeLessThanOrEqual(30); + } +}); + +test('the new exceptions stay catchable as ApiException', function () { + $limited = fakeHttp(['*/v1/analyze' => Factory::response([], 429)]); + $forbidden = fakeHttp(['*/v1/convert' => Factory::response([], 403)]); + + expect(fn () => client($limited)->analyze(ImageFormat::Jpg, 2_500_000))->toThrow(ApiException::class) + ->and(fn () => client($forbidden)->convert(Images::png(), ImageFormat::Jpg))->toThrow(ApiException::class); +}); + test('other failures map to ApiException with the status code', function () { $http = fakeHttp(['*/v1/optimize' => Factory::response(['message' => 'Server Error'], 500)]); From c20f0124efb135b879f035351fbf787ef0b30243 Mon Sep 17 00:00:00 2001 From: Mathias Grimm Date: Mon, 20 Jul 2026 14:07:58 -0300 Subject: [PATCH 3/3] Rename the namespace to MathiasGrimm\GlimpsePhp GlimpseImg\ becomes MathiasGrimm\GlimpsePhp\, aligning the SDK with the MathiasGrimm vendor namespace used across the other packages. Breaking change: consumers must update their imports. The Composer package name stays mathiasgrimm/glimpse-php. Co-Authored-By: Claude Fable 5 --- README.md | 4 ++-- composer.json | 4 ++-- src/ApiException.php | 2 +- src/AuthException.php | 2 +- src/Client.php | 2 +- src/ForbiddenException.php | 2 +- src/FrameCounter.php | 2 +- src/ImageFormat.php | 2 +- src/ImageInfo.php | 2 +- src/ImageResolution.php | 2 +- src/ImageResult.php | 2 +- src/ProbeResult.php | 2 +- src/RateLimitException.php | 2 +- src/SampleProbe.php | 2 +- src/SizeEstimate.php | 2 +- src/UsagePeriod.php | 2 +- src/UsageSummary.php | 2 +- src/User.php | 2 +- src/ValidationException.php | 2 +- tests/ClientTest.php | 24 ++++++++++++------------ tests/Fixtures/Images.php | 2 +- tests/FrameCounterTest.php | 4 ++-- tests/ImageFormatTest.php | 4 ++-- tests/Pest.php | 2 +- tests/SampleProbeTest.php | 4 ++-- 25 files changed, 41 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 6546680..ea81106 100644 --- a/README.md +++ b/README.md @@ -25,8 +25,8 @@ composer require mathiasgrimm/glimpse-php ``` ```php -use GlimpseImg\Client; -use GlimpseImg\ImageFormat; +use MathiasGrimm\GlimpsePhp\Client; +use MathiasGrimm\GlimpsePhp\ImageFormat; use Illuminate\Http\Client\Factory; $glimpse = new Client(new Factory, 'your-api-token'); diff --git a/composer.json b/composer.json index e8badbc..1818bcb 100644 --- a/composer.json +++ b/composer.json @@ -30,12 +30,12 @@ }, "autoload": { "psr-4": { - "GlimpseImg\\": "src/" + "MathiasGrimm\\GlimpsePhp\\": "src/" } }, "autoload-dev": { "psr-4": { - "GlimpseImg\\Tests\\": "tests/" + "MathiasGrimm\\GlimpsePhp\\Tests\\": "tests/" } }, "scripts": { diff --git a/src/ApiException.php b/src/ApiException.php index 9fadcf3..fd634f0 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -1,6 +1,6 @@ count(Images::animatedGif()))->toBe(3); diff --git a/tests/ImageFormatTest.php b/tests/ImageFormatTest.php index 3a72f37..0753b10 100644 --- a/tests/ImageFormatTest.php +++ b/tests/ImageFormatTest.php @@ -1,7 +1,7 @@ toBe(ImageFormat::Jpg) diff --git a/tests/Pest.php b/tests/Pest.php index d7a1b24..ccab4f9 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -1,6 +1,6 @@