feat: preserve HTTP status and response body on client errors#3
Merged
Merged
Conversation
…se body Callers could previously only catch a generic RuntimeException with no way to distinguish transient 5xx from permanent 4xx errors, or to read the Lever error payload. Introduce LeverApiException (extends RuntimeException for backward compatibility) that carries statusCode, responseBody, and the original Guzzle exception as the previous. Also calls parent::setUp() in the test case so the Log facade is available during request error paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LeverApiException(extendsRuntimeException) that carriesstatusCode,responseBody, andisClientError()/isServerError()helpers.handleExceptionnow throws this typed exception with the original Guzzle exception chained as$previous.Why
Downstream apps (e.g.
spot) were retrying 4xx errors seven times under exponential backoff because the rewrappedRuntimeExceptioncarried no status. This produced hundreds of Bugsnag events for a single malformed posting and gave admins no actionable error message.Backward compatibility
Safe:
catch (\RuntimeException $e)still catches —LeverApiException extends RuntimeException.catch (\Exception $e)/catch (\Throwable $e)still works.Minor behavior changes (suggest tagging as v1.1.0, not a patch):
$e->getCode()— was always0; now returns the HTTP status (e.g.400,503) forClientExceptionpaths.$e->getPrevious()— was alwaysnull; now returns the underlying GuzzleClientException.Real-world impact should be zero unless a consumer explicitly checks
getCode() === 0orgetPrevious() === null, but these are observable changes, hence minor (not patch) bump.Test plan
vendor/bin/phpunit— all 16 tests passisClientError(), and chained previous exception on a 400 responsefail_to_create_opportunity_when_no_perform_as_parameter_includedstill passes