diff --git a/src/github.com/matrix-org/go-neb/goneb.go b/src/github.com/matrix-org/go-neb/goneb.go index 9afaf38..7b73ca2 100644 --- a/src/github.com/matrix-org/go-neb/goneb.go +++ b/src/github.com/matrix-org/go-neb/goneb.go @@ -193,9 +193,9 @@ func main() { http.Handle("/metrics", prometheus.Handler()) http.Handle("/test", prometheus.InstrumentHandler("test", server.MakeJSONAPI(&heartbeatHandler{}))) wh := &webhookHandler{db: db, clients: clients} - http.HandleFunc("/services/hooks/", prometheus.InstrumentHandlerFunc("webhookHandler", wh.handle)) + http.HandleFunc("/services/hooks/", prometheus.InstrumentHandlerFunc("webhookHandler", server.Protect(wh.handle))) rh := &realmRedirectHandler{db: db} - http.HandleFunc("/realms/redirects/", prometheus.InstrumentHandlerFunc("realmRedirectHandler", rh.handle)) + http.HandleFunc("/realms/redirects/", prometheus.InstrumentHandlerFunc("realmRedirectHandler", server.Protect(rh.handle))) // Read exclusively from the config file if one was supplied. // Otherwise, add HTTP listeners for new Services/Sessions/Clients/etc. diff --git a/src/github.com/matrix-org/go-neb/matrix/worker.go b/src/github.com/matrix-org/go-neb/matrix/worker.go index c5770f5..3674601 100644 --- a/src/github.com/matrix-org/go-neb/matrix/worker.go +++ b/src/github.com/matrix-org/go-neb/matrix/worker.go @@ -1,5 +1,10 @@ package matrix +import ( + log "github.com/Sirupsen/logrus" + "runtime/debug" +) + // Worker processes incoming events and updates the Matrix client's data structures. It also informs // any attached listeners of the new events. type Worker struct { @@ -38,6 +43,21 @@ func (worker *Worker) notifyListeners(event *Event) { } func (worker *Worker) onSyncHTTPResponse(res syncHTTPResponse) { + defer func() { + if r := recover(); r != nil { + userID := "" + if worker.client != nil { + userID = worker.client.UserID + } + log.WithFields(log.Fields{ + "panic": r, + "user_id": userID, + }).Errorf( + "onSyncHTTPResponse panicked!\n%s", debug.Stack(), + ) + } + }() + for roomID, roomData := range res.Rooms.Join { room := worker.client.getOrCreateRoom(roomID) for _, event := range roomData.State.Events { diff --git a/src/github.com/matrix-org/go-neb/polling/polling.go b/src/github.com/matrix-org/go-neb/polling/polling.go index 2500d30..0a84a92 100644 --- a/src/github.com/matrix-org/go-neb/polling/polling.go +++ b/src/github.com/matrix-org/go-neb/polling/polling.go @@ -5,6 +5,7 @@ import ( "github.com/matrix-org/go-neb/clients" "github.com/matrix-org/go-neb/database" "github.com/matrix-org/go-neb/types" + "runtime/debug" "sync" "time" ) @@ -71,6 +72,17 @@ func pollLoop(service types.Service, ts int64) { "service_id": service.ServiceID(), "service_type": service.ServiceType(), }) + + defer func() { + // Kill the poll loop entirely as it is likely that whatever made us panic will + // make us panic again. We can whine bitterly about it though. + if r := recover(); r != nil { + logger.WithField("panic", r).Errorf( + "pollLoop panicked!\n%s", debug.Stack(), + ) + } + }() + poller, ok := service.(types.Poller) if !ok { logger.Error("Service is not a Poller.") diff --git a/src/github.com/matrix-org/go-neb/server/server.go b/src/github.com/matrix-org/go-neb/server/server.go index fa1bb7f..b240914 100644 --- a/src/github.com/matrix-org/go-neb/server/server.go +++ b/src/github.com/matrix-org/go-neb/server/server.go @@ -6,6 +6,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/matrix-org/go-neb/errors" "net/http" + "runtime/debug" ) // JSONRequestHandler represents an interface that must be satisfied in order to respond to incoming @@ -34,14 +35,38 @@ func WithCORSOptions(handler http.HandlerFunc) http.HandlerFunc { } } +// Protect panicking HTTP requests from taking down the entire process, and log them using +// the correct logger, returning a 500 with a JSON response rather than abruptly closing the +// connection. +func Protect(handler http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, req *http.Request) { + defer func() { + if r := recover(); r != nil { + log.WithFields(log.Fields{ + "panic": r, + "method": req.Method, + "url": req.URL, + }).Errorf( + "Request panicked!\n%s", debug.Stack(), + ) + jsonErrorResponse( + w, req, &errors.HTTPError{nil, "Internal Server Error", 500}, + ) + } + }() + handler(w, req) + } +} + // MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses. func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc { - return func(w http.ResponseWriter, req *http.Request) { + return Protect(func(w http.ResponseWriter, req *http.Request) { logger := log.WithFields(log.Fields{ "method": req.Method, "url": req.URL, }) - logger.Print(">>> Incoming request") + logger.Print("Incoming request") + res, httpErr := handler.OnIncomingRequest(req) // Set common headers returned regardless of the outcome of the request @@ -67,7 +92,7 @@ func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc { resBytes = r } w.Write(resBytes) - } + }) } func jsonErrorResponse(w http.ResponseWriter, req *http.Request, httpErr *errors.HTTPError) { diff --git a/src/github.com/matrix-org/go-neb/server/server_test.go b/src/github.com/matrix-org/go-neb/server/server_test.go new file mode 100644 index 0000000..920d0ae --- /dev/null +++ b/src/github.com/matrix-org/go-neb/server/server_test.go @@ -0,0 +1,29 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestProtect(t *testing.T) { + mockWriter := httptest.NewRecorder() + mockReq, _ := http.NewRequest("GET", "http://example.com/foo", nil) + h := Protect(func(w http.ResponseWriter, req *http.Request) { + var array []string + w.Write([]byte(array[5])) // NPE + }) + + h(mockWriter, mockReq) + + expectCode := 500 + if mockWriter.Code != expectCode { + t.Errorf("TestProtect wanted HTTP status %d, got %d", expectCode, mockWriter.Code) + } + + expectBody := `{"message":"Internal Server Error"}` + actualBody := mockWriter.Body.String() + if actualBody != expectBody { + t.Errorf("TestProtect wanted body %s, got %s", expectBody, actualBody) + } +}