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

ensure_integration_enabled_async doesn't work sometimes #2892

Open
sentrivana opened this issue Mar 22, 2024 · 2 comments
Open

ensure_integration_enabled_async doesn't work sometimes #2892

sentrivana opened this issue Mar 22, 2024 · 2 comments
Labels
Type: Bug Something isn't working

Comments

@sentrivana
Copy link
Contributor

sentrivana commented Mar 22, 2024

Two different ways in which the decorator seems to alter the behavior of the decorated function. Unsure whether it's the same issue or two different ones.

1. Repro via a simple Starlette app

Install sentry-sdk==2.0.0, uvicorn, starlette.

Create a simple Starlette app as app.py:

import sentry_sdk
from starlette.applications import Starlette

sentry_sdk.init()

app = Starlette()

Run with uvicorn app:app.

Request http://127.0.0.1:8000/ (or wherever your server is running). We didn't define any routes so it's not expected to resolve -- you should get a 404. Instead you'll get a 500:

ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/ivana/dev/repro/starlette-uvicorn/starlette.env/lib/python3.12/site-packages/uvicorn/protocols/http/h11_impl.py", line 407, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ivana/dev/repro/starlette-uvicorn/starlette.env/lib/python3.12/site-packages/uvicorn/middleware/proxy_headers.py", line 69, in __call__
    return await self.app(scope, receive, send)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ivana/dev/repro/starlette-uvicorn/starlette.env/lib/python3.12/site-packages/uvicorn/middleware/asgi2.py", line 14, in __call__
    instance = self.app(scope)
               ^^^^^^^^^^^^^^^
TypeError: 'coroutine' object is not callable

If you remove the ensure_integration_enabled_async decorator from the Starlette integration, rerun the server and request / again, you'll get a 404 as expected.

2. Repro via tests

  1. Open the Starlite integration.
  2. Decorate the _create_span_call wrapper with the ensure_integration_enabled_async decorator.
  3. Remove the now unnecessary check in the wrapper body here.
  4. Run the Starlite test suite.

The tests will fail, even though the code should behave the same way as before the change.

This can also be observed with Starlette in CI on this PR: these are the failing tests, and this change makes the tests green.

I've observed this in other integrations as well, so it appears to be a problem with the decorator itself. The sync version of the decorator works without issues.

@sentrivana sentrivana added the Type: Bug Something isn't working label Mar 22, 2024
@sentrivana sentrivana changed the title Using the ensure_integration_enabled_async decorator makes tests fail ensure_integration_enabled_async doesn't work sometimes Apr 26, 2024
@antonpirker
Copy link
Member

What I just noticed. In our starlette integration when using the decorator on _sentry_patched_asgi_app the error occurs. In the stacktrace above it is in asgi2.py in uvicorn. And our integration calls our internal _run_asgi3. So maybe it has something to do with ASGI 2 and 3 protocoll differences.

@jonathanberthias
Copy link

In my investigation, I found that the issue comes from this part in uvicorn:

https://github.com/encode/uvicorn/blob/0efd3835da6dcc713f74aadf7b52779d0d1fa17d/uvicorn/config.py#L439-L459

Basically, it tries to call the "app" (which can be an app factory at this point) with no arguments, and it expects the call to fail if you have an app and not a factory. However, the patched app.__call__ succeeds with no arguments, which leads uvicorn to think it was given an app factory instead of an app (hence the WARNING: ASGI app factory detected. Using it, but please consider setting the --factory flag explicitly. in #3021).

Then, the inspect.isfunction(self.loaded_app) returns false though I wasn't able to pinpoint why at the moment. This leads to uvicorn calling the app with ASGI2.

Hope this helps a little!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

4 participants