Skip to content

🐛 fix(http): answer a failed request body as a client error - #2187

Open
gaborbernat wants to merge 4 commits into
mainfrom
fix/client-body-failure-status-2184
Open

🐛 fix(http): answer a failed request body as a client error#2187
gaborbernat wants to merge 4 commits into
mainfrom
fix/client-body-failure-status-2184

Conversation

@gaborbernat

Copy link
Copy Markdown
Member

A request body that failed answered 502 with "upstream transfer failed". Nothing upstream serves a request body: the bytes were coming from the client and peryx is the server. The stall bound #2182 added made it reachable with the client still connected, so a client that paused for thirty seconds was told something upstream of peryx had gone wrong. 502 also tells an intermediary that a backend gave a bad answer, which invites a retry against a different backend for a condition no backend change reaches. 🔍

The distinction lives in one place. BodyFailure sits in peryx-driver::body beside the Stalled error the edge produces, and both ecosystems ask it the same question instead of each reading an opaque body error. A handler cannot recover the distinction on its own: the stall arrives boxed and wrapped by whatever read the body, so the classification walks the source chain rather than inspecting the outermost error. Putting it anywhere else would have meant two answers to one question, which is the per-handler patchwork #2176 exists to avoid. It landed in peryx-driver rather than peryx-http because the registry takes peryx-http only as a dev-dependency, while both ecosystems already depend on the driver at runtime.

The two statuses are chosen for what they tell the client to do next. A stall answers 408, which says the server gave up waiting for a message that never arrived, and leaves the client free to repeat the request; a registry upload also gets the offset its session stands at, so it resumes there instead of starting over. Everything else answers 400, since a body the server could not read fails the same way when repeated unchanged. Merging the two under 400 fails in the more damaging direction. It tells a client that stalled that its request came out malformed, so the useful move, sending it again, looks pointless.

I updated the four registry tests that asserted the old status, and their intent survives unchanged. The two stall cases asserted that a cut chunk must not read as accepted, which 408 satisfies as well as 502 did, and the message that now accompanies them says which end went quiet. The monolithic case took its name from the gateway status it happened to produce rather than from any gateway being involved, so it now names a client error.

On what changes for clients that retry the old 502: peryx's own upstream client sees no change, because should_retry_status already retries both is_server_error() and REQUEST_TIMEOUT, so it retries a 408 exactly where it retried a 502. RFC 9110 gives 408 the same permission, since the request never completed and may be repeated. What an external pusher such as docker or containerd does with either status is not something this repository can answer, so a client that treated 502 as fatal and 408 as retryable, or the reverse, would see a change this change cannot verify.

Closes #2184

@gaborbernat gaborbernat added the bug Something isn't working label Sep 4, 2026
@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 29 untouched benchmarks
⏩ 133 skipped benchmarks1


Comparing fix/client-body-failure-status-2184 (4628b0b) with main (0df149b)

Open in CodSpeed

Footnotes

  1. 133 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@read-the-docs-community

read-the-docs-community Bot commented Sep 4, 2026

Copy link
Copy Markdown

Documentation build overview

📚 peryx | 🛠️ Build #34390002 | 📁 Comparing 4628b0b against latest (e64cc83)

  🔍 Preview build  

No files changed.

The bytes of a request body come from the client, so no failure reading one is
an upstream fault. A handler holding the body error cannot see which failure it
has: the stall arrives boxed and wrapped by whatever read the body, so the
distinction is only recoverable at the edge that created it.

BodyFailure makes it there once. Every handler asks the same question of the
same classifier rather than each deriving an answer from an opaque error, which
is how a per-handler patchwork starts.

The two cases differ in what the client should do next, which is why they are
two. A stall means the request never completed and may be repeated, and a
resumable session picks up at the offset its bytes reached. Anything else means
repeating it unchanged fails the same way.
An upload whose body failed answered 502 with "upstream transfer failed". No
upstream serves a request body: the bytes were coming from the client and peryx
is the server. The stall bound #2182 added made it reachable with the client
still connected, so a client that paused for thirty seconds was told something
upstream of peryx had gone wrong.

502 also carries a meaning to intermediaries, a bad response from an upstream
server, which invites a retry against a different backend for a condition no
backend change reaches.

A stall now answers 408, which says the server gave up waiting for a message
that never arrived, and names the offset a resumable session stands at so the
client knows where to continue. Anything else the body ends with answers 400,
since repeating it unchanged fails the same way. Both come from the shared
classification rather than from each call site reading an opaque error.

Four tests asserted the old status. Their intent survives unchanged: the stall
pair asserted a cut chunk must not read as accepted, which 408 satisfies, and
the monolithic case was named for the gateway status it happened to produce
rather than for a gateway being involved.
Every multipart read error answered 400. That is the right family, since the
bytes come from the client either way, but it merges two conditions whose
correct next move differs. A malformed form repeated unchanged fails again. A
stall says nothing about the form at all, so repeating the upload is exactly
what the client should do, and 400 told it the opposite.

A stalled body now answers 408 through the same classification the registry
uses, so the two ecosystems cannot drift on what a stalled request body means.
@gaborbernat
gaborbernat force-pushed the fix/client-body-failure-status-2184 branch from f7cba3e to 881c63c Compare September 4, 2026 09:33
Moving the body failures off the gateway status left the fault arm beside them
with no test: the tests that used to reach it now produce a client error
instead, so the arm stayed reachable while nothing exercised it. A store that
fails while a chunk lands is peryx failing rather than the client, and it keeps
the gateway status, which is the line this change draws.

The stall's own wording is what reaches a log when the error propagates rather
than being classified, so it is worth pinning too.
@gaborbernat
gaborbernat force-pushed the fix/client-body-failure-status-2184 branch from 881c63c to 4628b0b Compare September 4, 2026 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report a client-side body failure as a client error

1 participant