`404`s with parameter type `path`

[wochinge]

Hi y'all,

we have just upgraded from sanic==20.3.0 to sanic==21.9.1 with sanic-routing==0.7.1. Our test suite now shows 404s and 405s where we didn't expect them.

We run into this with Python 3.7, 3.8 and 3.9 on Windows and Ubuntu.

Minimal Example

from sanic import Sanic
from sanic.response import text

app = Sanic("MyHelloWorldApp")

@app.get("/conversation/<conversation_id:path>/story")
async def story(request, conversation_id):
    return text("story")

@app.put("/conversation/<conversation_id:path>/tracker/events")
async def put_events(request, conversation_id):
    return text("put events")

@app.post("/conversation/<conversation_id:path>/tracker/events")
async def post_events(request, conversation_id):
    return text("post events")

How to reproduce If you do a curl http://127.0.0.1:8000/conversation/dasda-dasd/story, this will result in a 404 (instead of the expected "story") If you comment out put_events or post_events it works fine.

I believe this is somewhat related to https://github.com/sanic-org/sanic/issues/2130

Appreciate any help as this is currently blocking us to upgrade 🙌🏻

[wochinge]

It seems this was introduced in 0.7.1. Can't reproduce with 0.7.0

[ahopkins]

I see the problem. You are indeed correct this is a :bug:

I do not think it will be a difficult fix, but I am also not entirely sure of a good workaround yet. I will try to get an 0.7.2 out this weekend.

Thank you for providing the simple, easy to reproduce test case. This makes it very simple for me to narrow down the issue, and provides us with a pattern for regression testing.

[wochinge]

Thanks for jumping so quickly on this! This is really great community support! 🙌🏻

Thank you for providing the simple, easy to reproduce test case.

haha 😆 I'm often on the other side at Rasa so I know the pain of a maintainer 😄

[ahopkins]

As way of explanation of what is happening... this is a portion of the generated router with my own annotations:

    # This section only appears RIGHT BEFORE a grouped set of routes. 
    # In this case, since the same route has a POST and PUT, it helps direct to the correct one
    if method in frozenset({'PUT'}):
        route_idx = 0
    elif method in frozenset({'POST'}):
        route_idx = 1
    else:
        raise NoMethod

    # Matching for "/conversation/<conversation_id:path>/tracker/events"
    match = router.matchers[0].match(path)
    if match:
        basket['__params__'] = match.groupdict()
        return router.regex_routes[('conversation', '<__dynamic__:path>', 'tracker', 'events')][route_idx], basket

    # Matching for "/conversation/<conversation_id:path>/story"
    match = router.matchers[1].match(path)
    if match:
        basket['__params__'] = match.groupdict()
        return router.regex_routes[('conversation', '<__dynamic__:path>', 'story')][0], basket
    raise NotFound

So, the solution is probably to move the method checking inside the match conditional.

[ahopkins]

haha laughing I'm often on the other side at Rasa so I know the pain of a maintainer smile

Nice to finally "meet" you

[ahopkins]

Here's the diff of the fix. If you have a chance to make the PR. If not, I can get to it later tonight.

diff --git a/sanic_routing/router.py b/sanic_routing/router.py
index 8e964d6..210927d 100644
--- a/sanic_routing/router.py
+++ b/sanic_routing/router.py
@@ -424,9 +424,10 @@ class BaseRouter(ABC):
             )
             route_idx: t.Union[str, int] = 0

+            holder = []
             if len(group.routes) > 1:
                 route_idx = "route_idx"
-                Node._inject_method_check(src, 1, group)
+                Node._inject_method_check(holder, 2, group)

             src.extend(
                 [
@@ -438,6 +439,7 @@ class BaseRouter(ABC):
                         1,
                     ),
                     Line("if match:", 1),
+                    *holder,
                     Line("basket['__params__'] = match.groupdict()", 2),
                     Line(
                         (

[wochinge]

You're my hero! I'll try it out!

[wochinge]

Sweet - it works. I'll create a PR - just need to write a test

[ahopkins]

Released: https://pypi.org/project/sanic-routing/0.7.2/

[wochinge]

Thank you so much! This was really outstanding community support! 🙌🏻 Keep up the great work! 🚀

[ahopkins]

It helps that it was a simple fix :laughing: