From 3bb7a540c98b51ad78977b671171c78ac073ed6f Mon Sep 17 00:00:00 2001 From: akp Date: Tue, 5 Apr 2022 07:39:53 +0100 Subject: [PATCH] RFC: Return an instance of `*fiber.Error` when no handler found (#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 * 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 * Fix `Test_Logger_All` Signed-off-by: AKP * 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 --- app_test.go | 4 ++-- middleware/cache/cache_test.go | 10 +++++----- middleware/logger/logger_test.go | 2 +- router.go | 3 +-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app_test.go b/app_test.go index d7be6de391..178fdbea4e 100644 --- a/app_test.go +++ b/app_test.go @@ -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)) diff --git a/middleware/cache/cache_test.go b/middleware/cache/cache_test.go index 29ca9730f7..216d9cea27 100644 --- a/middleware/cache/cache_test.go +++ b/middleware/cache/cache_test.go @@ -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) diff --git a/middleware/logger/logger_test.go b/middleware/logger/logger_test.go index 3d564cfb19..0e0ba1d63c 100644 --- a/middleware/logger/logger_test.go +++ b/middleware/logger/logger_test.go @@ -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()) } diff --git a/router.go b/router.go index 53150e21c7..5e037da412 100644 --- a/router.go +++ b/router.go @@ -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