Skip to content

flux-network: harden HTTP failures and remove hot-path allocations - #144

Closed
Bronek wants to merge 15 commits into
bronek/api_server-a5from
bronek/api_server-b
Closed

flux-network: harden HTTP failures and remove hot-path allocations#144
Bronek wants to merge 15 commits into
bronek/api_server-a5from
bronek/api_server-b

Conversation

@Bronek

@Bronek Bronek commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

This PR builds on #142. It adds lingering close for rejected requests, outbound request deadlines with explicit failure reasons, support for the full HTTP status range, and allocation-free processing on warm connections.

Lingering close

A response that ends an incomplete request stream shuts only the write side once the answer drains, then reads and discards the peer's remaining upload. This lets the peer receive the HTTP error and EOF instead of a connection reset.

HttpConfig::linger: Option<Linger> bounds this phase by idle time since the last inbound byte (5 seconds by default) and total elapsed time (30 seconds by default). The clocks start only after the transport has taken the complete answer; until then, the ordinary idle timeout still bounds a peer that stops reading. without_linger() restores immediate close after the response drains.

If a connection exceeds its buffer limit while a request is pending, the service preserves the application's response, adds Connection: close, and then lingers. If no request is pending, it responds with 431 and lingers. A complete request answered with Connection: close continues to drain and close normally; lingering is reserved for lost request framing.

Request deadlines and failure reasons

HttpConfig::request_timeout: Option<Duration> gives each outbound request a deadline starting when its bytes are queued. It defaults to None.

An outbound request that can no longer be answered produces HttpEvent::RequestFailed { token, reason } before the corresponding Disconnected event:

pub enum RequestFailure {
    Timeout,
    Malformed,
    TooLarge,
    Disconnected,
}

TooLarge distinguishes operator-configured limits from malformed responses, while Disconnected distinguishes a lost in-flight request from an idle endpoint closing.

Status framing

Responses may use any status from 100 through 599. Common statuses use their conventional reason phrases; other statuses use an empty phrase, as RFC 9112 permits. A caller-chosen 1xx is final: it completes the request without a content length or body.

Invalid responses—such as an out-of-range status, invalid header, or response without a pending request—are refused before the body closure runs.

Allocation discipline

After connection warm-up, HTTP request serving and outbound response parsing allocate nothing. Parsing uses a fixed stack header scratch, an in-flight request records only whether it was HEAD, and respond_with renders into a service-owned reusable buffer.

The allocation test warms both directions, runs 1,000 round trips each way under a thread-local counting allocator, and requires zero allocation events.

HttpConfig::max_headers must be in 1..=MAX_HEADERS (128). HttpService::new validates direct struct construction under the same rule as with_max_headers, so both configuration paths behave identically.

Verification

The new test suites cover lingering over TCP and Unix-domain sockets, cap timing, over-limit pending responses, all request-failure reasons and their event ordering, status boundaries, and allocation-free warm paths. Additional HTTP and in-module tests pin header-count boundaries, rejected response composition, incomplete response heads, and internal discard behaviour without exposing representation through the public API.

Workspace tests, Clippy with warnings denied, the nightly formatting check, and cargo doc pass without new warnings.

The existing loopback tests still select unused TCP ports before binding them, so they retain a known race. A following change makes listen return its bound endpoint and converts the tests to bind port zero; until then, a job that loses this race can be rerun.

Scope

This PR does not change listen, which still returns io::Result<()>. Reporting the bound endpoint, binding tests directly to ephemeral ports, and avoiding the scratch copy in the slice-based respond path remain separate follow-up changes.

Base: #142.

Refer: #143

Assisted-by: Claude Code:claude-fable-5
Assisted-by: Codex:gpt-5

A client that meets a reset before it has read the answer to its upload
reports a node that is down rather than the status it was sent. A
response that closes a connection now shuts the write side alone
whenever the request stream behind it was cut short — every status the
service raises itself, and a connection driven over its buffer limit —
and reads its peer out under an idle and a total cap until the peer
stops, the caps run out, or it closes.

The bytes over the limit are no verdict on a request already delivered:
a connection holding one keeps it, and the answer the caller is
producing carries the close. Lingering connections answer to the
linger's caps rather than the idle sweep, and hold their place in the
group meanwhile.

Assisted-by: Claude Code:claude-opus-5
A request to an endpoint that answers nothing used to wait as long as
the connection stood, and a peer that broke the protocol dropped the
connection with nothing to say for it. An outbound request now carries a
deadline from the moment its bytes are queued — a request waiting behind
a backlog is already waiting — and every request that will not be
answered names its reason: the deadline, an answer the service cannot
frame, an answer over the caps the operator sets, or an endpoint that
closed with the request in flight.

The failure reaches the caller before the disconnect it causes, so a
puller learns why the connection went before learning that it did.

Assisted-by: Claude Code:claude-opus-5
An endpoint that echoes a status chosen elsewhere needs the whole range
and the phrase-free status line RFC 9112 §4.1 permits, so a status the
service has no phrase for is framed as `HTTP/1.1 250 ` rather than
carrying a phrase nobody chose. A caller-chosen 1xx is final: it
completes the request, framed with no length and no body.

`reason_phrase` is public, and the phrase it returns for a status it
does not name is now the empty string rather than `Unknown`.

Assisted-by: Claude Code:claude-opus-5
The caps were timing the wrong thing. They ran from the moment the
answer was queued, so a large answer over a small socket was cut off
mid-body — the idle cap ending a connection that had delivered a
fraction of the length it announced, which is the reset lingering exists
to avoid. They now time what they name: the reading and discarding that
begins where the answer ends. A connection still writing is bounded by
its transport, exactly as a draining one is.

This is the one network-side addition PR B needs: `write_side_shut`
reports that the peer has the whole of what was queued and the end of
the stream after it, which is where the clock starts. A linger whose
clock has yet to start folds no deadline of its own, since the write it
is waiting on is what wakes the poll.

A connection over its buffer limit with nothing pending is now answered
with the 431 every other head too large for the service gets, rather than
dropped where it stands: an over-limit that lands exactly on a request
boundary closes the way one that lands mid-request already did, however
the request before it was answered.

Assisted-by: Claude Code:claude-opus-5
The reason an outbound request carries is what tells an operator to
raise a cap apart from what tells them an endpoint is broken, so each
path that picks one is pinned: a head over its cap, a chunked body over
its cap and one the close delimits over its cap all report `TooLarge`,
while a chunk size the service cannot read and an answer framed twice
over report `Malformed`. Each asserts the failure reaches the caller
ahead of the disconnect that follows it.

A request deadline also has to be the deadline a blocking drive waits
for, which is what the last test holds it to.

Assisted-by: Claude Code:claude-opus-5
The caps time the reading and discarding, so a connection whose answer
has yet to reach its peer runs under none of them. A peer that stops
reading held such a connection open — and its place in the group —
against nothing but the transport, which bounds a TCP peer late and a
Unix-domain one never.

The sweep now lets go of a lingering connection only once its own clock
has taken over. Until then it is held to the same idle bound as any
other, a draining connection included, and folds that bound into the
deadline rather than none.

Assisted-by: Claude Code:claude-opus-5
Every parse borrowed a fresh header vector, every outbound request owned
a copy of its method and of the host it addressed, and every response
carried its body through the caller's slice. A warm connection needs
none of it: the scratch a parse borrows is bounded, all that is asked of
a request in flight is whether it was HEAD, and a body composed into a
buffer the service keeps reaches the wire in one copy.

Parsing borrows a stack array of MAX_HEADERS entries sliced to what the
configuration asks for, and with_max_headers refuses a larger cap. An
outbound connection records the request in flight rather than its
method. request() writes the Host it defaults to straight into the send
buffer. respond_with lends a closure the service's body scratch and
frames the Content-Length it wrote, HEAD included; respond is that call
with the body it was handed. Framing stays in one place, now the
Responder the request carries, and the wire bytes are unchanged.

Assisted-by: Claude Code:claude-opus-5
A warm keep-alive connection in each direction runs a thousand round
trips under a global allocator that counts allocation events on the
thread driving them, and each direction is asserted to have made none.
The peers are plain sockets driven from the same thread between
iterations of the network, so nothing else runs while a count is open
and nothing the peers do grows a buffer of their own. The bytes each
side reads are checked against the message it expects, so a direction
cannot pass by falling silent.

flux-profiler's CountingAllocator tallies bytes rather than events and
keeps its counters to the profiler, so the test brings its own.

Assisted-by: Claude Code:claude-opus-5
A lingering connection arms its caps once the transport has taken the
whole answer, and finding that out walks the network's connections. The
linger loop asked for every lingering connection on every tick; it now
asks only for one whose clock has yet to start, so a connection whose
caps are already running costs nothing further.

`write_side_shut` says what it means: the transport has taken everything
queued for the connection and the end of the stream after it, with up to
a socket buffer of that still on its way, not that the peer has read it.
The test pins that the caps run from the moment the answer has left,
which is the time the ticks are given, rather than from the start of the
poll wait that preceded it.

Assisted-by: Claude Code:claude-opus-5
A head that never ends is caught by the cap while the parse is still
incomplete, which is a path of its own: an answer whose open head runs
past the cap now pins `TooLarge` there, where before only the head that
parsed whole was held to it.

A request ending exactly at the buffer limit has two correct closes, and
which one the peer reads is settled by whether a tick falls between the
request and its answer. The pair that answer first are named for that,
and the third holds the other outcome: an answer that carries the close
itself, with no rejection behind it.

Assisted-by: Claude Code:claude-opus-5
A response the service refuses never reaches its body: the status range,
the header names and the pending request are all settled before the
closure that would compose one runs. Nothing said so, and nothing would
have noticed the check moving below the render.

The header count had the same gap on the other side. Its three tests
covered the builder alone, so what the count decides on the wire --- a
request at the count served, one over it answered 400, and a count
written past the scratch held to what the scratch holds --- went
unpinned, as did the answer an endpoint sending too many headers gets.

Both new server-side tests run over a socket file: what a header count
decides is the parse, and the loopback ports the rest of the suite
shares are contended enough already. The zero-alloc harness takes the
connection its service dialled straight from the listener rather than
opening a placeholder first, and records what one round trip already
settles.

Assisted-by: Claude Code:claude-opus-5
`HttpConfig`'s fields are public, so a configuration is as likely
written whole as built call by call. The builder refuses a header count
outside `1..=MAX_HEADERS`; a field written straight into the struct was
quietly held to `MAX_HEADERS` at every parse instead, so the same
configuration meant two different things.

`HttpService::new` validates the configuration it is handed, sharing the
check with the builder so the rule and its message cannot drift, and the
parse borrows the scratch the count names.

Assisted-by: Claude Code:claude-opus-5
`write_side_shut` answers a question the lingering close asks of its own
transport, and nothing outside the crate has cause to ask it. It is
`pub(crate)`, and the linger test that reached for it measures the same
instant from where a peer can see it: the client reads the whole answer
and then the end of the stream, which is the answer having left.

The test keeps its point by timing that instant against both ends. The
peer takes nothing for half a second, so the answer leaves only after
the pause -- and the caps end the connection an idle cap after that,
rather than an idle cap after it was queued.

Assisted-by: Claude Code:claude-opus-5
`buffered` handed the tests a connection's byte count, which is the
service's own representation and nothing a caller has business reading.
It goes, and what it stood in for is measured where it belongs.

The lingering close's discard is a state transition, so it is pinned by
a unit test that drives a connection through it: bytes read while
lingering leave the buffer empty, leave the total cap where the answer
put it, and move the idle cap to the read. The wire-level test keeps
what a peer can see -- the answer, then the end of the stream, and a
client that goes on uploading uncut.

The gates that make the over-limit-while-pending path deterministic ask
the network instead. An accepted connection is polled for reads alone
until the service writes, and a raw read empties the socket, so between
a request and its answer the one iteration reporting work is the one
that took the overrun whole.

Assisted-by: Claude Code:claude-opus-5
@Bronek
Bronek force-pushed the bronek/api_server-b branch from f9db1f1 to 755f23d Compare September 3, 2026 13:53
@Bronek Bronek closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant