diff --git a/middleware/proxy_1_11.go b/middleware/proxy_1_11.go index 019060f7e..0006bdde9 100644 --- a/middleware/proxy_1_11.go +++ b/middleware/proxy_1_11.go @@ -3,13 +3,21 @@ 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) { @@ -17,9 +25,22 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle 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 diff --git a/middleware/proxy_1_11_test.go b/middleware/proxy_1_11_test.go index 26feaabaa..b8ccf6988 100644 --- a/middleware/proxy_1_11_test.go +++ b/middleware/proxy_1_11_test.go @@ -3,10 +3,12 @@ package middleware import ( + "context" "net/http" "net/http/httptest" "net/url" "testing" + "time" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" @@ -50,4 +52,33 @@ 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) + go func() { + startTime := time.Now() + time.Sleep(5 * time.Second) + cancel() + endTime := time.Now() + assert.Less(t, endTime.Sub(startTime).Seconds(), float64(30)) + }() + e.ServeHTTP(rec, req) + assert.Equal(t, 499, rec.Code) }