Skip to content

Commit d4152fe

Browse files
fredrikekreclaude
andauthored
Fix MethodError when an armed retry is abandoned at the request deadline (#1324)
When a retry has been armed (its retry-bucket reservation acquired) but _sleep_retry_delay! bails out because the backoff delay would exceed the request deadline, the finally block refunded the reservation with `release(bucket, token, nothing)`. `release` only accepts an `Int` failure cost, so instead of gracefully giving up and returning the last response, the request failed with MethodError: no method matching release(::RetryBucket, ::RetryToken, ::Nothing) Pass the cost through `_retry_bucket_failure_cost(nothing)` (i.e. 0, a full refund) as intended. This path triggers in the wild whenever a deadline-bounded request (e.g. `request_timeout` set) is retried against a slow or unavailable server and the backoff no longer fits within the deadline. Add a regression test covering the abandoned-retry path, including that the reservation is refunded in full. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 04a21cf commit d4152fe

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

src/http_client_retry.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ function _arm_request_retry!(
206206
_sleep_retry_delay!(request, delay_ns) || return false, nothing, delay_ns
207207
ok = true
208208
finally
209-
ok || (bucket !== nothing && token !== nothing && release(bucket::RetryBucket, token, nothing))
209+
ok || (bucket !== nothing && token !== nothing && release(bucket::RetryBucket, token, _retry_bucket_failure_cost(nothing)))
210210
end
211211
controller.remaining -= 1
212212
return true, token, delay_ns

test/http_retry_tests.jl

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,39 @@ end
511511
end
512512
end
513513

514+
@testset "retry arming refunds the bucket when the deadline preempts the backoff" begin
515+
listener = ND.listen("tcp", "127.0.0.1:0"; backlog = 8)
516+
address = ND.join_host_port("127.0.0.1", Int((NC.addr(listener)::NC.SocketAddrV4).port))
517+
base_url = "http://$(address)"
518+
seen = Tuple{String, String, String}[]
519+
server_task = _serve_retry_sequence(listener, [
520+
(status = 503, reason = "Service Unavailable"),
521+
], seen)
522+
try
523+
# A backoff far beyond the request deadline means the armed retry is
524+
# abandoned before it sleeps. The bucket reservation must be refunded
525+
# rather than the release erroring (it used to be called with `nothing`
526+
# as the failure cost, which has no method).
527+
bucket = HT.RetryBucket(capacity = 15, backoff_scale_factor_ms = 60_000, max_backoff_secs = 60)
528+
response = HT.get(
529+
"$(base_url)/deadline";
530+
retries = 2,
531+
retry_bucket = bucket,
532+
request_timeout = 1,
533+
status_exception = false,
534+
)
535+
@test response.status == 503
536+
_wait_task_retry!(server_task)
537+
@test seen == [("GET", "/deadline", "")]
538+
# Full refund: with capacity 15 a second reservation (cost 10) only
539+
# succeeds if the abandoned one returned its 10.
540+
token = Base.acquire(bucket, only(keys(bucket.partitions)))
541+
Base.release(bucket, token, 0)
542+
finally
543+
HTTP.@try_ignore NC.close(listener)
544+
end
545+
end
546+
514547
@testset "HTTP.open retries idempotent buffered requests" begin
515548
listener = ND.listen("tcp", "127.0.0.1:0"; backlog = 8)
516549
address = ND.join_host_port("127.0.0.1", Int((NC.addr(listener)::NC.SocketAddrV4).port))

0 commit comments

Comments
 (0)