fix(router): walk parent tree for RouteNotFound handler in groups

[lyydsheep]

Summary

Fixes #2485

When a route exists in the router but the HTTP method is not allowed, the router previously always fell back to methodNotAllowedHandler. However, this bypassed any RouteNotFound handler registered at a parent or root group level.

Root Cause

In DefaultRouter.Route(), when currentNode.isHandler is true (path matched but method not), the code immediately set rInfo = methodNotAllowedRouteInfo without checking if any parent node had a notFoundHandler registered.

Fix

Traverse the parent node chain to look for a notFoundHandler. If found, use it; otherwise fall back to the existing methodNotAllowedHandler behavior.

Test

Existing route tests pass. The fix ensures RouteNotFound handlers registered at a group level are properly invoked for sub-paths.

Signed-off-by: lyydsheep 2230561977@qq.com

[aldas]

Tests are missing. Please add at least one testcase with example routing tree we can review this.

[lyydsheep]

Thanks for the feedback! I have added TestRouterRouteNotFoundHandlerInGroup in router_test.go covering the following scenarios:

1. Root-level RouteNotFound fires for a group sub-path when the method is not registered
2. Group-level RouteNotFound fires for a group sub-path when the method is not registered
3. Root-level handler fires when no group-level handler is defined
4. Falls back to methodNotAllowedHandler when no RouteNotFound handler is registered at any level

The test tree registers a /api group with GET /users and GET /items/:id, then issues requests with unsupported methods (POST/DELETE) to confirm the correct handler is selected based on where RouteNotFound is registered.

[aldas]

When I run these tests on unpatched Echo (v5.0.4) they all pass - meaning they do not test the feature in this PR. I would expect at least some of them to fails in unpatched version.

=== RUN   TestRouterRouteNotFoundHandlerInGroup
=== RUN   TestRouterRouteNotFoundHandlerInGroup/root_RouteNotFound_handler_fires_for_group_sub-path_with_wrong_method
--- PASS: TestRouterRouteNotFoundHandlerInGroup/root_RouteNotFound_handler_fires_for_group_sub-path_with_wrong_method (0.00s)
=== RUN   TestRouterRouteNotFoundHandlerInGroup/group_RouteNotFound_handler_fires_for_group_sub-path_with_wrong_method
--- PASS: TestRouterRouteNotFoundHandlerInGroup/group_RouteNotFound_handler_fires_for_group_sub-path_with_wrong_method (0.00s)
=== RUN   TestRouterRouteNotFoundHandlerInGroup/root_RouteNotFound_fires_when_no_group-level_handler_defined
--- PASS: TestRouterRouteNotFoundHandlerInGroup/root_RouteNotFound_fires_when_no_group-level_handler_defined (0.00s)
=== RUN   TestRouterRouteNotFoundHandlerInGroup/fallback_to_methodNotAllowed_when_no_RouteNotFound_handler_defined
--- PASS: TestRouterRouteNotFoundHandlerInGroup/fallback_to_methodNotAllowed_when_no_RouteNotFound_handler_defined (0.00s)
--- PASS: TestRouterRouteNotFoundHandlerInGroup (0.00s)
PASS

[lyydsheep]

Thank you for the thorough review, @aldas!

You are absolutely right — the previous tests used RouteNotFound("/*") which creates an anyKind wildcard node. The router's existing backtracking algorithm already finds that node without the parent-walk fix, so the tests passed on unpatched v5.0.4.

The fix is only exercised when RouteNotFound is registered on an exact prefix (no wildcard suffix), e.g. e.RouteNotFound("/api", ...) or g.RouteNotFound("", ...). In that case there is no anyKind child to backtrack into, and the router must walk the parent chain of the matched node to discover the handler.

I have updated the tests accordingly:

• Use e.RouteNotFound("/api") / g.RouteNotFound("") instead of "/*"
• Added explanatory comments describing why the wildcard variant is insufficient
• All four scenarios (root handler, group handler, param route, 405 fallback) are retained

These revised tests should fail on unpatched v5.0.4 and pass with the fix applied.

[aldas]

Where this requirement comes from

When a route exists in the router but the HTTP method is not allowed, the router previously always fell back to methodNotAllowedHandler. However, this bypassed any RouteNotFound handler registered at a parent or root group level.

In your examples you are setting up GET /api/users route.
and expect the response for POST /api/users to be 404, but the router resolves it to 405.

is not this behavior expected:

• when there is GET /api/users route and you request with any other method for that same path /api/users the router should indicate with status code 405 and Allow header to indicate that there seems to be a mistka and there actually exists routes for that path but for other methods.

NB: please do not use LLM and think this though by yourself.

[lyydsheep]

After thorough testing, I found that Echo v5 does not exhibit the bug described in issue #2485.

The original issue was reported against echo v4.11.1. Echo v5 has already addressed this in its routing implementation — POST /v0/foo (path does not exist, group has middleware) correctly falls back to the root RouteNotFound handler in v5.

I apologize for the noise. Closing this PR as it is not needed for v5.