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

Crash on starlette middleware in mount endpoints (StaticFiles) #1131

Closed
old-syniex opened this issue May 12, 2021 · 7 comments · Fixed by #1137
Closed

Crash on starlette middleware in mount endpoints (StaticFiles) #1131

old-syniex opened this issue May 12, 2021 · 7 comments · Fixed by #1137

Comments

@old-syniex
Copy link

old-syniex commented May 12, 2021

Describe the bug: Elastic middleware crash on access to static files

Environment (please complete the following information)

  • OS: [e.g. Linux]
  • Python version: 3.9.4
  • Framework and version [e.g. Django 2.1]: FastAPI
  • APM Server version: 7.12.1
  • Agent version: 7.12.1
from __future__ import annotations

import uvicorn
from elasticapm.contrib.starlette import ElasticAPM, make_apm_client
from fastapi import FastAPI
from fastapi.staticfiles import StaticFiles

app = FastAPI(routes=[])
apm_config = {
    "SERVICE_NAME": "testing",
    "ENABLED": True,
    "SERVER_URL": "http://localhost:8200",
    "CAPTURE_HEADERS": True,
    "CAPTURE_BODY": "all",
}
apm = make_apm_client(apm_config)
app.add_middleware(ElasticAPM, client=apm)
app.mount("/static",StaticFiles(directory="/"),name="static")


if __name__ == "__main__":
    uvicorn.run(app, host="0.0.0.0", debug=False)

The issue is happens when elastic running the function _get_route_name.
the middleware will try to loop over the routes where starlette will return None in some cases:

  File "/home/syniex/.local/lib/python3.9/site-packages/elasticapm/contrib/starlette/__init__.py", line 233, in _get_route_name
    for route in routes:
                 └ None

TypeError: 'NoneType' object is not iterable

Starlette code that makes the issue:

@property
    def routes(self) -> typing.List[BaseRoute]:
        return getattr(self.app, "routes", None)
@old-syniex
Copy link
Author

Starlette#1182

@acnebs
Copy link

acnebs commented May 18, 2021

Seems like this was introduced in this commit: a5e6660

@Sparkycz
Copy link
Contributor

https://github.com/elastic/apm-agent-python/blob/master/elasticapm/contrib/starlette/__init__.py#L238
if isinstance(route, Mount): -> if isinstance(route, Mount) and route.routes: and add tests.

Sparkycz added a commit to Sparkycz/apm-agent-python that referenced this issue May 26, 2021
Sparkycz added a commit to Sparkycz/apm-agent-python that referenced this issue May 26, 2021
@nourspace
Copy link

I get the same error when mounting a prometheus's ASGI app to /metrics endpoint. The app has no routes as it is a simple function that returns some payload.

@bjoern-meier-by
Copy link

Hi, we just hit this issue as well when mounting static files for e.g. the swagger UI and would be interested if the work on the the open PR is still ongoing as the apm client now breaks our app and we did not find a workaround so far.

@bvader
Copy link

bvader commented Jun 30, 2021

Hiya, @Sparkycz Any Updates on this? I have a customer eagerly awaiting this as well...the PR seems stalled?

@basepi
Copy link
Contributor

basepi commented Jun 30, 2021

The PR is waiting on me. We have some upstream test failures caused by the new version of aiopg, which I'm working to fix here: #1178

Once that's merged we should be able to get #1137 passing and merged. Thanks for your patience.

basepi added a commit that referenced this issue Jun 30, 2021
* Fix Starlette middleware - providing only static files

closes #1131

* Fix failing tests

* CHANGELOG

Co-authored-by: Benjamin Wohlwend <beni@elastic.co>
Co-authored-by: Colton Myers <colton.myers@gmail.com>
beniwohli added a commit to beniwohli/apm-agent-python that referenced this issue Sep 14, 2021
* Fix Starlette middleware - providing only static files

closes elastic#1131

* Fix failing tests

* CHANGELOG

Co-authored-by: Benjamin Wohlwend <beni@elastic.co>
Co-authored-by: Colton Myers <colton.myers@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants