Avoid context canceled errors

Return 499 Client Closed Request when the client has closed the request before the server could send a response

Signed-off-by: Seena Fallah <seenafallah@gmail.com>

Author: clwluvw

middleware/proxy_1_11.go

@@ -3,23 +3,44 @@
 package middleware
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httputil"
+	"strings"
 
 	"github.com/labstack/echo/v4"
 )
 
+// StatusCodeContextCanceled is HTTP status code for when client closed connection
+// 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;
+const StatusCodeContextCanceled = 499
+
 func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handler {
 	proxy := httputil.NewSingleHostReverseProxy(tgt.URL)
 	proxy.ErrorHandler = func(resp http.ResponseWriter, req *http.Request, err error) {
 		desc := tgt.URL.String()
 		if tgt.Name != "" {
 			desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String())
 		}
-		httpError := echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err))
-		httpError.Internal = err
-		c.Set("_error", httpError)
+		// 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 err == context.Canceled || strings.Contains(err.Error(), "operation was canceled") {
+			httpError := echo.NewHTTPError(StatusCodeContextCanceled, fmt.Sprintf("client closed connection: %v", err))
+			httpError.Internal = err
+			c.Set("_error", httpError)
+		} else {
+			httpError := echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err))
+			httpError.Internal = err
+			c.Set("_error", httpError)
+		}
 	}
 	proxy.Transport = config.Transport
 	proxy.ModifyResponse = config.ModifyResponse

middleware/proxy_1_11_test.go

@@ -7,9 +7,11 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"testing"
+	"time"
 
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/context"
 )
 
 func TestProxy_1_11(t *testing.T) {
@@ -50,4 +52,30 @@ func TestProxy_1_11(t *testing.T) {
 	e.ServeHTTP(rec, req)
 	assert.Equal(t, "/api/users", req.URL.Path)
 	assert.Equal(t, http.StatusBadGateway, rec.Code)
+
+	// client closed connection
+	HTTPTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(30 * time.Second)
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer HTTPTarget.Close()
+	targetURL, _ := url.Parse(HTTPTarget.URL)
+	target := &ProxyTarget{
+		Name: "target",
+		URL:  targetURL,
+	}
+	rb = NewRandomBalancer(nil)
+	assert.True(t, rb.AddTarget(target))
+	e = echo.New()
+	e.Use(Proxy(rb))
+	rec = httptest.NewRecorder()
+	req = httptest.NewRequest(http.MethodGet, "/", nil)
+	ctx, cancel := context.WithCancel(req.Context())
+	req = req.WithContext(ctx)
+	e.ServeHTTP(rec, req)
+	startTime := time.Now()
+	cancel()
+	endTime := time.Now()
+	assert.Less(t, endTime.Sub(startTime).Seconds(), float64(30))
+	assert.Equal(t, 499, rec.Code)
 }