Skip to content

Commit beb1169

Browse files
luojun96Rader
authored andcommitted
Fix reverse proxy canceled request status
1 parent c465f2d commit beb1169

5 files changed

Lines changed: 42 additions & 8 deletions

File tree

api/middleware/log.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,10 @@ import (
88

99
"github.com/gin-gonic/gin"
1010
"opencsg.com/csghub-server/api/httpbase"
11+
commonutils "opencsg.com/csghub-server/common/utils/common"
1112
"opencsg.com/csghub-server/common/utils/trace"
1213
)
1314

14-
// Status 499 is a non-standard code introduced by nginx to indicate
15-
// "Client Closed Request" — the client disconnected before the server
16-
// finished processing.
17-
const StatusClientClosedRequest = 499
18-
1915
func Log() gin.HandlerFunc {
2016
return func(ctx *gin.Context) {
2117
if ctx.Request.URL.Path == "/healthz" {
@@ -43,7 +39,7 @@ func Log() gin.HandlerFunc {
4339
// If the client disconnected (timeout or explicit cancel),
4440
// override the logged status to 499.
4541
if ctx.Request.Context().Err() == context.Canceled {
46-
status = StatusClientClosedRequest
42+
status = commonutils.StatusClientClosedRequest
4743
}
4844

4945
// Derive a non-canceled context for the log call.

api/middleware/log_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/gin-gonic/gin"
1414
"github.com/stretchr/testify/assert"
1515
"github.com/stretchr/testify/require"
16+
commonutils "opencsg.com/csghub-server/common/utils/common"
1617
)
1718

1819
// captureLog is a helper that installs a slog JSON handler writing to a buffer
@@ -82,7 +83,7 @@ func TestLog_ClientCancelBeforeHandler(t *testing.T) {
8283

8384
logs := readLogs()
8485
require.Len(t, logs, 1, "must emit a log even when client canceled")
85-
assert.Equal(t, float64(StatusClientClosedRequest), logs[0]["status"], "status should be 499 for client cancel")
86+
assert.Equal(t, float64(commonutils.StatusClientClosedRequest), logs[0]["status"], "status should be 499 for client cancel")
8687
assert.Equal(t, "/test", logs[0]["url"])
8788
}
8889

@@ -116,7 +117,7 @@ func TestLog_ClientCancelDuringSlowHandler(t *testing.T) {
116117

117118
logs := readLogs()
118119
require.Len(t, logs, 1, "must emit a log when client cancels during slow handler")
119-
assert.Equal(t, float64(StatusClientClosedRequest), logs[0]["status"], "status should be 499")
120+
assert.Equal(t, float64(commonutils.StatusClientClosedRequest), logs[0]["status"], "status should be 499")
120121
}
121122

122123
func TestLog_HandlerPanicWithRecovery(t *testing.T) {

builder/proxy/reverse_proxy.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package proxy
22

33
import (
4+
"context"
5+
"errors"
46
"log/slog"
57
"net/http"
68
"net/http/httputil"
79
"net/url"
810

911
"github.com/openai/openai-go/v3"
12+
commonutils "opencsg.com/csghub-server/common/utils/common"
1013
"opencsg.com/csghub-server/common/utils/trace"
1114
)
1215

@@ -54,6 +57,15 @@ func (rp *reverseProxyImpl) ServeHTTP(w http.ResponseWriter, r *http.Request, ap
5457
}
5558
}()
5659
proxy := httputil.NewSingleHostReverseProxy(rp.target)
60+
proxy.ErrorHandler = func(rw http.ResponseWriter, req *http.Request, err error) {
61+
if errors.Is(err, context.Canceled) {
62+
slog.InfoContext(context.WithoutCancel(req.Context()), "http: proxy request canceled", slog.Any("error", err))
63+
rw.WriteHeader(commonutils.StatusClientClosedRequest)
64+
return
65+
}
66+
slog.ErrorContext(req.Context(), "http: proxy error", slog.Any("error", err))
67+
rw.WriteHeader(http.StatusBadGateway)
68+
}
5769
proxy.Director = func(req *http.Request) {
5870
if len(svcHost) > 0 {
5971
slog.Info("update reverse proxy header host", slog.Any("svc-host", svcHost))

builder/proxy/reverse_proxy_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package proxy
22

33
import (
4+
"context"
45
"net/http"
56
"net/http/httptest"
67
"testing"
78

89
"github.com/stretchr/testify/require"
10+
commonutils "opencsg.com/csghub-server/common/utils/common"
911
)
1012

1113
func TestReverseProxy_AcceptEncodingDefaultGzip(t *testing.T) {
@@ -47,6 +49,24 @@ func TestReverseProxy_AcceptEncodingDisabled(t *testing.T) {
4749
require.Equal(t, "identity", downstreamAcceptEncoding)
4850
}
4951

52+
func TestReverseProxy_ContextCanceledWritesClientClosed(t *testing.T) {
53+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
54+
w.WriteHeader(http.StatusOK)
55+
}))
56+
defer server.Close()
57+
58+
rp, err := NewReverseProxy(server.URL)
59+
require.NoError(t, err)
60+
61+
ctx, cancel := context.WithCancel(context.Background())
62+
cancel()
63+
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
64+
resp := httptest.NewRecorder()
65+
rp.ServeHTTP(resp, req, "", "")
66+
67+
require.Equal(t, commonutils.StatusClientClosedRequest, resp.Code)
68+
}
69+
5070
// import (
5171
// "bytes"
5272
// "io"

common/utils/common/http_status.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package common
2+
3+
// StatusClientClosedRequest is the non-standard nginx status code for a client
4+
// closing the request before the server finishes processing it.
5+
const StatusClientClosedRequest = 499

0 commit comments

Comments
 (0)