getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.86k stars 486 forks source link

Sentry won't work properly with typer #1604

Open birdhackor opened 2 years ago

birdhackor commented 2 years ago

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.9.8

Steps to Reproduce

I try to use sentry with typer.

Seems sentry will not catch uphandled exception with them.

With typer method 1:

import typer
import sentry_sdk

sentry_sdk.init(
    dsn="dsn",
    debug=True,

    # Set traces_sample_rate to 1.0 to capture 100%
    # of transactions for performance monitoring.
    # We recommend adjusting this value in production.
    traces_sample_rate=1.0
)

def main():
    1/0

if __name__ == "__main__":
    typer.run(main)

With typer method 2:

import typer
import sentry_sdk

sentry_sdk.init(
    dsn="dsn",
    debug=True,

    # Set traces_sample_rate to 1.0 to capture 100%
    # of transactions for performance monitoring.
    # We recommend adjusting this value in production.
    traces_sample_rate=1.0
)

def main():
    1/0

if __name__ == "__main__":
    try:
        typer.run(main)
    except:
        print('YES, we enter exception handling block')
        raise

Both way don't work

Expected Result

Seems sentry should record error in both way.

Actual Result

Seems sentry don't record error in either way.

antonpirker commented 2 years ago

Hello @birdhackor !

Thanks for bringing this to our attention! We have not yet tried typer, but we are big fans of @tiangolo who is also doing FastAPI :-)

As typer is probably doing some error handling on its own, we probably have to create a typer integration to make it work properly.

Anyone who reads this: If you want to see a typer integration, please thumb-up this issue!

antonpirker commented 2 years ago

I have also created a discussion for this topic: https://github.com/getsentry/sentry-python/discussions/1612

danielbraun89 commented 1 year ago

For those who need a quick solution -

Option 1: set _TYPER_STANDARD_TRACEBACK=1 env variable , in order to disable typer exception handler

Option 2: init sentry in a typer callback:

def callback():
    sentry_sdk.init(...)

app = typer.Typer(callback=callback)
dltacube commented 1 year ago

@danielbraun89 do you mind posting what version of python, sentry-sdk and typer you're using? I've tried both methods you suggested and neither produce Sentry errors for me.

/edit I got it working by combining both options in typer 0.9. I assume you meant to say those two options each should have done the trick on their own?

Chanmoro commented 7 months ago

Hi,

I believe this issue occurs when an unhandled exception is raised. If you report the exception using methods like logger.error, logger.exception, capture_exception, or similar, the error is forwarded to Sentry.

Sentry captures unhandled exceptions by overriding sys.excepthook and sends errors whenever sys.excepthook is invoked.

https://github.com/getsentry/sentry-python/blob/e864eab559c2b37b44bdf6f353cbdb25c8f885ce/sentry_sdk/integrations/excepthook.py#L23-L41

By default, Typer handles unhandled exceptions without using sys.excepthook to provide prettier stack traces.

https://github.com/tiangolo/typer/blob/3a7264cd56181690805f220cb44a0301c0fdf9f3/typer/main.py#L53C5-L104

This default behavior can interfere with Sentry's error notification process when an unhandled exception occurs.

As @danielbraun89 mentioned in Option 1, disabling Typer's exception handler should resolve this issue and allow Sentry's error notifications to work properly.

Additionally, we can use the pretty_exceptions_enable option to disable it as well instead of setting the _TYPER_STANDARD_TRACEBACK env variable:

app = typer.Typer(pretty_exceptions_enable=False)

https://typer.tiangolo.com/tutorial/exceptions/#disable-pretty-exceptions

GeorchW commented 6 months ago

For me, it also works when initializing Sentry inside the Typer command:

import sentry_sdk
import typer

app = typer.Typer()

@app.command()
def main():
    sentry_sdk.init(dsn="SNIP")

    raise Exception("Oh no!")

if __name__ == "__main__":
    app()

This probably has the same effect as the callback method above

antonpirker commented 6 months ago

Thanks everyone for all the information. I guess we will add a typer integration to make it work automagically or at least update the docs to help people set it up correctly

tiangolo commented 6 months ago

I'm a big fan of Sentry, I don't know how it does its internal magic to make things work so I'm not sure what is needed from the Typer side, but I would change the internal implementation in Typer to allow an easier integration with Sentry (or expose some types of hooks, or something).

The custom error handling in Typer is just to not show a lot of useless internals from Typer and Click to developers and users of CLIs, as the errors are (normally) just in their own code, to make it easier to understand and fix those errors. But apart from being able to have pretty errors in the terminal, I would change things internally if that helps Sentry.

I also want to implement nicer/pretty errors in FastAPI, so it would be nice to figure out the right way to do this correctly before that.

@antonpirker, if I don't reply here, please reach out to me via DM, email, or something, it could get lost in my thousands of GitHub notifications. :sweat_smile:

antonpirker commented 5 months ago

That's great to hear @tiangolo ! Thanks for offering to change internals if it makes our lives easier!

I will get back to you as soon as we will pick this up! (Will take some time, because we are doing a Python SDK 2.0 right now)