refactor(httpx): one bounded drain idiom for every response body

[Smana]

Summary

The repo carried three spellings of what happens to a response body once the status has been checked:

spelling	sites	verdict
defer func() { _ = resp.Body.Close() }()	25	leaves the body unread on every early return
defer func() { _, _ = io.Copy(io.Discard, resp.Body); _ = resp.Body.Close() }()	2	hazard — unbounded
bounded copy, then close	1	correct, but a one-off

httpx.Drain consumes up to 32 KB of whatever is unread. Callers pair it with the Close it deliberately does not perform:

defer func() { httpx.Drain(resp.Body); _ = resp.Body.Close() }()

The unbounded pair is the part that actually mattered: a runaway upstream holds the caller's entire timeout open inside what reads like cleanup.

Linked issue

n/a — found reviewing #559.

Why the ugly shape, and not DrainClose(resp)

DrainClose(resp) reads better and was written first. It is not used, because hiding the Close inside a helper blinds the bodyclose linter — it then reports "response body must be closed" at every converted site. bodyclose catches a leak that is invisible in tests and expensive in production; this drain is worth far less than that, so the drain gives way.

Worth recording how nearly that went unnoticed. golangci-lint's max-same-issues defaults to 3, so 25 newly-broken sites surfaced as 3 findings — a different 3 each run. That reads exactly like the stale-cache phantom findings this repo has hit before, and the instinct is to blame the cache. The tell was that converting a single file moved the reported set.

On the value of the drain, stated smaller than it started

This began as a finding on #559 phrased as "every POST redoes DNS + TCP + TLS". Measured, that is not true. net/http has often already buffered a small response by the time Close runs, so the connection is pooled either way; past the cap the body never reaches EOF, so it is dropped either way. In between is a band where draining rescues a connection — and where that band falls moves with header size and the read-buffer boundary.

The claim was corrected in #559 before it merged.

There are no connection-reuse tests, deliberately

Three were written, and all three were unreliable:

1. a table of body size → dial count — flipped between runs;
2. the weaker "draining is never worse than a bare Close" — failed at 64 KB on a rerun;
3. a single-size reuse assertion — same instability.

Dial counts are not a deterministic function of body size, so an assertion on them is a flaky test wearing a proof. A flaky test pinning a claim is worse than no test: it gets muted, and the claim ends up both unguarded and believed. So the rationale lives in Drain's doc comment, sized honestly, and what is deterministic — the bound, and nil-tolerance — is what gets asserted. If the reuse effect is ever worth pinning, it wants a benchmark.

The test file says this in place of the tests, so the next person does not read the gap as an oversight and "fix" it.

How was it verified?

go build ./...      ✓
go vet ./...        ✓
go test ./...       ✓
gofmt -l .          ✓ (prints nothing)
hack/lint.sh        ✓ 0 issues   ← including bodyclose, which is the point
go test -race ./internal/...  ✓

bodyclose reporting 0 is the load-bearing gate result here: it is what says the shared helper did not cost the leak check.

Checklist

• go build ./... passes
• go vet ./... passes
• go test ./... passes (-race across ./internal/...)
• gofmt -l . prints nothing
• golangci-lint run ./... reports 0 issues
• Test added first — the bound and nil-tolerance; the reuse tests were written, found flaky, and deliberately removed
• Docs updated — n/a, no behaviour or config change
• Respects read-only-first: no new cluster writes
• No config key changes meaning — no ! needed