Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

☠️ Improve error handling flow #812

Merged
merged 1 commit into from
Sep 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 54 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
9 changes: 2 additions & 7 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions middleware/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,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 {
Expand Down Expand Up @@ -238,8 +238,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:
Expand Down Expand Up @@ -272,6 +272,6 @@ func New(config ...Config) fiber.Handler {
// Put buffer back to pool
bytebufferpool.Put(buf)

return nil
return handlerErr
}
}
15 changes: 8 additions & 7 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down