Fix CloseNotifier handling and avoid "the ResponseWriter does not implement CloseNotifier" warnings in logs

A change in #763 to address review comments caused problems. Originally,
instrumentedResponseWriter implemented the CloseNotifier interface, and
would panic if it was wrapping something that did not implement that
interface. This was split into a separate instrumentedResponseWriterCN
type that implements CloseNotifier, so there's a fallback if
instrumentedResponseWriter ever needs to wrap something that does not
implement this interface.

instrumentedResponseWriter's Value method would end up upcasting either
type back to instrumentedResponseWriter, which does not implement the
interface. In effect, instrumentedResponseWriterCN was never visible to
the handler.

This fixes the problem by implementing a wrapper Value method for
instrumentedResponseWriterCN.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
This commit is contained in:
Aaron Lehmann 2015-08-18 15:40:14 -07:00
parent ed7cc91e6f
commit 0e7462f1dd
2 changed files with 11 additions and 1 deletions

View File

@ -352,3 +352,13 @@ func (irw *instrumentedResponseWriter) Value(key interface{}) interface{} {
fallback:
return irw.Context.Value(key)
}
func (irw *instrumentedResponseWriterCN) Value(key interface{}) interface{} {
if keyStr, ok := key.(string); ok {
if keyStr == "http.response" {
return irw
}
}
return irw.instrumentedResponseWriter.Value(key)
}

View File

@ -29,7 +29,7 @@ func copyFullPayload(responseWriter http.ResponseWriter, r *http.Request, destWr
if notifier, ok := responseWriter.(http.CloseNotifier); ok {
clientClosed = notifier.CloseNotify()
} else {
ctxu.GetLogger(context).Warn("the ResponseWriter does not implement CloseNotifier")
ctxu.GetLogger(context).Warnf("the ResponseWriter does not implement CloseNotifier (type: %T)", responseWriter)
}
// Read in the data, if any.