Skip to content

Commit a4c3216

Browse files
Make raise_error work with retriable feature
raise_error converts error responses into StatusError inside each attempt, blocking retry_statuses matching from seeing the response. This prevents retries and makes retry_statuses dead configuration when both features are enabled. Recover the response carried by StatusError in try_request, so retry_statuses matching, delay calculation, callbacks, and OutOfRetriesError all see it. Treat StatusError with matching status as retriable in retry_request?. Non-matching StatusErrors still raise immediately. Principle: raise-on-status should act only after retry-on-status is exhausted, but raise_error runs per-attempt inside the retry loop. Co-authored-by: Hakan Ensari <hakanensari@gmail.com>
1 parent 557b127 commit a4c3216

4 files changed

Lines changed: 169 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020
JVM could garbage-collect llhttp's native callback trampolines while a parser
2121
was still in use, after which `llhttp_execute` succeeded without invoking any
2222
callbacks. Callback procs are now retained for the lifetime of the parser.
23+
- The `raise_error` and `retriable` features now compose. Previously
24+
`raise_error` converted an error response into a `StatusError` inside each
25+
attempt, so `retry_statuses` never saw the response and no retries happened.
26+
The retry performer now recovers the response carried by a `StatusError`,
27+
so status-based retries, `Retry-After` delay calculation, `on_retry`, and
28+
`OutOfRetriesError#response` all see it. ([#848])
2329

2430
## [6.0.4] - 2026-07-14
2531

@@ -330,6 +336,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
330336
[#785]: https://github.com/httprb/http/issues/785
331337
[#826]: https://github.com/httprb/http/issues/826
332338
[#841]: https://github.com/httprb/http/pull/841
339+
[#848]: https://github.com/httprb/http/pull/848
333340
[unreleased]: https://github.com/httprb/http/compare/v6.0.4...HEAD
334341
[6.0.4]: https://github.com/httprb/http/compare/v6.0.3...v6.0.4
335342
[6.0.3]: https://github.com/httprb/http/compare/v6.0.2...v6.0.3

lib/http/retriable/performer.rb

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ def try_request
113113
res = yield
114114
rescue Exception => e
115115
err = e
116+
# The raise_error feature converts an error response into a
117+
# StatusError inside the attempt; recover the response so
118+
# retry_statuses, delay calculation and OutOfRetriesError
119+
# still see it.
120+
res = e.response if e.is_a?(StatusError)
116121
end
117122

118123
[err, res]
@@ -127,7 +132,7 @@ def retry_request?(req, err, res, attempt)
127132
if @should_retry_proc
128133
@should_retry_proc.call(req, err, res, attempt)
129134
elsif err
130-
retry_exception?(err)
135+
retry_exception?(err) || retriable_status_error?(err)
131136
else
132137
retry_response?(res)
133138
end
@@ -141,6 +146,15 @@ def retry_exception?(err)
141146
@exception_classes.any? { |e| err.is_a?(e) }
142147
end
143148

149+
# Checks whether the error carries a response that warrants retry
150+
#
151+
# @param [Exception] err
152+
# @api private
153+
# @return [Boolean]
154+
def retriable_status_error?(err)
155+
err.is_a?(StatusError) && retry_response?(err.response)
156+
end
157+
144158
# Checks whether the response status warrants retry
145159
#
146160
# @api private

sig/http.rbs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,6 +1601,7 @@ module HTTP
16011601
def try_request: () { () -> Response? } -> [Exception?, Response?]
16021602
def retry_request?: (Request req, Exception? err, Response? res, Integer attempt) -> bool
16031603
def retry_exception?: (Exception err) -> bool
1604+
def retriable_status_error?: (Exception err) -> bool
16041605
def retry_response?: (untyped res) -> bool
16051606
def wait_for_retry_or_raise: (Request req, Exception? err, Response? res, Integer attempt) -> void
16061607
def out_of_retries_error: (Request request, Response? response, Exception? exception) -> OutOfRetriesError

test/http/retriable/performer_test.rb

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,12 @@ class CustomSubException < HTTP::TimeoutError
1414
end
1515
end
1616

17+
# Subclass for testing is_a? vs instance_of? in StatusError handling
18+
unless defined?(CustomStatusError)
19+
class CustomStatusError < HTTP::StatusError
20+
end
21+
end
22+
1723
class HTTPRetriablePerformerTest < Minitest::Test
1824
cover "HTTP::Retriable::Performer*"
1925

@@ -196,6 +202,146 @@ def test_perform_unexpected_status_does_not_retry
196202
assert_equal 1, counter_spy
197203
end
198204

205+
# -- StatusError raised inside the attempt (raise_error feature interop) --
206+
207+
def error_response
208+
@error_response ||= make_response(status: 503)
209+
end
210+
211+
def test_perform_retries_status_error_matching_retry_statuses
212+
assert_raises HTTP::OutOfRetriesError do
213+
perform(retry_statuses: [503], tries: 3) do
214+
raise HTTP::StatusError, error_response
215+
end
216+
end
217+
assert_equal 3, counter_spy
218+
end
219+
220+
def test_perform_retries_status_error_subclasses_and_attaches_response
221+
err = nil
222+
begin
223+
perform(retry_statuses: [503], tries: 2) do
224+
raise CustomStatusError, error_response
225+
end
226+
rescue HTTP::OutOfRetriesError => e
227+
err = e
228+
end
229+
230+
assert_equal error_response, err.response
231+
assert_equal 2, counter_spy
232+
end
233+
234+
def test_perform_does_not_retry_status_error_not_matching_retry_statuses
235+
assert_raises HTTP::StatusError do
236+
perform(retry_statuses: [503], tries: 3) do
237+
raise HTTP::StatusError, make_response(status: 404)
238+
end
239+
end
240+
assert_equal 1, counter_spy
241+
end
242+
243+
def test_perform_does_not_retry_plain_exception_when_retry_statuses_configured
244+
assert_raises CustomException do
245+
perform(retry_statuses: [503], tries: 3) do
246+
raise CustomException
247+
end
248+
end
249+
assert_equal 1, counter_spy
250+
end
251+
252+
def test_perform_does_not_retry_status_error_without_retry_statuses
253+
assert_raises HTTP::StatusError do
254+
perform(tries: 3) do
255+
raise HTTP::StatusError, error_response
256+
end
257+
end
258+
assert_equal 1, counter_spy
259+
end
260+
261+
def test_out_of_retries_error_from_status_error_has_response_and_cause
262+
err = nil
263+
begin
264+
perform(retry_statuses: [503], tries: 2) do
265+
raise HTTP::StatusError, error_response
266+
end
267+
rescue HTTP::OutOfRetriesError => e
268+
err = e
269+
end
270+
271+
assert_equal error_response, err.response
272+
assert_kind_of HTTP::StatusError, err.cause
273+
end
274+
275+
def test_out_of_retries_error_from_status_error_in_exceptions_list_has_response
276+
err = nil
277+
begin
278+
perform(exceptions: [HTTP::StatusError], tries: 2) do
279+
raise HTTP::StatusError, error_response
280+
end
281+
rescue HTTP::OutOfRetriesError => e
282+
err = e
283+
end
284+
285+
assert_equal error_response, err.response
286+
end
287+
288+
def test_on_retry_callback_with_status_error_receives_error_and_response
289+
callback_call_spy = 0
290+
291+
callback_spy = proc do |callback_request, error, callback_response|
292+
assert_equal request, callback_request
293+
assert_kind_of HTTP::StatusError, error
294+
assert_equal error_response, callback_response
295+
callback_call_spy += 1
296+
end
297+
298+
assert_raises HTTP::OutOfRetriesError do
299+
perform(retry_statuses: [503], tries: 3, on_retry: callback_spy) do
300+
raise HTTP::StatusError, error_response
301+
end
302+
end
303+
304+
assert_equal 2, callback_call_spy
305+
end
306+
307+
def test_calculate_delay_receives_status_error_response
308+
responses_seen = []
309+
310+
performer = HTTP::Retriable::Performer.new(delay: 0, retry_statuses: [503], tries: 2)
311+
calculator = performer.instance_variable_get(:@delay_calculator)
312+
original_call = calculator.method(:call)
313+
calculator.define_singleton_method(:call) do |iteration, resp|
314+
responses_seen << resp
315+
original_call.call(iteration, resp)
316+
end
317+
318+
begin
319+
performer.perform(client, request) { raise HTTP::StatusError, error_response }
320+
rescue HTTP::OutOfRetriesError
321+
nil
322+
end
323+
324+
assert_equal error_response, responses_seen.first
325+
end
326+
327+
def test_response_flushing_flushes_status_error_response_when_retries_exhausted
328+
flushed = false
329+
error_response.define_singleton_method(:flush) do
330+
flushed = true
331+
self
332+
end
333+
334+
begin
335+
perform(retry_statuses: [503], tries: 2) do
336+
raise HTTP::StatusError, error_response
337+
end
338+
rescue HTTP::OutOfRetriesError
339+
nil
340+
end
341+
342+
assert flushed, "expected StatusError response to be flushed on final attempt"
343+
end
344+
199345
# -- on_retry callback --
200346

201347
def test_on_retry_callback_with_exception

0 commit comments

Comments
 (0)