Skip to content

FastAPI integration ROOT_PATH problem #1832

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

Closed
sector119 opened this issue Jan 11, 2023 · 9 comments
Closed

FastAPI integration ROOT_PATH problem #1832

sector119 opened this issue Jan 11, 2023 · 9 comments

Comments

@sector119
Copy link

sector119 commented Jan 11, 2023

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.12.0

Steps to Reproduce

I have FastAPI app with nginx as proxy with following config. I host my api service on "/api/xmlrpc/v1" location and I rewrite it to send to my app on "/" location.

location /api/xmlrpc/v1 {
    proxy_set_header Host $http_host;
    proxy_set_header X-Real-IP $remote_addr;
    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header X-Forwarded-Port $server_port;
    proxy_set_header X-Forwarded-Proto $scheme;

    proxy_pass http://xmlrpc_api_service_upstream;
    rewrite ^/api/xmlrpc/v1/(.*)$ /$1 break;
}

To get FastAPI docs to work I must set FastAPI root_path argument to "/api/xmlrpc/v1" when I create app.

Here is request scope example
{'type': 'http', 'scheme': 'https', **'root_path': '/api/xmlrpc/v1'**, **'path': '/api/xmlrpc/v1'**}

Expected Result

What can I do to get url tag and curl example urls look as expected (https://api.my.site/api/xmlrpc/v1) ?

Actual Result

I get url tag like https://api.my.site/api/xmlrpc/v1/api/xmlrpc/v1 not https://api.my.site/api/xmlrpc/v1 as it has to be, as well as curl example.

api

That's happened because of asgi.py integration's _get_url() function where url path is
scope.get("root_path", "") + scope.get("path", "") that is why it got duplicated.

I can workaround it by setting FastAPI root_path=None and openapi_url='/api/xmlrpc/v1/openapi.json' and don't rewrite location for '/api/xmlrpc/v1/openapi.json' location while sending request to app from nginx, but it looks like a dirty hack.

Thank You

@smeubank
Copy link
Member

I understand then that the problem is nginx is kindof duplicating the api route, and you're wondering if we have a better work around to get the original FastAPI route?

@sector119
Copy link
Author

sector119 commented Jan 13, 2023

I understand then that the problem is nginx is kindof duplicating the api route, and you're wondering if we have a better work around to get the original FastAPI route?

Nginx doesn't duplicate it just rewrite url from "/api/xmlrpc/v1" to "/" while proxying request to upstream backend, but backend also get original url "/api/xmlrpc/v1" at scope["path"], but also FastAPI makes me set root_path to that original url "/api/xmlrpc/v1" to serve it's docs properly. And here we get duplicated path.

It's not a problem with nginx, but with asgi integration and _get_url() function IMO. It shouldn't concat root_path and path, or we have to redefine it by subclassing, I don't know..

So I don't wondering about a workaround. I think that I have to have a choice to prepend root_path to original path or not, because my backend app may or may not use root_path to generate route url. Right now it doesn't, only FastAPI use it to generate path to openapi.json correctly

@github-actions
Copy link

github-actions bot commented Feb 4, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@antonpirker
Copy link
Member

Hey @sector119

Now I get it. We need to support the FastAPI option root_path properly.
Note so self: Look here: https://fastapi.tiangolo.com/advanced/behind-a-proxy/?h=root_path

@sector119
Copy link
Author

Hey @sector119

Now I get it. We need to support the FastAPI option root_path properly. Note so self: Look here: https://fastapi.tiangolo.com/advanced/behind-a-proxy/?h=root_path

Yes, for sure !

@sentrivana sentrivana self-assigned this Jun 7, 2023
@sentrivana
Copy link
Contributor

Note to self: django/asgiref#229 (comment)

@sentrivana sentrivana added this to the Starlette/FastAPI Update milestone Jul 5, 2023
@antonpirker
Copy link
Member

Hey @sector119 !

I finally had time to test this. I tried to reproduce this with this sample project: https://github.com./antonpirker/test-fastapi-root-path

But there is no path duplication happening for me. Can you have a look at the demo repo I linked and tell me how your setup is different to mine?

Thanks!

@getsantry
Copy link

getsantry bot commented Aug 3, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Aug 3, 2023
@getsantry
Copy link

getsantry bot commented Aug 26, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Aug 26, 2023
@getsantry getsantry bot closed this as completed Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

5 participants