From 92ac46e9258bab0efe2ab44902fccfbb4ff4f6a6 Mon Sep 17 00:00:00 2001
From: Kegan Dougal <kegan@matrix.org>
Date: Mon, 24 Oct 2016 16:04:32 +0100
Subject: [PATCH] Factor out a Protect() function for incoming HTTP requests

---
 src/github.com/matrix-org/go-neb/goneb.go     |  4 +--
 .../matrix-org/go-neb/server/server.go        | 32 +++++++++++++------
 2 files changed, 25 insertions(+), 11 deletions(-)

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/server/server.go b/src/github.com/matrix-org/go-neb/server/server.go
index 91e64e8..b240914 100644
--- a/src/github.com/matrix-org/go-neb/server/server.go
+++ b/src/github.com/matrix-org/go-neb/server/server.go
@@ -35,17 +35,18 @@ func WithCORSOptions(handler http.HandlerFunc) http.HandlerFunc {
 	}
 }
 
-// MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses.
-func MakeJSONAPI(handler JSONRequestHandler) 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) {
-		logger := log.WithFields(log.Fields{
-			"method": req.Method,
-			"url":    req.URL,
-		})
-		logger.Print("Incoming request")
 		defer func() {
 			if r := recover(); r != nil {
-				logger.WithField("panic", r).Errorf(
+				log.WithFields(log.Fields{
+					"panic":  r,
+					"method": req.Method,
+					"url":    req.URL,
+				}).Errorf(
 					"Request panicked!\n%s", debug.Stack(),
 				)
 				jsonErrorResponse(
@@ -53,6 +54,19 @@ func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc {
 				)
 			}
 		}()
+		handler(w, req)
+	}
+}
+
+// MakeJSONAPI creates an HTTP handler which always responds to incoming requests with JSON responses.
+func MakeJSONAPI(handler JSONRequestHandler) http.HandlerFunc {
+	return Protect(func(w http.ResponseWriter, req *http.Request) {
+		logger := log.WithFields(log.Fields{
+			"method": req.Method,
+			"url":    req.URL,
+		})
+		logger.Print("Incoming request")
+
 		res, httpErr := handler.OnIncomingRequest(req)
 
 		// Set common headers returned regardless of the outcome of the request
@@ -78,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) {