Repository: labstack/echo
Pull request #1789 (2021-02-23 14:10:54)

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>

codecov[bot]: # [Codecov](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=h1) Report
> Merging [#1789](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=desc) (4a32468) into [master](https://codecov.io/gh/labstack/echo/commit/b0f56eaf969bf5f8a6e4b6727f3f5fc7f44f36a0?el=desc) (b0f56ea) will **decrease** coverage by `0.10%`.
> The diff coverage is `25.00%`.

[![Impacted file tree graph](https://codecov.io/gh/labstack/echo/pull/1789/graphs/tree.svg?width=650&height=150&src=pr&token=vqnT76l79i)](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1789      +/-   ##
==========================================
- Coverage   89.75%   89.65%   -0.11%     
==========================================
  Files          32       32              
  Lines        2675     2678       +3     
==========================================
  Hits         2401     2401              
- Misses        175      176       +1     
- Partials       99      101       +2     
```


| [Impacted Files](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [middleware/proxy\_1\_11.go](https://codecov.io/gh/labstack/echo/pull/1789/diff?src=pr&el=tree#diff-bWlkZGxld2FyZS9wcm94eV8xXzExLmdv) | `75.00% <25.00%> (-25.00%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=footer). Last update [b0f56ea...4a32468](https://codecov.io/gh/labstack/echo/pull/1789?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/middleware/proxy_1_11.go b/middleware/proxy_1_11.go
index a439278..17d142d 100644
--- a/middleware/proxy_1_11.go
+++ b/middleware/proxy_1_11.go
@@ -3,13 +3,22 @@
 package middleware
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/http/httputil"
+	"strings"
 
 	"github.com/labstack/echo/v4"
 )
 
+// StatusCodeContextCanceled is a custom HTTP status code for situations
+// where a client unexpectedly closed the connection to the server.
+// As there is no standard error code for "client closed connection", but
+// various well-known HTTP clients and server implement this HTTP code we use
+// 499 too instead of the more problematic 5xx, which does not allow to detect this situation
+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,7 +26,20 @@ func proxyHTTP(tgt *ProxyTarget, c echo.Context, config ProxyConfig) http.Handle
 		if tgt.Name != "" {
 			desc = fmt.Sprintf("%s(%s)", tgt.Name, tgt.URL.String())
 		}
-		c.Set("_error", echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("remote %s unreachable, could not forward: %v", desc, err)))
+		// If the client canceled the request (usually by closing the connection), we can report a
+		// client error (4xx) instead of a server error (5xx) to correctly identify the situation.
+		// The Go standard library (at of late 2020) wraps the exported, standard
+		// context.Canceled error with unexported garbage value requiring a substring check, see
+		// 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


Test patch:
diff --git a/middleware/proxy_1_11_test.go b/middleware/proxy_1_11_test.go
index 26feaab..c3541d5 100644
--- a/middleware/proxy_1_11_test.go
+++ b/middleware/proxy_1_11_test.go
@@ -3,10 +3,13 @@
 package middleware
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
@@ -51,3 +54,33 @@ func TestProxy_1_11(t *testing.T) {
 	assert.Equal(t, "/api/users", req.URL.Path)
 	assert.Equal(t, http.StatusBadGateway, rec.Code)
 }
+
+func TestClientCancelConnectionResultsHTTPCode499(t *testing.T) {
+	var timeoutStop sync.WaitGroup
+	timeoutStop.Add(1)
+	HTTPTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		timeoutStop.Wait() // wait until we have canceled the request
+		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() {
+		time.Sleep(10 * time.Millisecond)
+		cancel()
+	}()
+	e.ServeHTTP(rec, req)
+	timeoutStop.Done()
+	assert.Equal(t, 499, rec.Code)
+}