When a handler sets a cookie and then panics before sending anything, the recover middleware (internal/middleware/recoverer.go) now deletes Set-Cookie before writing its 500, so a request that failed never hands the client a credential. Every other header, Location included, is left as http.Error leaves it, which is what chi's Recoverer does. A response that was already sent before the panic is untouched: nothing can be taken back once it is on the wire, and there is still no second WriteHeader.
The doc comment on Recoverer records this in one sentence, so the difference from http.Error is not taken for a bug.
Tests:
a handler that sets Set-Cookie and Location and panics uncommitted gets a 500 with no cookie and its Location kept; removing the deletion fails it;
the existing committed-response test now also sets a cookie and asserts it still reaches the client.
Worth knowing: the header map is shared along the whole middleware chain, so a cookie set by a middleware further out than the recoverer would be dropped too. None of the current outer middlewares (request id, security headers, access log, metrics, CORS, timeout) sets one.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/webhooker/issues/193, option 3 as decided in https://git.eeqj.de/sneak/webhooker/issues/193#issuecomment-106293.
When a handler sets a cookie and then panics before sending anything, the recover middleware (`internal/middleware/recoverer.go`) now deletes `Set-Cookie` before writing its 500, so a request that failed never hands the client a credential. Every other header, `Location` included, is left as `http.Error` leaves it, which is what chi's `Recoverer` does. A response that was already sent before the panic is untouched: nothing can be taken back once it is on the wire, and there is still no second `WriteHeader`.
The doc comment on `Recoverer` records this in one sentence, so the difference from `http.Error` is not taken for a bug.
Tests:
- a handler that sets `Set-Cookie` and `Location` and panics uncommitted gets a 500 with no cookie and its `Location` kept; removing the deletion fails it;
- the existing committed-response test now also sets a cookie and asserts it still reaches the client.
Worth knowing: the header map is shared along the whole middleware chain, so a cookie set by a middleware further out than the recoverer would be dropped too. None of the current outer middlewares (request id, security headers, access log, metrics, CORS, timeout) sets one.
Model: opus-5-5
When a handler sets a cookie and then panics before sending
anything, the recover middleware now deletes Set-Cookie before
writing its 500, so a request that failed never hands the client
a credential. Every other header, Location included, is left as
http.Error leaves it, matching chi's Recoverer. A response that
was already sent is untouched.
Tests cover the uncommitted case (no cookie, Location kept) and
assert the cookie still reaches the client when the response was
committed before the panic.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #193, option 3 as decided in #193 (comment).
When a handler sets a cookie and then panics before sending anything, the recover middleware (
internal/middleware/recoverer.go) now deletesSet-Cookiebefore writing its 500, so a request that failed never hands the client a credential. Every other header,Locationincluded, is left ashttp.Errorleaves it, which is what chi'sRecovererdoes. A response that was already sent before the panic is untouched: nothing can be taken back once it is on the wire, and there is still no secondWriteHeader.The doc comment on
Recovererrecords this in one sentence, so the difference fromhttp.Erroris not taken for a bug.Tests:
Set-CookieandLocationand panics uncommitted gets a 500 with no cookie and itsLocationkept; removing the deletion fails it;Worth knowing: the header map is shared along the whole middleware chain, so a cookie set by a middleware further out than the recoverer would be dropped too. None of the current outer middlewares (request id, security headers, access log, metrics, CORS, timeout) sets one.
Model: opus-5-5
Review passed.
Model: opus-5-5