Bug: Sentry middleware doesn't catch errors

[vrslev]

• Set SENTRY_DSN environment variable
• Install sentry_dsk
• Run Starlite app:

import os

import sentry_sdk
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware
from starlite import Starlite, get


@get()
def index() -> None:
    raise Exception


sentry_sdk.init(os.environ["SENTRY_DSN"])
app = Starlite(route_handlers=[index], middleware=[SentryAsgiMiddleware])

Sentry didn't catch the Exception.

[Goldziher]

testing this. next time please add a test PR so we can easily identify the issue.

[Goldziher]

Ok, so I looked into this. IMO there is no issue. Look at this code from the middleware:

def _capture_exception(hub, exc, mechanism_type="asgi"):
    # type: (Hub, Any, str) -> None

    # Check client here as it might have been unset while streaming response
    if hub.client is not None:
        event, hint = event_from_exception(
            exc,
            client_options=hub.client.options,
            mechanism={"type": mechanism_type, "handled": False},
        )
        hub.capture_event(event, hint=hint)

And then where it might be invoked:

 with hub.start_transaction(
                        transaction, custom_sampling_context={"asgi_scope": scope}
                    ):
                        # XXX: Would be cool to have correct span status, but we
                        # would have to wrap send(). That is a bit hard to do with
                        # the current abstraction over ASGI 2/3.
                        try:
                            return await callback()
                        except Exception as exc:
                            _capture_exception(
                                hub, exc, mechanism_type=self.mechanism_type
                            )
                            raise exc from None

Basically an exception is captured only when its not handled, but it is a handled exception. The only kind of exception you might get here is from a middleware inside the middleware stack before this middleware, which might be unhandled. Sentry should still capture responses and response statuses correctly. So in my opinion this is a non-issue.

What do you think @vrslev ? also @cofin @peterschutt your input would be nice.

[peterschutt]

Knowing that Starlite wraps the route handlers in their own exception handling which doesn't let exceptions propagate back through the middleware stack, then yeh this is expected.

However, I'm pretty sure most would expect that an internal error such as this would be ingested by sentry, especially beginners.

I don't experience the issue due to the logging exception handler pattern:

def logging_exception_handler(request: Request, exc: Exception) -> Response:
    logger.error("Application Exception", exc_info=exc)
    return create_exception_response(exc)

... and the fact that sentry automatically ingests logs at ERROR or higher: https://docs.sentry.io/platforms/python/guides/logging/

[Goldziher]

Knowing that Starlite wraps the route handlers in their own exception handling which doesn't let exceptions propagate back through the middleware stack, then yeh this is expected.

However, I'm pretty sure most would expect that an internal error such as this would be ingested by sentry, especially beginners.

I don't experience the issue due to the logging exception handler pattern:

def logging_exception_handler(request: Request, exc: Exception) -> Response:
    logger.error("Application Exception", exc_info=exc)
    return create_exception_response(exc)

... and the fact that sentry automatically ingests logs at ERROR or higher: https://docs.sentry.io/platforms/python/guides/logging/

So what's your suggestion for resolving this?

[vrslev]

I agree with @peterschutt. For Starlite perspective, this behavior is expected, however users (for sure) expect that Sentry will catch server errors.

[peterschutt]

So what's your suggestion for resolving this?

from sentry_sdk.integrations.starlite import StarliteIntegration?

For context, specific starlette and fastapi integrations were merged 5 days ago (getsentry/sentry-python#1441 and getsentry/sentry-python#1495)

[peterschutt]

https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md#adding-a-new-integration-checklist

[Goldziher]

Ok, this is not a "bug" par se. Its incompatibility. The solution is to create a sentry integration - not in this repository. As such I created an issue in the Sentry repository, and am closing this one. See: getsentry/sentry-python#1549