httprequest
httprequest copied to clipboard
HTTP 200 status not being set for one handler signature pattern
In order to match the behavior of handler signature case 2: https://github.com/juju/httprequest/blob/77d36ac4b71a6095506c0617d5881846478558cb/handler.go#L418-L428
Wouldn't it make sense to also set the status to http.StatusOK
in case 1?
https://github.com/juju/httprequest/blob/77d36ac4b71a6095506c0617d5881846478558cb/handler.go#L413-L416
I know that ultimately it doesn't matter, as 200 would get set anyway, but I found this when logging via a middleware that was logging status as 0
at this point (but just for endpoints with return a single error value).
The expected use case for the single return value functions is that they will generally, or at least potentially, be writing directly to the ResponseWriter. In which case writing our own header is wrong, the go documentation for a ResponseWriter (https://golang.org/pkg/net/http/#ResponseWriter) states that "Only one header may be written".
To be able to write a header we would need to detect if the function wrote one first. This could be done by wrapping the given ResponseWriter in a new one which intercepted the WriteHeader call and logged whether it had been run. This however brings a new problem. ResponseWriters are often used to smuggle other interfaces (c.f. https://golang.org/pkg/net/http/#Hijacker) we would rather not elide those interfaces, but there is no reasonable way to try and preserve them only when they are present.
Also it means we don't have to have a robust discussion about whether to use "200 OK" or "204 No Content".
The problem/solution you described involving RequestWriter
and header handling is the same as used by Go Chi: https://github.com/go-chi/chi/blob/master/middleware/wrap_writer.go
Indeed, this can be a bit awkward to justify, especially considering the 200/204 distinction...