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

Logger.catch #224

Closed
ChristianSauer opened this issue Mar 16, 2020 · 6 comments
Closed

Logger.catch #224

ChristianSauer opened this issue Mar 16, 2020 · 6 comments
Labels
feature Request for adding a new feature

Comments

@ChristianSauer
Copy link

No description provided.

@ChristianSauer
Copy link
Author

Logger catch is very useful, but it is problematic in conjunction with click, especially the error code will always be 0, even if there was an exception.

e.g.

@click.group()
def cli():
    pass

@cli.command('setup')
def cli_setup():
    raise Exception("boom!")

@logger.catch(reraise=True)  
def main():
    cli()

If you execute python file.py setup this will set the exit code to 0, expected would be 1

@Delgan
Copy link
Owner

Delgan commented Mar 16, 2020

@ChristianSauer I assume main() is missing at the end of your code snippet, right?

Because on my computer, it's working fine, the exit code is 1 thanks to the reraise=True argument.

2020-03-16-191322_1920x1080_scrot

@ChristianSauer
Copy link
Author

@Delgan yes, you are correct.
And yes, reraise works as expected. The problem is that now the stacktrace is doubled, first in the log, then as the last output of the program. Its just ugly and complicated when you have a long stacktrace.
i would suggest adding a small sentence to the doc warning that without reraise, the exit code will be 0.

@Delgan
Copy link
Owner

Delgan commented Mar 24, 2020

I do agree this is a bit inconvenient. Actually, I just realized that in most of my scripts, I replaced @logger.catch with another snippet because of that.

try:
    main()
except Exception as e:
    logger.opt(exception=True).critical(str(e))
    sys.exit(1)

That's too bad. It means @catch is not used while it's one of the use case where it should shine.

I'm thinking of adding a new parameter to catch() to circumvent this problem. For example, logger.catch(exit=True) could automatically call sys.exit(1) if an exception is caught. Alternatively, a parameter accepting a function like onerror=lambda exc: sys.exit(1) would provide more flexibility.

@Delgan Delgan added the feature Request for adding a new feature label Mar 24, 2020
@ChristianSauer
Copy link
Author

Thats good news, thanks!
I would probably use the onerror variant, cli programs often use different exit codes for different failure modes so some flexibility would open up a lot of use cases.

@Delgan
Copy link
Owner

Delgan commented Apr 11, 2020

Done. The new onerror argument will be available in the next release. Thanks for your input! 😉

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

No branches or pull requests

2 participants