test(server): Assert upload cancellation on the server side

[korya]

Why

Master CI failed twice on TestBudgetCancellationOverHTTP/h2=false/upload=true with "canceled request succeeded", and the earlier patch to the same test did not stop it.

The assertion demanded that a client-canceled HTTP/1.1 upload always return an error. The stack does not promise that. The client's TLS close sends a close_notify alert before closing the socket; the upload handler reads that as end of body and, as the spec requires, replies 200 with the byte count; net/http deliberately returns a response that races cancellation. Commit 1456257 removed one route to a complete response (a clean EOF from the test's pipe) and left this one.

sequenceDiagram
    participant C as "client"
    participant S as "server"
    C->>S: "POST /nq/upload (chunked), chunk x"
    Note over S: "handler blocks on the next chunk"
    Note over C: "ctx canceled → tls.Conn.Close"
    C->>S: "TLS close_notify"
    Note over S: "body read → unexpected EOF, ignored"
    S->>C: "200 OK, body 1"
    C->>S: "TCP close"
    Note over C: "roundTrip: response ready vs ctx done → uses the response"

Loading

Only this subtest is exposed: downloads already hold the 200 and see a short body on cancel, and HTTP/2 cancels the stream with RST_STREAM.

What & how

Assert cancellation on the server: the aborted upload's body read must end with an error, and stop asserting how the client's request settled.

Assertion	Before	After
Client Do on the aborted upload	must return an error (racy)	awaited only
Server body read for the aborted upload	not observed	must end with a non-EOF error
Slot released, other transfer undisturbed, next request admitted	asserted	unchanged

The server-side check holds in every interleaving because the test closes the upload producer with an error, and the transport writes the terminating chunk only when the body copy succeeds. A mutation check confirms it: switching the abort to a clean pw.Close() fails with "aborted upload ended with EOF, want a read error".

Production code is left alone. Replying 4xx to a truncated upload would remove the raceable response at the source, but real clients abort uploads at every phase end and never read that reply, so it would trade a test race for client-side error noise. That is a product decision, not part of this fix.

The failure could not be reproduced on demand locally (about 4,000 runs on macOS, with and without the race detector). The mechanism was verified step by step: with the window between close_notify and the TCP close widened by 20 ms, the client transport was observed reading the 200 before its own socket closed, and one probe run returned the 200 with a nil error while the server logged the body read as unexpected EOF.

Other changes

• docs/guidelines.md: new quality-bar bullet stating that cancellation is asserted through server-side effects, since net/http prefers a delivered response over a cancel.

Related:

• fix(server): Bound concurrent client budget admissions #47
• https://github.com/korya/netquality/actions/runs/34562478384
• https://github.com/korya/netquality/actions/runs/34564265268

🤖 Generated with Claude Code

https://claude.ai/code/session_01B4xBvbUJ5Ci1b99X5x8LJL