Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recorded path is missing path prefix for mounted sub-applications #79

Closed
jdemaeyer opened this issue Dec 31, 2024 · 5 comments · Fixed by #80
Closed

Recorded path is missing path prefix for mounted sub-applications #79

jdemaeyer opened this issue Dec 31, 2024 · 5 comments · Fixed by #80
Assignees

Comments

@jdemaeyer
Copy link

Hi Simon, thanks so much for this great app!

I have a Starlette app that mounts three subapps, each under its own prefix, so that I can get three separate OpenAPI specs while still sharing some resources between the three sub-apps.

While all routes were correctly (i.e. including their subapp prefixes) shown in the Apitally dashboard on the first run, when I make an actual request, it is not counted towards one of these routes, but instead a new route without the subapp prefix is created.

Here's some minimal code to reproduce:

from starlette.applications import Starlette
from starlette.responses import JSONResponse
from starlette.routing import Match, Mount, Route


# XXX: This function copied from
#      https://github.com/apitally/apitally-py/blob/main/apitally/starlette.py#L226-L231

def get_path(request):
    for route in request.app.routes:
        match, _ = route.matches(request.scope)
        if match == Match.FULL:
            return route.path
    return None


async def subapp1_route(request):
    return JSONResponse({
        'subapp': 1,
        'path': get_path(request),
        'uid': request.path_params['uid'],
    })


async def subapp2_route(request):
    return JSONResponse({
        'subapp': 2,
        'path': get_path(request),
        'uid': request.path_params['uid'],
    })


async def root(request):
    return JSONResponse({
        'subapp': None,
        'path': get_path(request),
    })


subapp1 = Starlette(routes=[Route('/route/{uid}', subapp1_route)])
subapp2 = Starlette(routes=[Route('/route/{uid}', subapp2_route)])


app = Starlette(
    routes=[
        Route('/', root),
        Mount('/subapp1', subapp1),
        Mount('/subapp2', subapp2),
    ],
)

When opening /subapp1/route/123, I would expect to get /subapp1/route/{uid} as path, but instead I'm getting /route/{uid}. (Which of course makes sense, since request.app will be the subapp, which has no clue it's being mounted under some prefix.)

I've worked around this locally by changing

            return route.path

to

            return request.scope['root_path'] + route.path

But I'm unsure if that's adequate for all use cases.

PS: While trying around I also noticed that even without subapps, path detection breaks down when using Mount like in this example from the Starlette docs

@itssimon
Copy link
Member

Thanks so much for reporting this. I'll look into it and aim to have a fix shortly.

@itssimon
Copy link
Member

itssimon commented Jan 1, 2025

PS: While trying around I also noticed that even without subapps, path detection breaks down when using Mount like in this example from the Starlette docs

This is also fixed in #81. Thanks for catching this!

@itssimon
Copy link
Member

itssimon commented Jan 1, 2025

v0.14.3 has been released with these fixes. Thanks again!

@jdemaeyer
Copy link
Author

Hi Simon - awesome, thank you so much and happy new year!

@itssimon
Copy link
Member

itssimon commented Jan 1, 2025

Frohes Neues zurück und Grüße ins Münsterland! Ausgewanderter Bielefelder hier :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants