chi.Walk missing middlewares

[ebabani]

Hi,

I was trying to use chi.Walk to get a report of all routes and the middlewares used by every route. I noticed that some of the middlewares were missing from some of the nested routes when using groups.

Example routes:

	r := chi.NewRouter()

	r.Use(middleware.RequestID)

	r.Get("/A", func(w http.ResponseWriter, r *http.Request) {})

	r.Group(func(r chi.Router) {
		r.Use(middleware.Timeout(2500 * time.Millisecond))

		r.Get("/B", func(w http.ResponseWriter, r *http.Request) {})

		r.Route("/C", func(r chi.Router) {

			// Walk on the below route does not show the Timeout middleware
			r.Get("/D", func(w http.ResponseWriter, r *http.Request) {})
		})
	})

In the example above, I expected the C/D route to have both RequestId and Timeout middlewares. but chi.Walk only shows RequestID.

It's possible I'm missing something here and this is behaving as expected.

Go playground link: https://go.dev/play/p/HKh7cA35Bgx

[VojtechVitek]

I guess the issue is that r.Route() creates a new sub-router (unlike r.Group() or r.Handle()) and Walk() doesn't reach out to its middleware stack.

https:/go-chi/chi/blob/v0.9.0/mux.go#L169

[ebabani]

It does seem related to the Group to me. I updated the example with some nested routes, and for those it shows the correct middleware chain.

https://go.dev/play/p/fia_kU8Lzqs

[pkieltyka]

Group() is used for inlining middleware, where as Rotue() mount a subrouter -- it's a subtle difference, but there is one. The middlewares however will be there along the C/D route.

If I recall -- the inline middleware stack is computed and reduced in memory, and we don't keep around in memory the middleware stack, which is why we can't walk it. We could definitely add it, it just means a bit more code. It would be safe to keep, but would only be useful for walking like in this case.

[ebabani]

In terms of functionality all is good, it was just surprising behaviour when I hit this.

For context, I am using the walk function to write tests agains the router we're using. I was writing checks to ensure certain endpoints were always behind an auth middleware. I'm not super familiars with the chi internals, but if you can point me in the general area I can take a look and create a PR.

[ganicus]

In terms of functionality all is good, it was just surprising behaviour when I hit this.

For context, I am using the walk function to write tests agains the router we're using. I was writing checks to ensure certain endpoints were always behind an auth middleware. I'm not super familiars with the chi internals, but if you can point me in the general area I can take a look and create a PR.

@ebabani what did you end up doing for testing with Route() and Group()?

[ebabani]

Hey @ganicus,

We ended up not writing an explicit test for the middleware checks. We only have a handful of routes that don't have the auth middleware, and keeping that grouping separate from the main routing logic.

The only route tests we have are to make sure our api routes match our documented routes. We use https:/swaggo/swag for our swagger docs, and our route tests confirm that all the chi routes exactly match the swagger declared routes.