Skip to content

Commit

Permalink
RFC: Return an instance of *fiber.Error when no handler found (gofi…
Browse files Browse the repository at this point in the history
…ber#1847)

* Return an instance of `*fiber.Error` when no handler found

When a handler cannot be found for a given path, previously Fiber
would construct a plaintext response that cannot be modified.

This commit switches to returning a new instance of `*fiber.Error`
with identical error message so that users can customise the look
of their 404 pages.

Signed-off-by: AKP <tom@tdpain.net>

* Fix `Test_App_Next_Method`

This test was failing as the error returned by `c.Next()` that's
required to generate the correct 404 status code was not being
passed through the middleware and being silently ignored.

Signed-off-by: AKP <tom@tdpain.net>

* Fix `Test_Logger_All`

Signed-off-by: AKP <tom@tdpain.net>

* Fix `Test_Cache_WithHeadThenGet` test

As far as I can tell, this test is meant to check that a cached
HEAD request to a given endpoint does not return the cached
content to a GET request to the same endpoint, and the test has
been altered to correctly check for this.

Signed-off-by: AKP <tom@tdpain.net>
  • Loading branch information
codemicro authored and trim21 committed Aug 15, 2022
1 parent 458c712 commit 3bb7a54
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 10 deletions.
4 changes: 2 additions & 2 deletions app_test.go
Expand Up @@ -1089,9 +1089,9 @@ func Test_App_Next_Method(t *testing.T) {

app.Use(func(c *Ctx) error {
utils.AssertEqual(t, MethodGet, c.Method())
c.Next()
err := c.Next()
utils.AssertEqual(t, MethodGet, c.Method())
return nil
return err
})

resp, err := app.Test(httptest.NewRequest(MethodGet, "/", nil))
Expand Down
10 changes: 5 additions & 5 deletions middleware/cache/cache_test.go
Expand Up @@ -391,32 +391,32 @@ func Test_Cache_WithHead(t *testing.T) {
func Test_Cache_WithHeadThenGet(t *testing.T) {
app := fiber.New()
app.Use(New())
app.Get("/get", func(c *fiber.Ctx) error {
app.Get("/", func(c *fiber.Ctx) error {
return c.SendString(c.Query("cache"))
})

headResp, err := app.Test(httptest.NewRequest("HEAD", "/head?cache=123", nil))
headResp, err := app.Test(httptest.NewRequest("HEAD", "/?cache=123", nil))
utils.AssertEqual(t, nil, err)
headBody, err := ioutil.ReadAll(headResp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, "", string(headBody))
utils.AssertEqual(t, cacheMiss, headResp.Header.Get("X-Cache"))

headResp, err = app.Test(httptest.NewRequest("HEAD", "/head?cache=123", nil))
headResp, err = app.Test(httptest.NewRequest("HEAD", "/?cache=123", nil))
utils.AssertEqual(t, nil, err)
headBody, err = ioutil.ReadAll(headResp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, "", string(headBody))
utils.AssertEqual(t, cacheHit, headResp.Header.Get("X-Cache"))

getResp, err := app.Test(httptest.NewRequest("GET", "/get?cache=123", nil))
getResp, err := app.Test(httptest.NewRequest("GET", "/?cache=123", nil))
utils.AssertEqual(t, nil, err)
getBody, err := ioutil.ReadAll(getResp.Body)
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, "123", string(getBody))
utils.AssertEqual(t, cacheMiss, getResp.Header.Get("X-Cache"))

getResp, err = app.Test(httptest.NewRequest("GET", "/get?cache=123", nil))
getResp, err = app.Test(httptest.NewRequest("GET", "/?cache=123", nil))
utils.AssertEqual(t, nil, err)
getBody, err = ioutil.ReadAll(getResp.Body)
utils.AssertEqual(t, nil, err)
Expand Down
2 changes: 1 addition & 1 deletion middleware/logger/logger_test.go
Expand Up @@ -148,7 +148,7 @@ func Test_Logger_All(t *testing.T) {
utils.AssertEqual(t, nil, err)
utils.AssertEqual(t, fiber.StatusNotFound, resp.StatusCode)

expected := fmt.Sprintf("%dHost=example.comhttp0.0.0.0example.com/?foo=bar/%s%s%s%s%s%s%s%s%s-", os.Getpid(), cBlack, cRed, cGreen, cYellow, cBlue, cMagenta, cCyan, cWhite, cReset)
expected := fmt.Sprintf("%dHost=example.comhttp0.0.0.0example.com/?foo=bar/%s%s%s%s%s%s%s%s%sCannot GET /", os.Getpid(), cBlack, cRed, cGreen, cYellow, cBlue, cMagenta, cCyan, cWhite, cReset)
utils.AssertEqual(t, expected, buf.String())
}

Expand Down
3 changes: 1 addition & 2 deletions router.go
Expand Up @@ -134,8 +134,7 @@ func (app *App) next(c *Ctx) (match bool, err error) {
}

// If c.Next() does not match, return 404
_ = c.SendStatus(StatusNotFound)
_ = c.SendString("Cannot " + c.method + " " + c.pathOriginal)
err = NewError(StatusNotFound, "Cannot " + c.method + " " + c.pathOriginal)

// If no match, scan stack again if other methods match the request
// Moved from app.handler because middleware may break the route chain
Expand Down

0 comments on commit 3bb7a54

Please sign in to comment.