From 21b32b467e31a09fe473667bc33590bad5069b56 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Fri, 5 Apr 2024 15:39:44 -0700 Subject: [PATCH] tsweb: handle panics in retHandler We would have incomplete stats and missing logs in cases of panics. Updates tailscale/corp#18687 Signed-off-by: Maisem Ali --- tsweb/tsweb.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tsweb/tsweb.go b/tsweb/tsweb.go index 8841c8c5d..c28374d87 100644 --- a/tsweb/tsweb.go +++ b/tsweb/tsweb.go @@ -330,7 +330,32 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } lw := &loggingResponseWriter{ResponseWriter: w, logf: h.opts.Logf} - err := h.rh.ServeHTTPReturn(lw, r) + + // In case the handler panics, we want to recover and continue logging the + // error before raising the panic again for the server to handle. + var ( + didPanic bool + panicRes any + ) + defer func() { + if didPanic { + panic(panicRes) + } + }() + runWithPanicProtection := func() (err error) { + defer func() { + if r := recover(); r != nil { + didPanic = true + panicRes = r + // Even if r is an error, do not wrap it as an error here as + // that would allow things like panic(vizerror.New("foo")) which + // is really hard to define the behavior of. + err = fmt.Errorf("panic: %v", r) + } + }() + return h.rh.ServeHTTPReturn(lw, r) + } + err := runWithPanicProtection() var hErr HTTPError var hErrOK bool