From 6c5ca6236c30d2102b0de2ad398bac0350f8b63a Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Wed, 17 Apr 2019 11:49:36 -0600 Subject: [PATCH 1/2] Handle context canceled errors + better error handling --- proxy/http_handler.go | 3 ++- proxy/http_proxy.go | 18 ++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/proxy/http_handler.go b/proxy/http_handler.go index cfde275eb..fe30c38ad 100644 --- a/proxy/http_handler.go +++ b/proxy/http_handler.go @@ -7,7 +7,7 @@ import ( "time" ) -func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration) http.Handler { +func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration, errorHandler func(http.ResponseWriter, *http.Request, error)) http.Handler { return &httputil.ReverseProxy{ // this is a simplified director function based on the // httputil.NewSingleHostReverseProxy() which does not @@ -25,5 +25,6 @@ func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration) ht }, FlushInterval: flush, Transport: tr, + ErrorHandler: errorHandler, } } diff --git a/proxy/http_proxy.go b/proxy/http_proxy.go index c8226a3a5..1e5aa6efa 100644 --- a/proxy/http_proxy.go +++ b/proxy/http_proxy.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "errors" "io" + "log" "net" "net/http" "net/url" @@ -187,10 +188,10 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { case accept == "text/event-stream": // use the flush interval for SSE (server-sent events) // must be > 0s to be effective - h = newHTTPProxy(targetURL, tr, p.Config.FlushInterval) + h = newHTTPProxy(targetURL, tr, p.Config.FlushInterval, httpProxyErrorHandler) default: - h = newHTTPProxy(targetURL, tr, p.Config.GlobalFlushInterval) + h = newHTTPProxy(targetURL, tr, p.Config.GlobalFlushInterval, httpProxyErrorHandler) } if p.Config.GZIPContentTypes != nil { @@ -238,6 +239,19 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } +func httpProxyErrorHandler(w http.ResponseWriter, r *http.Request, err error) { + // According to https://golang.org/src/net/http/httputil/reverseproxy.go#L74, Go will return a 502 (Bad Gateway) StatusCode by default if no ErroHandler is provided + // If a "context canceled" error is returned by the http.Request handler this means the client closed the connection before getting a response + // So we are changing the StatusCode on these situations to the non-standard 499 (Client Closed Connection) + if rErr := r.Context().Err(); rErr != nil && rErr.Error() == "context canceled" { + w.WriteHeader(499) + } else if err != nil { + log.Printf("[ERROR] %s", err.Error()) + w.WriteHeader(504) + } + return +} + func key(code int) string { b := []byte("http.status.") b = strconv.AppendInt(b, int64(code), 10) From 1763ed058defe281f81acce3f5d77609274124b4 Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Thu, 18 Apr 2019 12:39:47 -0600 Subject: [PATCH 2/2] Handle context canceled errors + better error handling --- proxy/http_handler.go | 40 ++++++++++++++++++++++++++++++++++++++-- proxy/http_proxy.go | 18 ++---------------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/proxy/http_handler.go b/proxy/http_handler.go index fe30c38ad..5cda47160 100644 --- a/proxy/http_handler.go +++ b/proxy/http_handler.go @@ -1,13 +1,20 @@ package proxy import ( + "context" + "io" + "log" + "net" "net/http" "net/http/httputil" "net/url" "time" ) -func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration, errorHandler func(http.ResponseWriter, *http.Request, error)) http.Handler { +// StatusClientClosedRequest non-standard HTTP status code for client disconnection +const StatusClientClosedRequest = 499 + +func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration) http.Handler { return &httputil.ReverseProxy{ // this is a simplified director function based on the // httputil.NewSingleHostReverseProxy() which does not @@ -25,6 +32,35 @@ func newHTTPProxy(target *url.URL, tr http.RoundTripper, flush time.Duration, er }, FlushInterval: flush, Transport: tr, - ErrorHandler: errorHandler, + ErrorHandler: httpProxyErrorHandler, } } + +func httpProxyErrorHandler(w http.ResponseWriter, r *http.Request, err error) { + // According to https://golang.org/src/net/http/httputil/reverseproxy.go#L74, Go will return a 502 (Bad Gateway) StatusCode by default if no ErroHandler is provided + // If a "context canceled" error is returned by the http.Request handler this means the client closed the connection before getting a response + // So we are changing the StatusCode on these situations to the non-standard 499 (Client Closed Request) + + statusCode := http.StatusInternalServerError + + if e, ok := err.(net.Error); ok { + if e.Timeout() { + statusCode = http.StatusGatewayTimeout + } else { + statusCode = http.StatusBadGateway + } + } else if err == io.EOF { + statusCode = http.StatusBadGateway + } else if err == context.Canceled { + statusCode = StatusClientClosedRequest + } + + w.WriteHeader(statusCode) + // Theres nothing we can do if the client closes the connection and logging the "context canceled" errors will just add noise to the error log + // Note: The access_log will still log the 499 response status codes + if statusCode != StatusClientClosedRequest { + log.Print("[ERROR] ", err) + } + + return +} diff --git a/proxy/http_proxy.go b/proxy/http_proxy.go index 1e5aa6efa..c8226a3a5 100644 --- a/proxy/http_proxy.go +++ b/proxy/http_proxy.go @@ -5,7 +5,6 @@ import ( "crypto/tls" "errors" "io" - "log" "net" "net/http" "net/url" @@ -188,10 +187,10 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { case accept == "text/event-stream": // use the flush interval for SSE (server-sent events) // must be > 0s to be effective - h = newHTTPProxy(targetURL, tr, p.Config.FlushInterval, httpProxyErrorHandler) + h = newHTTPProxy(targetURL, tr, p.Config.FlushInterval) default: - h = newHTTPProxy(targetURL, tr, p.Config.GlobalFlushInterval, httpProxyErrorHandler) + h = newHTTPProxy(targetURL, tr, p.Config.GlobalFlushInterval) } if p.Config.GZIPContentTypes != nil { @@ -239,19 +238,6 @@ func (p *HTTPProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -func httpProxyErrorHandler(w http.ResponseWriter, r *http.Request, err error) { - // According to https://golang.org/src/net/http/httputil/reverseproxy.go#L74, Go will return a 502 (Bad Gateway) StatusCode by default if no ErroHandler is provided - // If a "context canceled" error is returned by the http.Request handler this means the client closed the connection before getting a response - // So we are changing the StatusCode on these situations to the non-standard 499 (Client Closed Connection) - if rErr := r.Context().Err(); rErr != nil && rErr.Error() == "context canceled" { - w.WriteHeader(499) - } else if err != nil { - log.Printf("[ERROR] %s", err.Error()) - w.WriteHeader(504) - } - return -} - func key(code int) string { b := []byte("http.status.") b = strconv.AppendInt(b, int64(code), 10)