Merge pull request #812 from ReneWerner87/improveErrorHandling

☠️ Improve error handling flow

app_test.go

@@ -180,6 +180,60 @@ func Test_App_ErrorHandler_Custom(t *testing.T) {
 	utils.AssertEqual(t, "hi, i'm an custom error", string(body))
 }
 
+func Test_App_ErrorHandler_HandlerStack(t *testing.T) {
+	app := New(Config{
+		ErrorHandler: func(c *Ctx, err error) error {
+			utils.AssertEqual(t, "1: USE error", err.Error())
+			return DefaultErrorHandler(c, err)
+		},
+	})
+	app.Use("/", func(c *Ctx) error {
+		err := c.Next() // call next USE
+		utils.AssertEqual(t, "2: USE error", err.Error())
+		return errors.New("1: USE error")
+	}, func(c *Ctx) error {
+		err := c.Next() // call [0] GET
+		utils.AssertEqual(t, "0: GET error", err.Error())
+		return errors.New("2: USE error")
+	})
+	app.Get("/", func(c *Ctx) error {
+		return errors.New("0: GET error")
+	})
+
+	resp, err := app.Test(httptest.NewRequest("GET", "/", nil))
+	utils.AssertEqual(t, nil, err, "app.Test(req)")
+	utils.AssertEqual(t, 500, resp.StatusCode, "Status code")
+
+	body, err := ioutil.ReadAll(resp.Body)
+	utils.AssertEqual(t, nil, err)
+	utils.AssertEqual(t, "1: USE error", string(body))
+}
+
+func Test_App_ErrorHandler_RouteStack(t *testing.T) {
+	app := New(Config{
+		ErrorHandler: func(c *Ctx, err error) error {
+			utils.AssertEqual(t, "1: USE error", err.Error())
+			return DefaultErrorHandler(c, err)
+		},
+	})
+	app.Use("/", func(c *Ctx) error {
+		err := c.Next()
+		utils.AssertEqual(t, "0: GET error", err.Error())
+		return errors.New("1: USE error") // [2] call ErrorHandler
+	})
+	app.Get("/test", func(c *Ctx) error {
+		return errors.New("0: GET error") // [1] return to USE
+	})
+
+	resp, err := app.Test(httptest.NewRequest("GET", "/test", nil))
+	utils.AssertEqual(t, nil, err, "app.Test(req)")
+	utils.AssertEqual(t, 500, resp.StatusCode, "Status code")
+
+	body, err := ioutil.ReadAll(resp.Body)
+	utils.AssertEqual(t, nil, err)
+	utils.AssertEqual(t, "1: USE error", string(body))
+}
+
 func Test_App_Nested_Params(t *testing.T) {
 	app := New()
 

ctx.go

@@ -599,17 +599,12 @@ func (c *Ctx) Next() (err error) {
 	// Did we executed all route handlers?
 	if c.indexHandler < len(c.route.Handlers) {
 		// Continue route stack
-		if err = c.route.Handlers[c.indexHandler](c); err != nil {
-			if err = c.app.config.ErrorHandler(c, err); err != nil {
-				_ = c.SendStatus(StatusInternalServerError)
-			}
-			return err
-		}
+		err = c.route.Handlers[c.indexHandler](c)
 	} else {
 		// Continue handler stack
 		_, err = c.app.next(c)
 	}
-	return
+	return err
 }
 
 // OriginalURL contains the original request URL.

middleware/logger/logger.go

@@ -173,7 +173,7 @@ func New(config ...Config) fiber.Handler {
 			start = time.Now()
 		}
 		// Handle request, store err for logging
-		err = c.Next()
+		handlerErr := c.Next()
 
 		// Set latency stop time
 		if cfg.haveLatency {
@@ -239,8 +239,8 @@ func New(config ...Config) fiber.Handler {
 			case TagReset:
 				return buf.WriteString(cReset)
 			case TagError:
-				if err != nil {
-					return buf.WriteString(err.Error())
+				if handlerErr != nil {
+					return buf.WriteString(handlerErr.Error())
 				}
 				return buf.WriteString("-")
 			default:
@@ -273,7 +273,7 @@ func New(config ...Config) fiber.Handler {
 		// Put buffer back to pool
 		bytebufferpool.Put(buf)
 
-		return nil
+		return handlerErr
 	}
 }
 

router.go

@@ -118,12 +118,8 @@ func (app *App) next(c *Ctx) (match bool, err error) {
 
 		// Execute first handler of route
 		c.indexHandler = 0
-		if err = route.Handlers[0](c); err != nil {
-			if catch := c.app.config.ErrorHandler(c, err); catch != nil {
-				_ = c.SendStatus(StatusInternalServerError)
-			}
-		}
-		return // Stop scanning the stack
+		err = route.Handlers[0](c)
+		return match, err // Stop scanning the stack
 	}
 
 	// If c.Next() does not match, return 404
@@ -152,7 +148,12 @@ func (app *App) handler(rctx *fasthttp.RequestCtx) {
 	}
 
 	// Find match in stack
-	match, _ := app.next(c)
+	match, err := app.next(c)
+	if err != nil {
+		if catch := c.app.config.ErrorHandler(c, err); catch != nil {
+			_ = c.SendStatus(StatusInternalServerError)
+		}
+	}
 	// Generate ETag if enabled
 	if match && app.config.ETag {
 		setETag(c, false)