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

Sentry won't work properly with typer #1604

Open
birdhackor opened this issue Sep 6, 2022 · 9 comments · May be fixed by #3869
Open

Sentry won't work properly with typer #1604

birdhackor opened this issue Sep 6, 2022 · 9 comments · May be fixed by #3869
Labels
New Integration Integrating with a new framework or library Triaged Has been looked at recently during old issue triage

Comments

@birdhackor
Copy link

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 antonpirker added New Integration Integrating with a new framework or library Status: Backlog and removed Status: Untriaged labels Sep 13, 2022
@antonpirker
Copy link
Member

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
Copy link
Member

I have also created a discussion for this topic: #1612

@danielbraun89
Copy link

danielbraun89 commented Apr 11, 2023

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)

supervacuus added a commit to getsentry/symx that referenced this issue Jun 1, 2023
@dltacube
Copy link

dltacube commented Jun 27, 2023

@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?

@antonpirker antonpirker added the Triaged Has been looked at recently during old issue triage label Jan 29, 2024
@Chanmoro
Copy link

Chanmoro commented Jan 30, 2024

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.

class ExcepthookIntegration(Integration):
identifier = "excepthook"
always_run = False
def __init__(self, always_run=False):
# type: (bool) -> None
if not isinstance(always_run, bool):
raise ValueError(
"Invalid value for always_run: %s (must be type boolean)"
% (always_run,)
)
self.always_run = always_run
@staticmethod
def setup_once():
# type: () -> None
sys.excepthook = _make_excepthook(sys.excepthook)

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
Copy link

GeorchW commented Mar 12, 2024

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
Copy link
Member

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
Copy link
Contributor

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. 😅

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 18, 2024
@antonpirker
Copy link
Member

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)

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 25, 2024
@patrick91 patrick91 linked a pull request Dec 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Integration Integrating with a new framework or library Triaged Has been looked at recently during old issue triage
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

9 participants