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

SentryAsgiMiddleware not compatible with Uvicorn 0.13.0 #947

Closed
LanderMoerkerke opened this issue Dec 9, 2020 · 33 comments
Closed

SentryAsgiMiddleware not compatible with Uvicorn 0.13.0 #947

LanderMoerkerke opened this issue Dec 9, 2020 · 33 comments
Labels
Type: Bug Something isn't working

Comments

@LanderMoerkerke
Copy link

On December 8th Uvicorn updated from 0.12.3 to 0.13.0. This results in an error at startup, see output with minimal example. When downgrading Uvicorn to 0.12.3 the example runs fine.

Why this error is thrown or which changes resulted in the error, I have no clue. Could you help me with this?

app.py

from sanic import Sanic
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware

app = SentryAsgiMiddleware(Sanic(__name__))

requirements.txt

sanic==20.9.1
sentry-sdk==0.19.4
uvicorn==0.13.0

Command to run:

uvicorn app:app --port 5000 --workers=1 --debug --reload

Output:

INFO:     Uvicorn running on http://127.0.0.1:5000 (Press CTRL+C to quit)
INFO:     Started reloader process [614068] using statreload
Process SpawnProcess-1:
Traceback (most recent call last):
  File "/usr/lib/python3.9/multiprocessing/process.py", line 315, in _bootstrap
    self.run()
  File "/usr/lib/python3.9/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/home/lander/.local/share/virtualenvs/san-iJ5wdX60/lib/python3.9/site-packages/uvicorn/subprocess.py", line 61, in subprocess_started
    target(sockets=sockets)
  File "/home/lander/.local/share/virtualenvs/san-iJ5wdX60/lib/python3.9/site-packages/uvicorn/server.py", line 48, in run
    loop.run_until_complete(self.serve(sockets=sockets))
  File "uvloop/loop.pyx", line 1456, in uvloop.loop.Loop.run_until_complete
  File "/home/lander/.local/share/virtualenvs/san-iJ5wdX60/lib/python3.9/site-packages/uvicorn/server.py", line 55, in serve
    config.load()
  File "/home/lander/.local/share/virtualenvs/san-iJ5wdX60/lib/python3.9/site-packages/uvicorn/config.py", line 319, in load
    elif not inspect.signature(self.loaded_app).parameters:
  File "/usr/lib/python3.9/inspect.py", line 3118, in signature
    return Signature.from_callable(obj, follow_wrapped=follow_wrapped)
  File "/usr/lib/python3.9/inspect.py", line 2867, in from_callable
    return _signature_from_callable(obj, sigcls=cls,
  File "/usr/lib/python3.9/inspect.py", line 2409, in _signature_from_callable
    sig = _signature_from_callable(
  File "/usr/lib/python3.9/inspect.py", line 2242, in _signature_from_callable
    raise TypeError('{!r} is not a callable object'.format(obj))
TypeError: <member '__call__' of 'SentryAsgiMiddleware' objects> is not a callable object
@untitaker
Copy link
Member

Does Sanic expose asgi-2 interface or asgi-3? Our detection may be off by a bit. I would append ._run_asgi2 or ._run_asgi3 to that last line and see which one works:

app = SentryAsgiMiddleware(Sanic(__name__))._run_asgi2
app = SentryAsgiMiddleware(Sanic(__name__))._run_asgi3

@LanderMoerkerke
Copy link
Author

@untitaker thanks for the reply!

Both of them seem to successfully start up, a log is presented that the ASGI 'lifespan' protocol is not supported:

INFO:     ASGI 'lifespan' protocol appears unsupported.

Once I try to create request:

from sanic import Sanic
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware


def create_app():
    app = Sanic()

    @app.get("/")
    def root(request):
        return "OK"

    return app


app = SentryAsgiMiddleware(create_app())._run_asgi3

I get the following error / logs:

INFO:     Uvicorn running on http://127.0.0.1:5000 (Press CTRL+C to quit)
INFO:     Started reloader process [49384] using statreload
INFO:     Started server process [49386]
INFO:     Waiting for application startup.
INFO:     ASGI 'lifespan' protocol appears unsupported.
INFO:     Application startup complete.
INFO:     127.0.0.1:35010 - "GET / HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/home/lander/.local/share/virtualenvs/sanic-uvicorn-XMPCd-qp/lib/python3.9/site-packages/uvicorn/protocols/http/httptools_impl.py", line 396, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/home/lander/.local/share/virtualenvs/sanic-uvicorn-XMPCd-qp/lib/python3.9/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "/home/lander/.local/share/virtualenvs/sanic-uvicorn-XMPCd-qp/lib/python3.9/site-packages/uvicorn/middleware/debug.py", line 96, in __call__
    raise exc from None
  File "/home/lander/.local/share/virtualenvs/sanic-uvicorn-XMPCd-qp/lib/python3.9/site-packages/uvicorn/middleware/debug.py", line 78, in __call__
    await self.app(scope, receive, inner_send)
  File "/home/lander/.local/share/virtualenvs/sanic-uvicorn-XMPCd-qp/lib/python3.9/site-packages/uvicorn/middleware/asgi2.py", line 6, in __call__
    instance = self.app(scope)
TypeError: _run_asgi3() missing 2 required positional arguments: 'receive' and 'send'

@untitaker
Copy link
Member

Right, then you need _run_asgi2.

@LanderMoerkerke
Copy link
Author

Both the _run_asgi2 and the _run_asgi3 raise the same exception when requesting the webserver. Is there something I need to add?

I tried editing the __call__ variable of SentryAsgiMiddleware in the source code to setup the callback as _run_asgi3 or as _run_asgi2. Once I run the application with Uvicorn, it crashes at startup, with the initial issue of a not callable object.

@untitaker
Copy link
Member

Was able to get it running with .run_asgi3 and --interface asgi3 passed as CLI flag.

I will keep this issue open but I think we may just drop ASGI 2 support instead of actually fixing this issue. There's too much inspection magic going on inside of the SDK and inside of uvicorn to figure out with which arguments to call the app.

@untitaker untitaker added the Type: Bug Something isn't working label Dec 10, 2020
@LanderMoerkerke
Copy link
Author

Great, works indeed. Thanks!

@LanderMoerkerke LanderMoerkerke changed the title SentryAsgiMiddleware not compatible with Uvicorn 0.14.0 SentryAsgiMiddleware not compatible with Uvicorn 0.13.0 Dec 15, 2020
@kristjanvalur
Copy link

I am getting the same problem, but this time with gunicorn api:app -w 1 -k uvicorn.workers.UvicornWorker
This is the recommended production environment for Starlette.
However, the gunicorn doesn't allow to select asgi2/asgi3 as uvicorn does, and so I get the "missing positional arguments" error, if I use either _run_asgi2 or _run_asgi3 on my Middleware.

Interestingly, I don't seem to need to actually call the middleware.
Just changing my file to this:

sentry_sdk.init(dsn=settings.SENTRY_DSN, traces_sample_rate=1.0)
dummy_app = SentryAsgiMiddleware(app)

seems to catch errors and report them, even if no-one is actually calling dummy_app. It is as though Sentry somehow hooks it self into the app....

@untitaker
Copy link
Member

@kristjanvalur then the middleware is not the one doing the error-capturing. But that's fine, its primary purpose is to attach request data to events. Error handling may be done via error logs as well.

@kristjanvalur
Copy link

I'm using FastAPI.
You are right, without instancing the middleware, I still get the errors delivered. It seems sufficient to init the sentry_sdk.
However, do you have any suggestion on how to make the Middleware work with a gunicorn/uvicorn.workers.UvicornWorker?

@untitaker
Copy link
Member

What exact error do you get? I wonder which version gunicorn attempts to use

@kristjanvalur
Copy link

This is how I invoke the server:

gunicorn api:app -w 1 -k uvicorn.workers.UvicornWorker

So, using app = SentryAsgiMiddleware(app), I get:

File "/usr/lib/python3.7/inspect.py", line 2208, in _signature_from_callable
    raise TypeError('{!r} is not a callable object'.format(obj))

and the server won't start up at all.

If I add the _run_asgi2 or _run_asgi3 property access to the Middleware, the servers tarts, but on the first request, I get respectively

TypeError: __call__() missing 2 required positional arguments: 'receive' and 'send'

and

TypeError: _run_asgi3() missing 2 required positional arguments: 'receive' and 'send'

@jjwwiodyne
Copy link

We're seeing this as well with the latest FastAPI + uvicorn:

sentry_sdk.init(dsn="https://<sentry_key_url>",
                    integrations=[AioHttpIntegration()],
                    release=f"<app_name>@<version>")
  • uvicorn[standard]==0.13.1
  • gunicorn==20.0.4
  • fastapi==0.62.0

@kristjanvalur
Copy link

I'm not a ASGI expert. I used to know WSGI. Is there a wrapper I can write to make the middleware callable without waiting for an upstream fix?

@untitaker
Copy link
Member

Hm. Okay here is a workaround that does not involve setting any server options:

class Asgi3(SentryAsgiMiddleware):
    async def __call__(self, *a, **kw):
        return await self._run_asgi3(*a, **kw)


app = Asgi3(create_app())

The problem really is that uvicorn tries to inspect method signatures too much to figure out what to call... it calls inspect.isfunction but _run_asgi3 is a method, so it returns false.

@kristjanvalur
Copy link

interesting. I'll mention this with the uvicorn devs. thanks for the help.

@untitaker
Copy link
Member

@kristjanvalur I don't think they can do much about it fwiw. It's just something we'll have to see through until asgi2 is dead.

@kristjanvalur
Copy link

Actually, the wrapper class above is problematic, because the parent class defines an instance attribute, __call__ that should cause the method to be hidden. That's what my linter says. but it fact it works.... somehow.
I'm using a simple wrapping closure instead:

def Wrap(middleware):
        async def wrapper(*args, **kwds):
            return await middleware(*args, **kwds)
        return wrapper

There is no need to explicitly call the _run_asgi3 method, signature detection of the middleware seems to work. It is only the code that is trying to analyze how to call the middleware (from uvicorn) that fails.

1 It fails to understand a call instance attribute which is a bound method.
2 it fails to understand a bound method directly.

Not sure if we should continue this, but it could help to point out that the SentryAsgiMiddleware has an unusal way of being callable and that it might need to be wrapped like above in certain "smart" frameworks.

@kristjanvalur
Copy link

@kristjanvalur I don't think they can do much about it fwiw. It's just something we'll have to see through until asgi2 is dead.

Well, if there is a way for uvicorn.workers.Uvicornworker to somehow accept arguments. or have a subclass that just assumes protocol 3.

@untitaker
Copy link
Member

@kristjanvalur fair enough, so the fn body is actually irrelevant, yeah. I think the way forward is to drop asgi2 support, then the "unusual way" disappears

@florimondmanca
Copy link

florimondmanca commented Dec 26, 2020

Hey folks,

Could anyone using the Sentry ASGI middleware install Uvicorn from encode/uvicorn#914 as a quick confirmation that it fixes the problem seen here?

pip install "git+https://github.com/encode/uvicorn.git@fm/factory-inspection#egg=uvicorn"

@sevaho
Copy link

sevaho commented Dec 26, 2020

@florimondmanca works for me!

@florimondmanca
Copy link

Sweet. :)

@untitaker
Copy link
Member

To be clear AFAICT the bug here is not related to determining ASGI 2 vs ASGI 3, but related to "app instances" vs "app factories"

Are y'all sure about this? I don't see how the middleware exposes itself as app factory.

@florimondmanca
Copy link

I don't see how the middleware exposes itself as app factory.

It doesn't, but due to the __call__ trick the code that Uvicorn was using to decide whether an app was a factory or not was failing. :-)

@jensenbox
Copy link

This is affecting Django users (me) as well as the above Sanic

@florimondmanca
Copy link

@jesenbox Feel free to try the latest state as referred to in #947 (comment) — should make it go away (you might have to install from master now, feel free to use a specific commit hash). We'll go ahead and issue a new release of Uvicorn with this patch soon.

@ricky-sb
Copy link

Are you still experiencing this error with the latest version of uvicorn? If so, are we good to close this issue?

@kristjanvalur
Copy link

It's fixed, from my end.

@adamantike
Copy link

I think this issue can be closed, after the implemented fixes.

@virajkanwade
Copy link

inspect.py

call = _signature_get_user_defined_method(type(obj), '__call__')

this is the problem line.

        if _looks_like_asgi3(app):
            self.__call__ = self._run_asgi3  # type: Callable[..., Any]
        else:
            self.__call__ = self._run_asgi2

sets the call attribute for the object instance and not the Class

Since SentryAsgiMiddleware.call is technically a member_descriptor and not a function, callable() says its not a callable function.
Blacksheep also suffers from the same issue when you do
app.middlewares.append(SentryAsgiMiddleware(app))

Can we do something to somehow set SentryAsgiMiddleware.__call__ = SentryAsgiMiddleware._run_asgi3/2?
I thing that should solve the issue.

@virajkanwade
Copy link

did a quick hack

    setattr(SentryAsgiMiddleware, '__call__', SentryAsgiMiddleware._run_asgi3)
    app.middlewares.append(SentryAsgiMiddleware(app))

and commented

        #if _looks_like_asgi3(app):
        #    self.__call__ = self._run_asgi3  # type: Callable[..., Any]
        #else:
        #    self.__call__ = self._run_asgi2

in SentryAsgiMiddleware.init

The app started properly with the middleware.

@virajkanwade
Copy link

virajkanwade commented May 20, 2022

sentry_asgi3.py

from sentry_sdk.integrations.asgi import (
    SentryAsgiMiddleware,
    TRANSACTION_STYLE_VALUES
)
from sentry_sdk.utils import (
    HAS_REAL_CONTEXTVARS,
    CONTEXTVARS_ERROR_MESSAGE
)


class SentryAsgi3Middleware(SentryAsgiMiddleware):

    # copy of SentryAsgiMiddleware.__init__ except the dynamic __call__ association
    def __init__(self, app, unsafe_context_data=False, transaction_style="endpoint"):
        # type: (Any, bool, str) -> None
        """
        Instrument an ASGI application with Sentry. Provides HTTP/websocket
        data to sent events and basic handling for exceptions bubbling up
        through the middleware.

        :param unsafe_context_data: Disable errors when a proper contextvars installation could not be found. We do not recommend changing this from the default.
        """

        if not unsafe_context_data and not HAS_REAL_CONTEXTVARS:
            # We better have contextvars or we're going to leak state between
            # requests.
            raise RuntimeError(
                "The ASGI middleware for Sentry requires Python 3.7+ "
                "or the aiocontextvars package." + CONTEXTVARS_ERROR_MESSAGE
            )
        if transaction_style not in TRANSACTION_STYLE_VALUES:
            raise ValueError(
                "Invalid value for transaction_style: %s (must be in %s)"
                % (transaction_style, TRANSACTION_STYLE_VALUES)
            )
        self.transaction_style = transaction_style
        self.app = app

    async def __call__(self, scope, receive, send):
        # type: (Any, Any, Any) -> Any
        return await self._run_asgi3(scope, receive, send)

app.middlewares.append(SentryAsgi3Middleware(app))

seems to be working without errors

@virajkanwade
Copy link

virajkanwade commented May 20, 2022

Arggh, my bad. Missed comment #947 (comment) which already does this! Wasted quite some time on this investigation!

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
None yet
Development

No branches or pull requests

10 participants