fix: superfluous response.WriteHeader call #724
Conversation
This makes WriteHeader do nothing if wroteHeader us already true. It is a more robust way of avoiding superfluous response.WriteHeader call. Old version fixed redundant calls made by client_golang, this one closes the gap on calls made by third party code. It also makes delegator code simpler. License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
| @@ -53,6 +53,10 @@ func (r *responseWriterDelegator) Written() int64 { | |||
| } | |||
|
|
|||
| func (r *responseWriterDelegator) WriteHeader(code int) { | |||
| // Avoid superfluous response.WriteHeader call | |||
| @@ -86,9 +88,7 @@ func (d closeNotifierDelegator) CloseNotify() <-chan bool { | |||
| func (d flusherDelegator) Flush() { | |||
| // If applicable, call WriteHeader here so that observeWriteHeader is | |||
There was a problem hiding this comment.
The "If applicable" is now a bit confusing (as there is no if statement anymore). How about changing this and the other comments to "Call WriteHeader here so that observeWriteHeader is handled appropriately if still needed."
|
Thinking about it, perhaps "more robust" is actually not the right way here. I mean, the console message is not a problem on its own. It is there to expose the bug that Perhaps we should hear opinions from people experienced with Go HTTP stuff. |
|
Current state of my thinking process: I tend towards the delegator not changing anything it is not in charge of, i.e. it should not mask/fix superfluous calls of |
|
Breaking news: Now we have pretty solid evidence for the root cause, see #672. It's |
|
@beorn7 cool, I see you mentioned working on a fix – should we close this PR then? |
|
Yes, I'll close this now. I mostly kept it open to remind myself of #724 (comment) . I'll work that into my upcoming fix. Thanks everyone. |
prometheus/client_golang v1.0.0 produces superfluous response.WriteHeader call. Because the unnecessary call was fixed with v1.5.1, updating the version makes this exporter's logs cleaner. See prometheus/client_golang#724 for more details.
This makes
WriteHeaderdo nothing ifr.wroteHeaderis already true.It is a more robust way of avoiding
superfluous response.WriteHeader callerror in the console. Old version fixed redundant calls made by client_golang, this one closes the gap on calls made by third party code, removing error from stdout:It also makes delegator code simpler and less error prone during refactors.