From 0672cfffa2fd05da45cdc2da5ce9d2d87a4d3701 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 11 Jun 2021 12:50:22 +0300 Subject: [PATCH] app/vmauth: properly handle http.ErrAbortHandler panic This panic can be raised by the reverseProxy on aborted request to the backend. So handle it (e.g. suppress) at reverseProxy.ServeHTTP call. Do not suppress the panic at lib/httpserver generic HTTP handler, since it may result in an inconsistent state left after the panicking handler. Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1353 --- app/vmauth/main.go | 16 +++++++++++++++- lib/httpserver/httpserver.go | 4 +--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/vmauth/main.go b/app/vmauth/main.go index 2825a93dc7..b12672fccb 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -81,10 +81,24 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool { return true } r.Header.Set("vm-target-url", targetURL.String()) - reverseProxy.ServeHTTP(w, r) + proxyRequest(w, r) return true } +func proxyRequest(w http.ResponseWriter, r *http.Request) { + defer func() { + err := recover() + if err == nil || err == http.ErrAbortHandler { + // Suppress http.ErrAbortHandler panic. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1353 + return + } + // Forward other panics to the caller. + panic(err) + }() + reverseProxy.ServeHTTP(w, r) +} + var configReloadRequests = metrics.NewCounter(`vmagent_http_requests_total{path="/-/reload"}`) var reverseProxy = &httputil.ReverseProxy{ diff --git a/lib/httpserver/httpserver.go b/lib/httpserver/httpserver.go index 57eb587e22..792eaef2eb 100644 --- a/lib/httpserver/httpserver.go +++ b/lib/httpserver/httpserver.go @@ -212,9 +212,7 @@ func handlerWrapper(s *server, w http.ResponseWriter, r *http.Request, rh Reques // The following recover() code works around this by explicitly stopping the process after logging the panic. // See https://github.com/golang/go/issues/16542#issuecomment-246549902 for details. defer func() { - // need to check for abortHandler - // https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1353 - if err := recover(); err != nil && err != http.ErrAbortHandler { + if err := recover(); err != nil { buf := make([]byte, 1<<20) n := runtime.Stack(buf, false) fmt.Fprintf(os.Stderr, "panic: %v\n\n%s", err, buf[:n])