69 lines
3.5 KiB
Markdown
69 lines
3.5 KiB
Markdown
# 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.
|
|
|
|
## 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.
|
|
|
|
```sh
|
|
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.
|