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

Some changes for fastapi bootstrapper #50

Merged
merged 5 commits into from
Dec 8, 2024
Merged

Conversation

lesnik512
Copy link
Contributor

@lesnik512 lesnik512 commented Dec 7, 2024

What's changed

  1. replace sentry FastApiIntegration with SentryAsgiMiddleware, because FastApiIntegration works really bad

@lesnik512 lesnik512 requested review from insani7y and vrslev December 7, 2024 17:34
@lesnik512 lesnik512 self-assigned this Dec 7, 2024
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
unittests 98.77% <100.00%> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
microbootstrap/bootstrappers/fastapi.py 100.00% <100.00%> (+2.89%) ⬆️
microbootstrap/instruments/base.py 96.96% <100.00%> (+2.85%) ⬆️

@lesnik512 lesnik512 requested a review from insani7y December 8, 2024 12:45
@vrslev
Copy link
Contributor

vrslev commented Dec 8, 2024

replace sentry FastApiIntegration with SentryAsgiMiddleware, because FastApiIntegration works really bad

What exactly works badly when you use FastApiIntegration? Should we use SentryAsgiMidleware for Litestar as well?

@lesnik512
Copy link
Contributor Author

lesnik512 commented Dec 8, 2024

It has some problems with Middlewares, raises errors. I'll post them tomorrow. It's only fastapi integration has problems, I think

@vrslev
Copy link
Contributor

vrslev commented Dec 8, 2024

Perhaps, the issue should be placed at https://github.com/getsentry/sentry-python/issues?

@vrslev vrslev merged commit e381371 into main Dec 8, 2024
15 checks passed
@vrslev vrslev deleted the feature/improve-fastapi branch December 8, 2024 17:14
@lesnik512
Copy link
Contributor Author

lesnik512 commented Dec 9, 2024

@vrslev it's not clear, what's happening. But here is the trace

Application callable raised an exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/granian/_futures.py", line 4, in future_watcher
    await inner(watcher.scope, watcher.proto)
  File "/usr/local/lib/python3.12/site-packages/fastapi/applications.py", line 1054, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/starlette.py", line 409, in _sentry_patched_asgi_app
    return await middleware(scope, receive, send)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/asgi.py", line 161, in _run_asgi3
    return await self._run_app(scope, receive, send, asgi_version=3)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/asgi.py", line 257, in _run_app
    return await self.app(
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/starlette/applications.py", line 123, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/starlette.py", line 200, in _create_span_call
    return await old_call(app, scope, new_receive, new_send, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/starlette.py", line 200, in _create_span_call
    return await old_call(app, scope, new_receive, new_send, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/prometheus_fastapi_instrumentator/middleware.py", line 167, in __call__
    await self.app(scope, receive, send_wrapper)
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/starlette.py", line 200, in _create_span_call
    return await old_call(app, scope, new_receive, new_send, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/starlette/middleware/base.py", line 190, in __call__
    async with anyio.create_task_group() as task_group:
  File "/usr/local/lib/python3.12/site-packages/anyio/_backends/_asyncio.py", line 767, in __aexit__
    raise exc_val
  File "/usr/local/lib/python3.12/site-packages/starlette/middleware/base.py", line 192, in __call__
    await response(scope, wrapped_receive, send)
  File "/usr/local/lib/python3.12/site-packages/starlette/responses.py", line 258, in __call__
    async with anyio.create_task_group() as task_group:
  File "/usr/local/lib/python3.12/site-packages/anyio/_backends/_asyncio.py", line 767, in __aexit__
    raise exc_val
  File "/usr/local/lib/python3.12/site-packages/starlette/responses.py", line 265, in __call__
    await wrap(partial(self.listen_for_disconnect, receive))
  File "/usr/local/lib/python3.12/site-packages/starlette/responses.py", line 261, in wrap
    await func()
  File "/usr/local/lib/python3.12/site-packages/starlette/responses.py", line 238, in listen_for_disconnect
    message = await receive()
              ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/starlette/middleware/base.py", line 54, in wrapped_receive
    msg = await self.receive()
          ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/sentry_sdk/integrations/starlette.py", line 179, in _sentry_receive
    return await receive(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
CancelledError: Future cancelled.

@lesnik512
Copy link
Contributor Author

https://docs.sentry.io/platforms/python/integrations/fastapi/

If you have the fastapi package in your dependencies, the FastAPI integration will be enabled automatically when you initialize the Sentry SDK.

@lesnik512
Copy link
Contributor Author

And more https://docs.sentry.io/platforms/python/integrations/fastapi/#options

By adding FastApiIntegration to your sentry_sdk.init() call explicitly, you can set options for FastApiIntegration to change its behavior. Because FastAPI is based on the Starlette framework, both integrations, StarletteIntegration and FastApiIntegration, must be instantiated.

@lesnik512
Copy link
Contributor Author

Also https://docs.sentry.io/platforms/python/integrations/asgi/

Please check our list of supported integrations as there might already be a specific integration (like FastAPI or Sanic) that is easier to use and captures more useful information than our generic ASGI middleware. If that's the case, you should use the specific integration instead of this middleware.

@lesnik512
Copy link
Contributor Author

So, I'll remove both middleware and explicit middleware adding

@vrslev
Copy link
Contributor

vrslev commented Dec 9, 2024

Actually, I’ve recently encountered the exact same issue with Litestar 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants