🐛 [Bug]: Healthcheck middleware doesn't work with group about fiber HOT 17 CLOSED

@ReneWerner87 sure! I can check it out today and try to send a PR with the fix

from fiber.

Sure thx

from fiber.

Thanks for opening your first issue here! 🎉 Be sure to follow the issue template! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

from fiber.

@ulasakdeniz What happens if you do this instead:

group.Use("/livez", healthcheck.New())

from fiber.

group.Use() is expecting multiple params, and the middleware only returns a Fiber.Handler()

https://github.com/gofiber/fiber/blob/v2/group.go#L62

https://github.com/gofiber/fiber/blob/v2/middleware/healthcheck/healthcheck.go#L13

@ReneWerner87 Is this a limitation, bug, or possible feature we need to add?

from fiber.

@efectn Forgot to tag you :-)

from fiber.

@ulasakdeniz What happens if you do this instead:

group.Use("/livez", healthcheck.New())

Hi @gaby thanks for your answer. That also doesn't work.

Edit: My Go version: go version go1.21.4 darwin/amd64

from fiber.

from fiber.

@luk3skyw4lker can you help with a fix ?

from fiber.

@ReneWerner87 by what I've read right now the problem is that c.Path returns the full path (like /group/livez) and the check fails, so I should be reducing the c.Path to check if the last route is a /livez route. Is this right?

from fiber.

right, that would be one solution, a second one is to check only the suffix of the route, but that would be a not quite perfect one

from fiber.

https://github.com/gofiber/fiber/blob/main/router.go#L62
https://docs.gofiber.io/api/ctx#route

we should reduce it with the information of the current route

from fiber.

@ReneWerner87 I could only think about splitting the Path and checking the suffix, but that would remove any possbility of having any /[n+]/(livez | readyz) routes. But I think this could be a problem in the future, right?

from fiber.

someting like this but better

		switch c.Path()[len(c.Route().Path):] {
		case cfg.ReadinessEndpoint:
			return isReadyHandler(c)
		case cfg.LivenessEndpoint:
			return isLiveHandler(c)
		}

from fiber.

func Test_HealthCheck_Group_Default(t *testing.T) {
	t.Parallel()

	app := fiber.New()
	app.Group("/v1", New())
	v2 := app.Group("/v2/")
	customer := v2.Group("/customer/")
	customer.Use(New())

	req, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/readyz", nil))
	utils.AssertEqual(t, nil, err)
	utils.AssertEqual(t, fiber.StatusOK, req.StatusCode)

	req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v1/livez", nil))
	utils.AssertEqual(t, nil, err)
	utils.AssertEqual(t, fiber.StatusOK, req.StatusCode)

	req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/readyz", nil))
	utils.AssertEqual(t, nil, err)
	utils.AssertEqual(t, fiber.StatusOK, req.StatusCode)

	req, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/v2/customer/livez", nil))
	utils.AssertEqual(t, nil, err)
	utils.AssertEqual(t, fiber.StatusOK, req.StatusCode)
}

		prefixCount := len(utils.TrimRight(c.Route().Path, '/'))
		if len(c.Path()) >= prefixCount {
			switch c.Path()[prefixCount:] {
			case cfg.ReadinessEndpoint:
				return isReadyHandler(c)
			case cfg.LivenessEndpoint:
				return isLiveHandler(c)
			}
		}

can you make these adjustments and check them again
and also the benchmarks

perhaps we also need to test the strictRouting mode

and you find a better way to compare routes

from fiber.

@ReneWerner87 yep, I'll check them and write a test with StrictRouting mode too. Thanks for the help and heads up!

from fiber.

@ReneWerner87 I'll send a PR with the reports and results by Sunday, is that okay?

from fiber.