Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[blocked] Record client disconnected errors as HTTP 499 #2321

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions authfe/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ func newHTTPProxy(cfg proxyConfig) (*httpProxy, error) {
return &httpProxy{
proxyConfig: cfg,
reverseProxy: httputil.ReverseProxy{
Director: func(*http.Request) {},
Transport: proxyTransport,
Director: func(*http.Request) {},
Transport: proxyTransport,
ErrorHandler: handleError,
},
balancer: balancer,
}, nil
Expand Down Expand Up @@ -233,6 +234,18 @@ func (p *httpProxy) proxyWS(w http.ResponseWriter, r *http.Request) {
logger.Debugf("proxy: websocket: connection closed")
}

func handleError(rw http.ResponseWriter, outReq *http.Request, err error) {
ctx := outReq.Context()
var header int
if ctx.Err() == context.Canceled {
header = 499 // Client Closed Request (nginx convention)
} else {
user.LogWith(ctx, logging.Global()).WithField("err", err).Errorln("http proxy error")
header = http.StatusBadGateway
}
rw.WriteHeader(header)
}

type closeWriter interface {
CloseWrite() error
}
Expand Down
46 changes: 46 additions & 0 deletions authfe/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,49 @@ func TestProxyGRPCTracing(t *testing.T) {
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, "world", string(body[:c]))
}

func TestProxyClientClosed(t *testing.T) {
serverCh := make(chan interface{})

// Set up a slow server
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(418)
serverCh <- nil // Synchonise with the test
}))
defer server.Close()

// Set up a proxy handler pointing at the server
serverURL, err := url.Parse(server.URL)
assert.NoError(t, err, "Cannot parse URL")
proxyHandler, _ := newProxy(proxyConfig{hostAndPort: serverURL.Host, protocol: "http"})

// Intercept the proxy response to check the response code
codeCh := make(chan int)
interceptedProxyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
recorder := httptest.NewRecorder()
proxyHandler.ServeHTTP(recorder, r)
codeCh <- recorder.Code
w.WriteHeader(recorder.Code)
})

// Set up the proxy server
proxyServer := httptest.NewServer(interceptedProxyHandler)
defer proxyServer.Close()

// Make a request which times out faster than the slow server
req, err := http.NewRequest("GET", proxyServer.URL, nil)
require.NoError(t, err)
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
req = req.WithContext(ctx)

_, err = http.DefaultClient.Do(req)
// Check that the request timed out early
assert.Error(t, context.DeadlineExceeded, err)
// Wait for the slow server to receive the request, and allow it to continue
assert.Nil(t, <-serverCh)
// Wait for the proxyHandler to finish handling the request, and allow it to continue
responseCode := <-codeCh
// Check that the proxy server set the response code to 499
assert.Equal(t, 499, responseCode)
}