Skip to content

Carry the HTTP status code in the exceptions ApiCall::getException() creates - #125

Open
cyppe wants to merge 1 commit into
typesense:masterfrom
cyppe:fix/exception-http-status-code
Open

Carry the HTTP status code in the exceptions ApiCall::getException() creates#125
cyppe wants to merge 1 commit into
typesense:masterfrom
cyppe:fix/exception-http-status-code

Conversation

@cyppe

@cyppe cyppe commented Sep 9, 2026

Copy link
Copy Markdown

Problem

Every exception created by ApiCall::getException() is constructed without a code, so $exception->getCode() is always 0:

try {
    $client->collections['missing']->retrieve();
} catch (TypesenseClientError $e) {
    $e->getCode(); // 0, whatever the server answered
}

A caller that wants the status has to reverse-map the exception class, and the classes cannot express the difference between statuses that share a class: 403, 429, 502 and 504 all arrive as a bare TypesenseClientError. In our application we needed the status to decide between "retry later" (0, 408, 429, 5xx) and "give up" (other 4xx) and to log it; we have been running this change from a fork since July.

Change

getException() passes the HTTP status as the exception code. The message is still empty at construction and is still set by ->setMessage() in makeRequest(), so:

  • exception classes are unchanged,
  • messages are unchanged,
  • retry behaviour is unchanged,
  • the only observable difference is that getCode() returns the HTTP status where it returned 0 before (HTTPStatus0Error keeps 0).

This is the convention of the other HTTP-facing PHP clients (Guzzle's RequestException, Symfony's HttpExceptionInterface::getStatusCode()), and it is additive for callers: code that never read getCode() sees no difference.

Tests

  • testExceptionsCarryTheHttpStatusCode: every mapped status (0, 400, 401, 404, 409, 422, 500, 503) and unmapped ones (403, 408, 429, 502, 504) return an exception whose code is the status and whose message is empty.
  • testThrownExceptionsCarryTheStatusCodeAndTheServerMessage: end to end through ApiCall::get() with a mocked 404 response: ObjectNotFound, code 404, message from the server body.

vendor/bin/phpunit tests/Feature/ApiCallRetryTest.php: 15 tests, 58 assertions, OK. Full suite against Typesense 30.2: the same 6 cases fail with and without this change (analytics events and curation sets, environment-specific), everything else passes. phpcs reports no new violations in the two touched files (the pre-existing ones in ApiCall.php are untouched).

🤖 Generated with Claude Code

…creates

Every exception built by ApiCall::getException() was constructed without a
code, so $exception->getCode() is always 0. A caller that wants the status
(to log it, to decide between retry and give-up, or to map it to its own
error) has to reverse-map the exception class, and the classes cannot
express the difference between, say, a 429 and a 502: both arrive as a
bare TypesenseClientError.

Pass the status code as the exception code. The message is still empty at
construction time and is still set by ApiCall::setMessage() afterwards, so
messages, exception classes and the retry behaviour are unchanged; the only
difference is that getCode() now returns the HTTP status where it returned
0 before (HTTPStatus0Error keeps 0).

Two tests: one for every mapped and unmapped status, and one end to end
through ApiCall::get() with a mocked 404 response that checks both the
code and the server message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant