loggingResponseWriter has no Unwrap, so http.ResponseController cannot reach the real writer through the shipped chain #191
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/webhooker#191
Reference in New Issue
Block a user
Found while implementing #187. Pre-existing, not caused by that work. NOT milestoned 1.0.0 — nothing in the tree flushes, hijacks or sets a write deadline today, so it is latent rather than live.
loggingResponseWriterininternal/middlewarewrapshttp.ResponseWriterand implements noUnwrap() http.ResponseWriter. Since Go 1.20,http.ResponseControllerwalks the chain viaUnwrapto reach the underlying writer's optional interfaces —Flush,Hijack,SetReadDeadline,SetWriteDeadline. A wrapper withoutUnwrapterminates that walk, soResponseControllermethods returnhttp.ErrNotSupportedfor any handler behind this middleware, and the type-assertion route (w.(http.Flusher)) fails the same way.Why it is worth fixing despite being latent: it fails silently and at a distance. The first handler that needs to stream — SSE for live event tailing, a chunked export, anything long-poll — will not fail at the wrapper. It will fail at the handler with a
nilorErrNotSupported, and the cause will be a middleware nobody was looking at. The fix is one method; the debugging session it prevents is not.The recoverer added by #189 does implement
Unwrapon its own writer wrapper, so it is not the gap — but that also means the chain is inconsistent, which is worse than uniformly wrong.Definition of done
loggingResponseWriterimplementsUnwrap() http.ResponseWriter.http.ResponseWriterwrapper in the repo is audited for the same gap, not just this one — enumerate what you found, including the ones that already comply. A partial sweep here just relocates the trap.http.ResponseController.Flush(or an equivalent optional interface) through the production middleware chain, not a hand-assembled one — the point is that the shipped chain works, and a hand-built stack is exactly how this was missed.Unwrapmust fail that test.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make bootstrapin a fresh clone before gating — browser assets are fetched at build time, andmake lintneeds Docker.make checkplus the cache-defeated Docker lint path. All linting runs in Docker, never on the host. Clean up every container and image you start; never run any prune.clawbot referenced this issue2026-08-18 10:50:50 +02:00
Plan. The issue body is the brief.
loggingResponseWritergainsUnwrap() http.ResponseWriter; everyhttp.ResponseWriterwrapper in the repo is audited and listed in the PR, compliant ones included (the recoverers from #382 among them); a test driveshttp.ResponseController.Flushthrough the production router and fails withUnwrapremoved. One line of the issue is out of date: browser assets are no longer fetched at build time (#345), somake checkneeds no bootstrap step.Model: opus-5-5
Built in #428. The access log's response writer now has
Unwrap, and the Metrics middleware is registered outside Logging, because go-http-metrics' writer only passes a flush on when the writer inside it has aFlushmethod, so with metrics on a flush had silently done nothing. A new test flushes through the production router in both configurations, on a global route and in an admin page route group. The audit of every response writer wrapper is in the PR.Judgement call: the Metrics reorder goes past the issue's literal list; it is what makes the shipped chain flush with metrics on.
Remaining gap, read from the library source and not tested: with metrics on, setting a deadline through
http.ResponseControllerstill fails, because go-http-metrics' writer has noUnwrap.Model: opus-5-5