Skip to content

Commit 0c3b758

Browse files
quinnjclaude
andauthored
Fix retry-budget exhaustion and correlated reused-connection death (#1354)
* Fix retry-budget exhaustion and correlated reused-connection death A client sending periodic bursts against a peer that silently discards idle pooled connections saw transient errors escape HTTP.request with default retry=true after ~50 burst cycles, permanently (#1353). Root cause: every armed high-level retry consumed 10 of the per-host RetryBucket's 500 tokens even when the retried attempt succeeded, the bucket never refilled, and once empty the denial was silent — so the budget was a fuse that burned out in ~50 recoveries, after which every transient failure surfaced raw. - RetryBucket: refund the reservation when a retried attempt reaches a non-retryable response (retryable 408/429/5xx responses keep partial cost, exception outcomes keep full cost), and credit 1 unit per successful non-retried request so a drained partition heals from healthy traffic. A depleted-partition counter keeps the per-request replenish check lock-free while all partitions are full. - Transport: retry a replayable idempotent request while failures keep landing on *reused* pooled connections (bounded by max_idle_per_host + 1 acquisitions) instead of exactly once, so a correlated-death batch is burned through down to a fresh dial without consuming high-level retry budget; classify TLSError on reused connections by its cause so dead reused TLS connections take this path too. - Add RetrySkippedEvent so a denied (:retry_bucket) or deadline- preempted (:deadline) retry is observable in request traces. - Add TLSTransportError for TLS I/O failures on established connections; handshake failures are now typed TLSHandshakeError at the dial sites instead of one blanket wrap that mislabeled mid-request read errors as handshake errors. isrecoverable unwraps both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Cover TLS dial-site wrapping and RetrySkippedEvent emission paths Add tests for the handshake-phase TLSHandshakeError wrap on both the HTTP/1 transport dial and connect_h2!, the request-path (exception) RetrySkippedEvent emission, and the verbose trace formatting of RetrySkippedEvent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(client): harden retry and transport recovery Make retry-budget accounting exact across built-in and custom policies. Recover poisoned pooled connections without leaking slots or reusing a failed connection. Normalize established TLS failures across HTTP/1 and HTTP/2 public client boundaries. Add regression coverage for terminal accounting, trace failures, one-shot bodies, concurrent pool handoff, and truncated TLS records. Fixes #1353 * Centralize the TLS truncation-message classification Both retry classifiers matched Reseau's truncated-TLS-stream message with a duplicated string literal; hoist it to a single documented constant so a Reseau wording change is a one-line fix, and note that the end-to-end truncation tests pin the coupling. Also smooth the retry-budget CHANGELOG entry into readable prose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent bc251f0 commit 0c3b758

17 files changed

Lines changed: 2029 additions & 309 deletions

CHANGELOG.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,51 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111
- Added `HTTP2Settings` to configure HTTP/2 receive flow-control windows (per-stream `initial_window_size` and connection-level `connection_window_size`). Pass it via the `http2_settings` keyword on `Client`, `Server`, `listen!`, `serve!`, `serve`, and `connect_h2!`. Defaults preserve the protocol-default 65535-byte windows, and the per-stream receive buffer cap is derived from the window. Raising the windows improves single-stream throughput on links with non-trivial latency.
1212
- Added `HTTP.peeraddr(::HTTP.Stream)`, returning the remote (client) `SocketAddr` of a server stream for both plain-TCP and TLS connections and both HTTP/1 and HTTP/2. This is the supported way to obtain the client IP (for rate limiting, audit logging, and per-client policy) without reaching into transport internals, and restores the capability `Sockets.getpeername(::HTTP.Stream)` provided in HTTP.jl 1.x.
13+
- Added `HTTP.RetrySkippedEvent`, a request trace event emitted when the retry
14+
policy wanted to retry an attempt but the retry was not armed — because the
15+
transport's `RetryBucket` denied capacity (`reason = :retry_bucket`) or the
16+
request deadline preempted the backoff (`reason = :deadline`). Previously a
17+
denied retry was indistinguishable from a non-retryable failure. ([#1353])
18+
- Added `HTTP.TLSTransportError`, raised when TLS-level I/O fails on an
19+
established connection during a request. Previously such failures were
20+
mislabeled `TLSHandshakeError`; that type is now reserved for actual
21+
connection-setup failures. `HTTP.isrecoverable` classifies both wrappers by
22+
their underlying cause. ([#1353])
1323

1424
### Fixed
1525
- Restored HTTP and WebSocket server task scheduling to Julia's `:interactive`
1626
thread pool so default-pool compute work cannot starve server and health-check
1727
tasks when an interactive thread is configured. ([#1342])
1828
- Percent-decode `userinfo` before building the `Basic` auth header (RFC 3986); fixes wrong credentials for request URLs and proxies containing percent-encoded characters.
29+
- Fixed the client retry budget (`RetryBucket`) treating a successful retried
30+
attempt as a full-cost failure. The per-host budget drained by 10 of 500
31+
units on every retry — even one that recovered with a 2xx — and never
32+
refilled, so after ~50 retries against a host every subsequent retry was
33+
silently denied for the transport's lifetime and transient errors surfaced
34+
raw despite `retry=true`. A retry reservation is now settled by the
35+
effective retry decision for the response it produced: refunded in full when
36+
the built-in policy (or a custom `retry_if`) no longer wants a retry, and
37+
keeping the partial cost when the response is still classified as a failure.
38+
On the final attempt the built-in classification applies without invoking
39+
`retry_if`, and a retry that `retry_if` explicitly requested conservatively
40+
keeps cost on a non-2xx/3xx outcome. Each successful non-retried request
41+
restores one unit of previously consumed budget, retry reservations and
42+
response connections are released even when a trace or retry-policy callback
43+
throws, and the request deadline is rechecked after the backoff sleep.
44+
([#1353])
45+
- The HTTP/1 transport now retries a replayable idempotent request for as long
46+
as failures land on *reused* pooled connections. It tries at most
47+
`max_idle_per_host` reused connections, then forces a fresh dial instead of
48+
accepting another concurrent pool return. PUT and DELETE receive the same
49+
stale-connection recovery as the other idempotent methods.
50+
Pooled connections can be discarded by the peer in correlated batches, in
51+
which case the single retry would draw the next equally-dead pooled
52+
connection and fail. ([#1353])
53+
- Dead reused TLS connections that fail with `Reseau.TLS.TLSError` (for
54+
example an RST surfacing as a wrapped `SystemError` or a truncated TLS record
55+
reported as `unexpected EOF`) are now classified by their public error shape
56+
in the transport's reused-connection retry. HTTP/2 read-loop wrappers also
57+
preserve this classification. ([#1353])
1958

2059
## [v2.0.0] - 2026-04-27
2160
HTTP.jl 2.0 is a major rewrite of the package internals and public API. The
@@ -826,3 +865,4 @@ See changes for 0.9.15: this release is equivalent to 0.9.15 with [#752] reverte
826865
[#1126]: https://github.com/JuliaWeb/HTTP.jl/issues/1126
827866
[#1127]: https://github.com/JuliaWeb/HTTP.jl/issues/1127
828867
[#1342]: https://github.com/JuliaWeb/HTTP.jl/issues/1342
868+
[#1353]: https://github.com/JuliaWeb/HTTP.jl/issues/1353

docs/src/api/client.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ HTTP.isaborted
4343
HTTP.RequestEvent
4444
HTTP.ResponseHeadEvent
4545
HTTP.RetryEvent
46+
HTTP.RetrySkippedEvent
4647
HTTP.RedirectEvent
4748
HTTP.DoneEvent
4849
```

docs/src/api/core.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ HTTP.TooManyRedirectsError
2727
HTTP.ConnectError
2828
HTTP.DNSError
2929
HTTP.TLSHandshakeError
30+
HTTP.TLSTransportError
3031
HTTP.AddressInUseError
3132
```
3233

docs/src/guides/client.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,7 @@ logger — pass a `trace` callback. The callback receives subtypes of
511511
- [`HTTP.RequestEvent`](@ref) — request being sent
512512
- [`HTTP.ResponseHeadEvent`](@ref) — response headers received
513513
- [`HTTP.RetryEvent`](@ref) — retry scheduled
514+
- [`HTTP.RetrySkippedEvent`](@ref) — retry denied by the budget or deadline
514515
- [`HTTP.RedirectEvent`](@ref) — redirect followed
515516
- [`HTTP.DoneEvent`](@ref) — request finished (with response or error)
516517

src/HTTP.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ include("http_websockets.jl")
7878
:Handlers, :Headers, :NoProxy, :ParseError, :ProtocolError, :ProxyConfig,
7979
:ProxyFromEnvironment, :ProxyURL, :RedirectEvent, :Request, :RequestContext,
8080
:RequestEvent, :RequestRetryError, :Response, :ResponseHeadEvent, :RetryBucket,
81-
:RetryEvent, :SSEEvent, :SSEStream, :Server, :StatusError, :Stream, :TLSHandshakeError,
81+
:RetryEvent, :RetrySkippedEvent, :SSEEvent, :SSEStream, :Server, :StatusError, :Stream,
82+
:TLSHandshakeError, :TLSTransportError,
8283
:TimeoutError, :TooManyRedirectsError, :Transport, :addtrailer, :appendheader,
8384
:body_close!, :body_closed, :body_read!, :cancel!, :canceled, :canonical_header_key,
8485
:close_idle_connections!, :defaultheader!, :delete, :do!, :expired, :fileserver,

src/http2_client.jl

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -471,8 +471,14 @@ end
471471

472472
@inline function _throw_stream_error(conn::H2Connection, state::H2StreamState)::Nothing
473473
err = state.stream_error
474-
err === nothing || throw(err::Exception)
475-
state.conn_errored && throw(_stream_conn_error(conn))
474+
if err !== nothing
475+
wrapped = _wrap_tls_transport_error(err::Exception)
476+
throw(wrapped)
477+
end
478+
if state.conn_errored
479+
wrapped = _wrap_tls_transport_error(_stream_conn_error(conn))
480+
throw(wrapped)
481+
end
476482
return nothing
477483
end
478484

@@ -1213,10 +1219,17 @@ function _connect_h2_from_tcp!(
12131219
stream_reader = nothing
12141220
connect_deadline_ns == 0 || TCP.set_deadline!(tcp, connect_deadline_ns)
12151221
if secure
1216-
cfg = _make_tls_config_for_h2(tls_config, address)
1217-
tls_conn = TLS.client(tcp, cfg)
1218-
connect_deadline_ns == 0 || TLS.set_deadline!(tls_conn, connect_deadline_ns)
1219-
TLS.handshake!(tls_conn)
1222+
try
1223+
cfg = _make_tls_config_for_h2(tls_config, address)
1224+
tls_conn = TLS.client(tcp, cfg)
1225+
connect_deadline_ns == 0 || TLS.set_deadline!(tls_conn, connect_deadline_ns)
1226+
TLS.handshake!(tls_conn)
1227+
catch err
1228+
# TLS.client can fail while it initializes client state, before
1229+
# handshake! starts. Both operations are connection setup.
1230+
err isa TLS.TLSError && throw(TLSHandshakeError(err::TLS.TLSError))
1231+
rethrow()
1232+
end
12201233
stream_reader = _ConnReader(tls_conn::TLS.Conn)
12211234
else
12221235
stream_reader = _ConnReader(tcp)
@@ -1653,7 +1666,7 @@ function body_read!(body::H2Body, dst::Vector{UInt8})::Int
16531666
@atomic :release body.closed = true
16541667
_clear_h2_cancel_callback!(body)
16551668
_unregister_stream!(body.conn, body.stream_id)
1656-
throw(terminal_error::Exception)
1669+
throw(_wrap_tls_transport_error(terminal_error::Exception))
16571670
end
16581671
if too_many
16591672
@atomic :release body.closed = true
@@ -1664,7 +1677,12 @@ function body_read!(body::H2Body, dst::Vector{UInt8})::Int
16641677
end
16651678
if nread > 0
16661679
body.bytes_read += Int64(nread)
1667-
_send_window_updates!(body.conn, body.stream_id, nread)
1680+
try
1681+
_send_window_updates!(body.conn, body.stream_id, nread)
1682+
catch err
1683+
wrapped = err isa Exception ? _wrap_tls_transport_error(err::Exception) : err
1684+
wrapped === err ? rethrow() : throw(wrapped)
1685+
end
16681686
return nread
16691687
end
16701688
if done
@@ -1884,5 +1902,10 @@ Send `request` over an existing `H2Connection` and return the streaming
18841902
`Response`.
18851903
"""
18861904
function h2_roundtrip!(conn::H2Connection, request::Request)::Response
1887-
return _streaming_response(_h2_roundtrip_incoming!(conn, request))
1905+
try
1906+
return _streaming_response(_h2_roundtrip_incoming!(conn, request))
1907+
catch err
1908+
wrapped = err isa Exception ? _wrap_tls_transport_error(err::Exception) : err
1909+
wrapped === err ? rethrow() : throw(wrapped)
1910+
end
18881911
end

0 commit comments

Comments
 (0)