Files
crossbar/docs/plans/v2.1/01-cancel-record.md

4.5 KiB

v2.1 task 01: a delivered response is never recorded as cancelled

Branch: v2.1 (run git switch -c v2.1 master if it does not exist, else git switch v2.1; git status --short must be empty, otherwise stop) Commit subject: Record cancellation from what the reverse proxy observed, not the request context

Goal

The accounting row for a forwarded request takes its status from what the reverse proxy did. A response that was delivered in full is recorded with the status the upstream returned, even when the client closes its connection the instant the body ends. Status 499 ("client cancelled") is recorded in exactly two cases: the reverse proxy's transport failed with a context error before any response byte was written, or the client left mid-body (the http.ErrAbortHandler panic the recover path already handles).

Context

v1's forward.go writes the row after rp.ServeHTTP returns and, if r.Context().Err() is non-nil at that moment, turns the row into a 499 error. The server cancels a request's context when the client's connection closes, and a pooled client closes a connection as soon as it has read a response whenever its idle pool is full. So a served 200 becomes a recorded 499 whenever that close lands before the row is written. Measured on 2026-09-25: about a third of delivered responses under the given test's load; TestQueueFullIs503 flaked on it. The reverse proxy's ErrorHandler already sees context.Canceled for the "client gone before the response" case and currently returns without leaving a trace, which is why the post-hoc check was there.

Facts about httputil.ReverseProxy (Go 1.26) — you cannot read its source from here

The standard library lives outside the repository and the sandbox refuses reads there; do not try. What you need:

  • ServeHTTP calls ErrorHandler(w, req, err) when the outgoing request fails before any response byte was written — for a client that left, err satisfies errors.Is(err, context.Canceled). After the response headers were written, ErrorHandler is never called.
  • If copying the response body to the client fails (the client left mid-body), ServeHTTP panics with http.ErrAbortHandler; v1.1's deferred recover in forward.go already turns that into the 499 row and re-panics.
  • ServeHTTP returning normally therefore means the response was delivered in full (or ErrorHandler answered). The request's context may nonetheless already be cancelled at that moment — the server cancels it when the client's connection closes — which is exactly the signal the current code misreads.

Files

  • Copy: internal/proxy/served_test.go
  • Modify: internal/proxy/forward.go, docs/implementer-log.md

Rules the tests check

  • TestServedResponseIsNeverRecordedCancelled (given): 32 concurrent requests on one host (parallel = 8, queue_max = 64), each on its own connection that closes after the response is read; every response is 200; the usage row has 32 requests and 0 errors; the status counts hold only status 200.
  • TestClientCancelMidStreamIsRecorded and TestClientCancelWhileQueuedIsRecorded (v1, in the tree) still pass: mid-stream and while-queued cancellations are still 499 rows with a non-empty err.
  • TestQueueFullIs503 (v1, in the tree) still passes: 3 requests, 1 error.

Rule for the implementation: the ErrorHandler records that it observed a cancellation (a field on forwardState is the natural place) and the row is 499 when that field is set or the recover path saw http.ErrAbortHandler. The check of r.Context().Err() after the forward is removed. Nothing else in the row changes. forward.go stays under 400 lines.

Steps

  • 1. Branch as above; copy the given test.
  • 2. See it fail: go test -race -count=3 -run 'TestServedResponseIsNeverRecordedCancelled$' ./internal/proxy/ fails every run with want 0 errors.
  • 3. Change forward.go per the rule. gofmt -w internal/proxy/.
  • 4. go test -race -count=3 ./internal/proxy/ → ok three times. 5. go test -race -count=1 ./... → all ok.
  • 6. make gate. 7. Row v2.1/01-cancel-record; commit.
git add internal/proxy docs/implementer-log.md
git commit

Done when

  • The given test passes three times in a row under -race; the two v1 cancel tests and TestQueueFullIs503 pass; gate ok; the given file byte-identical.

Stop and report if

  • The given test still fails after the post-hoc check is gone: quote the status counts.
  • Making the given test pass requires editing any _test.go file.