uses a top level middleware to more easily catch application errors (#…

…1248)

* uses a top level middleware to more easily catch application errors

* fixes broken tests

* custom error handlers don't have access to context variables set in middleware fixes #1250

* improved the test a lot

* fixes gofmt issues

* uses a top level middleware to more easily catch application errors

* fixes broken tests

* custom error handlers don't have access to context variables set in middleware fixes #1250

* improved the test a lot

* fixed fmt

* fix fmt

app.go

@@ -15,14 +15,15 @@ import (
 type App struct {
 	Options
 	// Middleware returns the current MiddlewareStack for the App/Group.
-	Middleware    *MiddlewareStack
-	ErrorHandlers ErrorHandlers
-	router        *mux.Router
-	moot          *sync.Mutex
-	routes        RouteList
-	root          *App
-	children      []*App
-	filepaths     []string
+	Middleware      *MiddlewareStack
+	ErrorHandlers   ErrorHandlers
+	ErrorMiddleware MiddlewareFunc
+	router          *mux.Router
+	moot            *sync.Mutex
+	routes          RouteList
+	root            *App
+	children        []*App
+	filepaths       []string
 }
 
 // New returns a new instance of App and adds some sane, and useful, defaults.
@@ -31,8 +32,7 @@ func New(opts Options) *App {
 	opts = optionsWithDefaults(opts)
 
 	a := &App{
-		Options:    opts,
-		Middleware: newMiddlewareStack(),
+		Options: opts,
 		ErrorHandlers: ErrorHandlers{
 			404: defaultErrorHandler,
 			500: defaultErrorHandler,
@@ -43,6 +43,12 @@ func New(opts Options) *App {
 		children: []*App{},
 	}
 
+	dem := a.defaultErrorMiddleware
+	if a.ErrorMiddleware != nil {
+		dem = a.ErrorMiddleware
+	}
+	a.Middleware = newMiddlewareStack(dem)
+
 	notFoundHandler := func(errorf string, code int) http.HandlerFunc {
 		return func(res http.ResponseWriter, req *http.Request) {
 			c := a.newContext(RouteInfo{}, res, req)

errors.go

@@ -1,6 +1,7 @@
 package buffalo
 
 import (
+	"database/sql"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -74,6 +75,35 @@ func (a *App) PanicHandler(next Handler) Handler {
 	}
 }
 
+func (a *App) defaultErrorMiddleware(next Handler) Handler {
+	return func(c Context) error {
+		err := next(c)
+		if err == nil {
+			return nil
+		}
+		status := 500
+		// unpack root cause and check for HTTPError
+		cause := errors.Cause(err)
+		switch cause {
+		case sql.ErrNoRows:
+			status = 404
+		default:
+			if h, ok := cause.(HTTPError); ok {
+				status = h.Status
+			}
+		}
+		eh := a.ErrorHandlers.Get(status)
+		err = eh(status, err, c)
+		if err != nil {
+			// things have really hit the fan if we're here!!
+			a.Logger.Error(err)
+			c.Response().WriteHeader(500)
+			c.Response().Write([]byte(err.Error()))
+		}
+		return nil
+	}
+}
+
 func productionErrorResponseFor(status int) []byte {
 	if status == http.StatusNotFound {
 		return []byte(prodNotFoundTmpl)

errors_test.go

@@ -56,3 +56,31 @@ func Test_PanicHandler(t *testing.T) {
 		})
 	}
 }
+
+func Test_defaultErrorMiddleware(t *testing.T) {
+	r := require.New(t)
+	app := New(Options{})
+	var x string
+	var ok bool
+	app.ErrorHandlers[422] = func(code int, err error, c Context) error {
+		x, ok = c.Value("T").(string)
+		c.Response().WriteHeader(code)
+		c.Response().Write([]byte(err.Error()))
+		return nil
+	}
+	app.Use(func(next Handler) Handler {
+		return func(c Context) error {
+			c.Set("T", "t")
+			return c.Error(422, errors.New("boom"))
+		}
+	})
+	app.GET("/", func(c Context) error {
+		return nil
+	})
+
+	w := httptest.New(app)
+	res := w.HTML("/").Get()
+	r.Equal(422, res.Code)
+	r.True(ok)
+	r.Equal("t", x)
+}

route_info.go

@@ -1,11 +1,9 @@
 package buffalo
 
 import (
-	"database/sql"
 	"net/http"
 
 	gcontext "github.com/gorilla/context"
-	"github.com/pkg/errors"
 )
 
 func (info RouteInfo) ServeHTTP(res http.ResponseWriter, req *http.Request) {
@@ -19,24 +17,9 @@ func (info RouteInfo) ServeHTTP(res http.ResponseWriter, req *http.Request) {
 	err := a.Middleware.handler(info)(c)
 
 	if err != nil {
-		status := 500
-		// unpack root cause and check for HTTPError
-		cause := errors.Cause(err)
-		switch cause {
-		case sql.ErrNoRows:
-			status = 404
-		default:
-			if h, ok := cause.(HTTPError); ok {
-				status = h.Status
-			}
-		}
-		eh := a.ErrorHandlers.Get(status)
-		err = eh(status, err, c)
-		if err != nil {
-			// things have really hit the fan if we're here!!
-			a.Logger.Error(err)
-			c.Response().WriteHeader(500)
-			c.Response().Write([]byte(err.Error()))
-		}
+		// things have really hit the fan if we're here!!
+		a.Logger.Error(err)
+		c.Response().WriteHeader(500)
+		c.Response().Write([]byte(err.Error()))
 	}
 }

router_test.go

@@ -299,15 +299,15 @@ func Test_Router_Group_Middleware(t *testing.T) {
 
 	a := testApp()
 	a.Use(func(h Handler) Handler { return h })
-	r.Len(a.Middleware.stack, 4)
+	r.Len(a.Middleware.stack, 5)
 
 	g := a.Group("/api/v1")
-	r.Len(a.Middleware.stack, 4)
-	r.Len(g.Middleware.stack, 4)
+	r.Len(a.Middleware.stack, 5)
+	r.Len(g.Middleware.stack, 5)
 
 	g.Use(func(h Handler) Handler { return h })
-	r.Len(a.Middleware.stack, 4)
-	r.Len(g.Middleware.stack, 5)
+	r.Len(a.Middleware.stack, 5)
+	r.Len(g.Middleware.stack, 6)
 }
 
 func Test_Router_Redirect(t *testing.T) {