From 53aa60afff2e66b4a2b2ca6968fe5cb6c8567437 Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Wed, 25 Nov 2020 10:54:23 -0700 Subject: reverseproxy: Handle "operation was canceled" errors (#3816) * fix(caddy): Avoid "operation was canceled" errors - Also add error handling for StatusGatewayTimeout * revert(caddy): Revert 504 handling - This will potentially break load balancing and health checks * Handle client cancellation as different error Co-authored-by: Matthew Holt --- modules/caddyhttp/reverseproxy/reverseproxy.go | 27 +++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'modules') diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index bed5289..3a2457f 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -384,8 +385,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht // DialInfo struct should have valid network address syntax dialInfo, err := upstream.fillDialInfo(r) if err != nil { - err = fmt.Errorf("making dial info: %v", err) - return caddyhttp.Error(http.StatusBadGateway, err) + return statusError(fmt.Errorf("making dial info: %v", err)) } // attach to the request information about how to dial the upstream; @@ -438,7 +438,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyht } } - return caddyhttp.Error(http.StatusBadGateway, proxyErr) + return statusError(proxyErr) } // prepareRequest modifies req so that it is ready to be proxied, @@ -762,6 +762,27 @@ func removeConnectionHeaders(h http.Header) { } } +// statusError returns an error value that has a status code. +func statusError(err error) error { + // errors proxying usually mean there is a problem with the upstream(s) + statusCode := http.StatusBadGateway + + // if the client canceled the request (usually this means they closed + // the connection, so they won't see any response), we can report it + // as a client error (4xx) and not a server error (5xx); unfortunately + // the Go standard library, at least at time of writing in late 2020, + // obnoxiously wraps the exported, standard context.Canceled error with + // an unexported garbage value that we have to do a substring check for: + // https://github.com/golang/go/blob/6965b01ea248cabb70c3749fd218b36089a21efb/src/net/net.go#L416-L430 + if errors.Is(err, context.Canceled) || strings.Contains(err.Error(), "operation was canceled") { + // regrettably, there is no standard error code for "client closed connection", but + // for historical reasons we can use a code that a lot of people are already using; + // using 5xx is problematic for users; see #3748 + statusCode = 499 + } + return caddyhttp.Error(statusCode, err) +} + // LoadBalancing has parameters related to load balancing. type LoadBalancing struct { // A selection policy is how to choose an available backend. -- cgit v1.2.3