Blame the client only when it left with bytes still owed to it - #1
Merged
Conversation
The check job ran pull request code with a writable token, no time limit, and actions on mutable tags. Its lint step ran no linter at all. .github/workflows/ci.yml: - Read-only token. - persist-credentials: false, so the token is not left in .git/config. - timeout-minutes on both jobs, instead of the 6 hour default. - Actions pinned to commit SHAs. - Force-pushing a PR cancels the superseded run. - Installs golangci-lint from a pinned, checksummed tarball. The runner image does not ship one. Makefile: make lint now fails when golangci-lint is missing and CI is set, instead of falling back to go vet. .golangci.yml: new. Removes the default output limits, which cap findings at 3 of a kind and 50 per linter and keep only one per line. .github/dependabot.yml: new. Keeps the pins current. 23 lint fixes, so the job passes. Mostly unchecked error returns from Close and Fprintf. Three others: tint.NewHandler is deprecated, replaced with tint.NewTextHandler; a deliberately discarded config.Load return now uses _; one switch is now tagged. Fork PRs are gated by a repository setting rather than this file. It is set to require approval for all outside collaborators. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FnZG8whBis7HMNVNAWFxDr
TestNonStreamingTruncatedBody failed about one full-suite run in four, blaming
the client for a body the vendor cut mid-JSON:
Side = client (kind=client_disconnect_ctx), want upstream
verdict: your LLM tool hung up first. The vendor did not interrupt anything.
postmortem demoted an upstream fault to a client one by reading
rec.ClientGone() — live, mutable state — after the copy loop had already
reached its verdict. The client watcher stays armed until ServeHTTP returns,
well past that point, so any client close landing in the window retroactively
rewrote the answer.
The client was doing nothing wrong. The upstream sends a well-formed HTTP
response whose body ends mid-JSON: 64 bytes, Content-Length satisfied. The
proxy forwards all 64. The client sees a complete response and closes, which is
what any client without keep-alives does on every successful request. Whether
the vendor got blamed for its own truncated body came down to whether that FIN
was processed before one line of Go ran. Real clients hit this, not just the
test harness.
AsInduced is a fallback, not the main path. A client that hangs up mid-stream is
already handled structurally: the proxy cancels the upstream read itself with a
stamped ErrClientGone, and fromCancellation reads that cause — which is why
TestClientDisconnectsWhileUpstreamIsSilent can assert the verdict is not
induced. So the demotion only needs the narrower causal question, and now asks
it as a single atomic read: was the client gone, *and* were bytes still owed to
it? responseWasComplete already compares BytesToClient against
BytesFromUpstream, so the comparison is not a new idea here. A client that
received every byte read from the upstream interrupted nothing, whenever its FIN
arrives. The ordering stops mattering rather than being won.
Verified by forcing the race with a temporary sleep before the check: the old
predicate fails 10/10, the new one passes 10/10, and the forced failure is
character-identical to the flake. 0 failures in 12 consecutive full-suite runs
under -race, against 1 in 4 before.
The second ClientGone() use in postmortem is deliberately left alone. That one
wants the late observation, with completeness deciding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016X5Bmd7YUcjf9ydMGMtWQg
peterhoneder
force-pushed
the
fix-truncation-blame
branch
from
September 1, 2026 11:27
3b1a6ec to
bfa5279
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestNonStreamingTruncatedBodyfailed about one run in four, blaming the client for a body the vendor cut short.postmortemasked whether the client was gone, after the copy loop had already decided. A client without keep-alives closes after every successful request, so the verdict depended on when its FIN arrived.It now asks whether the client was gone and bytes were still owed to it, in one read. Ordering stops mattering.
0 failures in 12 full-suite runs under
-race, against 1 in 4 before.🤖 Generated with Claude Code
https://claude.ai/code/session_01FnZG8whBis7HMNVNAWFxDr