Skip to content

Commit 287e3c0

Browse files
authored
refactor: handle ErrAbortHandler in gin recovery middleware instead of per-handler recover (#134)
Move SSE panic handling from a per-handler recover() in proxy.go and ErrorHandler in transparent.go to a centralized CustomRecoveryWithZap middleware. This provides consistent ErrAbortHandler handling across the entire app without masking legitimate copy errors in the reverse proxy. Relates to #128
1 parent 76d1ac5 commit 287e3c0

3 files changed

Lines changed: 7 additions & 48 deletions

File tree

pkg/backend/transparent.go

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -125,27 +125,7 @@ func (p *TransparentBackend) Run(ctx context.Context) (http.Handler, error) {
125125
base: http.DefaultTransport,
126126
targetHost: p.url.Host,
127127
},
128-
// FlushInterval -1 enables immediate flushing for SSE streams.
129-
// Without this the proxy buffers the response, breaking
130-
// Server-Sent Events used by MCP Streamable HTTP.
131128
FlushInterval: -1,
132-
// ErrorHandler prevents the default ReverseProxy behavior of
133-
// panicking when the backend closes the connection (e.g. when
134-
// an SSE stream ends or the client disconnects). Instead we
135-
// log the error and return a 502 to the client.
136-
ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) {
137-
p.logger.Warn("reverse proxy error",
138-
zap.String("method", r.Method),
139-
zap.String("path", r.URL.Path),
140-
zap.Error(err),
141-
)
142-
// Only write an error response if headers haven't been sent yet
143-
// (i.e. the stream hasn't started). For in-flight SSE streams
144-
// the ResponseWriter is already flushed, so just return.
145-
if !isHeadersSent(w) {
146-
http.Error(w, "Bad Gateway", http.StatusBadGateway)
147-
}
148-
},
149129
Rewrite: func(pr *httputil.ProxyRequest) {
150130
pr.SetURL(p.url)
151131
if p.isTrusted(pr.In.RemoteAddr) {
@@ -168,17 +148,6 @@ func (p *TransparentBackend) Run(ctx context.Context) (http.Handler, error) {
168148
return &rp, nil
169149
}
170150

171-
// isHeadersSent checks whether the ResponseWriter has already started sending
172-
// the response (status + headers). Once flushed we can no longer write an
173-
// error status, so callers should just close the connection instead.
174-
func isHeadersSent(w http.ResponseWriter) bool {
175-
// A non-zero status in the header map means WriteHeader was called.
176-
// The standard http.response tracks this internally; the only reliable
177-
// external signal is whether Content-Type (or any header set by the
178-
// backend) is already present in the response.
179-
return w.Header().Get("Content-Type") != ""
180-
}
181-
182151
func (p *TransparentBackend) isTrusted(hostport string) bool {
183152
if host, _, err := net.SplitHostPort(hostport); err == nil {
184153
hostport = host

pkg/mcp-proxy/main.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,13 @@ func Run(
305305
})
306306

307307
router.Use(ginzap.Ginzap(logger, time.RFC3339, true))
308-
router.Use(ginzap.RecoveryWithZap(logger, true))
308+
router.Use(ginzap.CustomRecoveryWithZap(logger, true, func(c *gin.Context, err any) {
309+
if err == http.ErrAbortHandler {
310+
c.Abort()
311+
return
312+
}
313+
c.AbortWithStatus(http.StatusInternalServerError)
314+
}))
309315
store := cookie.NewStore(secret)
310316
store.Options(sessions.Options{
311317
Path: "/",

pkg/proxy/proxy.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,22 +87,6 @@ func (p *ProxyRouter) handleProxy(c *gin.Context) {
8787
}
8888
}
8989

90-
// Recover from http.ErrAbortHandler panics that occur when SSE
91-
// streams end (backend closes connection, client disconnects).
92-
// The default ReverseProxy panics with ErrAbortHandler on flush
93-
// errors — this is expected for SSE and should not crash the proxy.
94-
defer func() {
95-
if r := recover(); r != nil {
96-
// http.ErrAbortHandler is the sentinel panic used by
97-
// net/http to silently abort a handler. Let it go.
98-
if r == http.ErrAbortHandler {
99-
return
100-
}
101-
// Re-panic for anything unexpected
102-
panic(r)
103-
}
104-
}()
105-
10690
p.proxy.ServeHTTP(c.Writer, c.Request)
10791
}
10892

0 commit comments

Comments
 (0)