Raise Peddler errors when retries are exhausted on an error status - #274
Merged
Conversation
When retries are enabled, HTTP::OutOfRetriesError from the http client previously bypassed Peddler's error wrapping. The same Amazon 429 would raise a different error class depending on the retries setting, breaking rescue Peddler::Error blocks. API#request now rescues HTTP::OutOfRetriesError: when it carries the last HTTP response, Peddler raises the same error that response would have raised unretried (Ruby's implicit exception chaining preserves the original error in cause). Exhaustion with no response (all attempts failed at the network level) re-raises untouched. Includes two tests covering both cases.
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
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.
Problem
With
retries: > 0, exhausted retries raiseHTTP::OutOfRetriesErrorfrom inside the http client, bypassingResponse.wrapentirely. The error class for the same Amazon failure depends on client configuration:Anyone following the README (
rescue Peddler::Error, promised since v5.0: "All HTTP errors now raisePeddler::Error") silently stops catching throttles the moment they turn retries on — which is exactly when throttles matter most.Verified against a local always-503 server: retriable alone retries correctly and raises
OutOfRetriesErrorwith the final response attached; that error then escapes Peddler's wrapping.Fix
API#requestrescuesHTTP::OutOfRetriesError. When it carries the last HTTP response (OutOfRetriesError#response, populated whenever the final attempt produced a status), Peddler raises the same error that response would have raised unretried, minted by the same builder:Ruby's implicit exception chaining preserves the original in
cause(asserted in the test):Exhaustion with no response at all — every attempt failed at the network level, or the final attempt did (http.rb's performer only carries the last attempt's result) — re-raises untouched. There is no HTTP error to represent; wrapping transport failures is a separate, deliberately deferred design question.
Notes
HTTP::OutOfRetriesErroraround Peddler calls; filed under Fixed as it restores the documented contract, with the original error still reachable viacause.