apiserver: preserve stack trace in handler panic beyond timeout handler

pull/58/head
Dr. Stefan Schimanski 2018-11-15 09:56:49 +01:00
parent e43e5e2e45
commit 96fd0482f4
4 changed files with 43 additions and 10 deletions

View File

@ -63,7 +63,11 @@ func HandleCrash(additionalHandlers ...func(interface{})) {
// logPanic logs the caller tree when a panic occurs.
func logPanic(r interface{}) {
callers := getCallers(r)
klog.Errorf("Observed a panic: %#v (%v)\n%v", r, r, callers)
if _, ok := r.(string); ok {
klog.Errorf("Observed a panic: %s\n%v", r, callers)
} else {
klog.Errorf("Observed a panic: %#v (%v)\n%v", r, r, callers)
}
}
func getCallers(r interface{}) string {

View File

@ -23,6 +23,7 @@ import (
"fmt"
"net"
"net/http"
"runtime"
"sync"
"time"
@ -91,16 +92,23 @@ func (t *timeoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}
result := make(chan interface{})
errCh := make(chan interface{})
tw := newTimeoutWriter(w)
go func() {
defer func() {
result <- recover()
err := recover()
if err != nil {
const size = 64 << 10
buf := make([]byte, size)
buf = buf[:runtime.Stack(buf, false)]
err = fmt.Sprintf("%v\n%s", err, buf)
}
errCh <- err
}()
t.handler.ServeHTTP(tw, r)
}()
select {
case err := <-result:
case err := <-errCh:
if err != nil {
panic(err)
}

View File

@ -18,10 +18,12 @@ package filters
import (
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"reflect"
"strings"
"sync"
"testing"
"time"
@ -72,16 +74,21 @@ func TestTimeout(t *testing.T) {
sendResponse := make(chan string, 1)
doPanic := make(chan struct{}, 1)
writeErrors := make(chan error, 1)
gotPanic := make(chan interface{}, 1)
timeout := make(chan time.Time, 1)
resp := "test response"
timeoutErr := apierrors.NewServerTimeout(schema.GroupResource{Group: "foo", Resource: "bar"}, "get", 0)
record := &recorder{}
handler := newHandler(sendResponse, doPanic, writeErrors)
ts := httptest.NewServer(WithPanicRecovery(WithTimeout(handler,
func(req *http.Request) (*http.Request, <-chan time.Time, func(), *apierrors.StatusError) {
ts := httptest.NewServer(withPanicRecovery(
WithTimeout(handler, func(req *http.Request) (*http.Request, <-chan time.Time, func(), *apierrors.StatusError) {
return req, timeout, record.Record, timeoutErr
})))
}), func(w http.ResponseWriter, req *http.Request, err interface{}) {
gotPanic <- err
http.Error(w, "This request caused apiserver to panic. Look in the logs for details.", http.StatusInternalServerError)
}),
)
defer ts.Close()
// No timeouts
@ -140,4 +147,13 @@ func TestTimeout(t *testing.T) {
if res.StatusCode != http.StatusInternalServerError {
t.Errorf("got res.StatusCode %d; expected %d due to panic", res.StatusCode, http.StatusInternalServerError)
}
select {
case err := <-gotPanic:
msg := fmt.Sprintf("%v", err)
if !strings.Contains(msg, "newHandler") {
t.Errorf("expected line with root cause panic in the stack trace, but didn't: %v", err)
}
case <-time.After(30 * time.Second):
t.Fatalf("expected to see a handler panic, but didn't")
}
}

View File

@ -18,7 +18,6 @@ package filters
import (
"net/http"
"runtime/debug"
"k8s.io/klog"
@ -28,10 +27,16 @@ import (
// WithPanicRecovery wraps an http Handler to recover and log panics.
func WithPanicRecovery(handler http.Handler) http.Handler {
return withPanicRecovery(handler, func(w http.ResponseWriter, req *http.Request, err interface{}) {
http.Error(w, "This request caused apiserver to panic. Look in the logs for details.", http.StatusInternalServerError)
klog.Errorf("apiserver panic'd on %v %v", req.Method, req.RequestURI)
})
}
func withPanicRecovery(handler http.Handler, crashHandler func(http.ResponseWriter, *http.Request, interface{})) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
defer runtime.HandleCrash(func(err interface{}) {
http.Error(w, "This request caused apiserver to panic. Look in the logs for details.", http.StatusInternalServerError)
klog.Errorf("apiserver panic'd on %v %v: %v\n%s\n", req.Method, req.RequestURI, err, debug.Stack())
crashHandler(w, req, err)
})
logger := httplog.NewLogged(req, &w)