diff --git a/prometheus/promhttp/delegator.go b/prometheus/promhttp/delegator.go index d1354b101..5070e72e2 100644 --- a/prometheus/promhttp/delegator.go +++ b/prometheus/promhttp/delegator.go @@ -53,12 +53,16 @@ func (r *responseWriterDelegator) Written() int64 { } func (r *responseWriterDelegator) WriteHeader(code int) { + if r.observeWriteHeader != nil && !r.wroteHeader { + // Only call observeWriteHeader for the 1st time. It's a bug if + // WriteHeader is called more than once, but we want to protect + // against it here. Note that we still delegate the WriteHeader + // to the original ResponseWriter to not mask the bug from it. + r.observeWriteHeader(code) + } r.status = code r.wroteHeader = true r.ResponseWriter.WriteHeader(code) - if r.observeWriteHeader != nil { - r.observeWriteHeader(code) - } } func (r *responseWriterDelegator) Write(b []byte) (int, error) { diff --git a/prometheus/promhttp/http.go b/prometheus/promhttp/http.go index b0ee4678e..5e1c4546c 100644 --- a/prometheus/promhttp/http.go +++ b/prometheus/promhttp/http.go @@ -167,15 +167,12 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { enc := expfmt.NewEncoder(w, contentType) - var lastErr error - // handleError handles the error according to opts.ErrorHandling // and returns true if we have to abort after the handling. handleError := func(err error) bool { if err == nil { return false } - lastErr = err if opts.ErrorLog != nil { opts.ErrorLog.Println("error encoding and sending metric family:", err) } @@ -184,7 +181,10 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { case PanicOnError: panic(err) case HTTPErrorOnError: - httpError(rsp, err) + // We cannot really send an HTTP error at this + // point because we most likely have written + // something to rsp already. But at least we can + // stop sending. return true } // Do nothing in all other cases, including ContinueOnError. @@ -202,10 +202,6 @@ func HandlerFor(reg prometheus.Gatherer, opts HandlerOpts) http.Handler { return } } - - if lastErr != nil { - httpError(rsp, lastErr) - } }) if opts.Timeout <= 0 { @@ -276,7 +272,12 @@ type HandlerErrorHandling int // errors are encountered. const ( // Serve an HTTP status code 500 upon the first error - // encountered. Report the error message in the body. + // encountered. Report the error message in the body. Note that HTTP + // errors cannot be served anymore once the beginning of a regular + // payload has been sent. Thus, in the (unlikely) case that encoding the + // payload into the negotiated wire format fails, serving the response + // will simply be aborted. Set an ErrorLog in HandlerOpts to detect + // those errors. HTTPErrorOnError HandlerErrorHandling = iota // Ignore errors and try to serve as many metrics as possible. However, // if no metrics can be served, serve an HTTP status code 500 and the @@ -365,11 +366,9 @@ func gzipAccepted(header http.Header) bool { } // httpError removes any content-encoding header and then calls http.Error with -// the provided error and http.StatusInternalServerErrer. Error contents is -// supposed to be uncompressed plain text. However, same as with a plain -// http.Error, any header settings will be void if the header has already been -// sent. The error message will still be written to the writer, but it will -// probably be of limited use. +// the provided error and http.StatusInternalServerError. Error contents is +// supposed to be uncompressed plain text. Same as with a plain http.Error, this +// must not be called if the header or any payload has already been sent. func httpError(rsp http.ResponseWriter, err error) { rsp.Header().Del(contentEncodingHeader) http.Error(