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 .opt(exception=...) quirks and trying to log exception without sys.exc_info #1284

Open
karlicoss opened this issue Jan 24, 2025 · 2 comments

Comments

@karlicoss
Copy link

Hi! I've been trying to figure out how to dump stack trace on exception in loguru by default

Consider this snippet:

from loguru import logger
def throw() -> str:
    raise RuntimeError('throwing')

try:
    throw()
except Exception as e:
    logger.exception(e)

this works exactly as expected and is logging the stack trace.

However, in my code I often have some sort of defensive logic -- I execute a bunch of operations, catch & collect the Exceptions and then do something else with them downstream, i.e. logging might not happen in the except clause.

Simplified, something like this:

try:
    throw()
except Exception as e:
    ex = e  # store for later

# ...

logger.exception(ex)

Now we get this

2025-01-24 21:13:40.909 | ERROR    | __main__:<module>:44 - throwing
NoneType: None

This is because at the point of logger.exception call, sys.exc_info is None (we're outside of except clause).
This behaviour by default perhaps makes some sense -- e.g. it's consistent with builtin logging.exception

So next thing I tried is using opt:

logger.opt(exception=ex).exception(ex)

... which to my surprise also resulted in:

2025-01-24 21:16:48.149 | ERROR    | __main__:<module>:57 - throwing
NoneType: None

This is because Logger.exception function unconditionally sets options[0] to True, overwriting an actual Exception object/tuple.

loguru/loguru/_logger.py

Lines 2098 to 2101 in 8653835

def exception(__self, __message, *args, **kwargs): # noqa: N805
r"""Log an ``'ERROR'```` message while also capturing the currently handled exception."""
options = (True,) + __self._options[1:]
__self._log("ERROR", False, options, __message, args, kwargs)

Funny enough, if we use anything except .exception to log, it works as expected and prints the trace (e.g. logger.opt(exception=ex).debug(ex)).

IMO it doesn't make much sense that .exception logs less exception information than other functions!

Shouldn't it at least check first and only set options[0] to True if the previous value was None or False?

suggestion to opt-in to use exception's exception info

Now, even if that was working correctly, it's a bit mouthful to write .opt(exception=ex).exception(ex) every time when you're trying to log an exception.

I think it would be nice if there was possible to do something like that:

logger = logger.opt(exception='auto')

# ... other code

ex: Exception = some_function_returning_exception()   # note: returning exception, not throwing!

logger.exception(ex)   # this would be equivalent to logger.opt(exception=ex).exception(ex)

, i.e. make .exception calls automatically use the exception they are logging for the traceback information.

'auto' is just the first thing I came up with, open to a better name suggestion!

Do you think it would make sense? I'm happy to try to implement this along with some tests.

@karlicoss karlicoss changed the title some .opt(exception=...) quirks and trying to log some .opt(exception=...) quirks and trying to log exception without sys.exc_info Jan 24, 2025
@Delgan
Copy link
Owner

Delgan commented Jan 28, 2025

Hey @karlicoss, thank you for sharing your detailed thoughts.

I hadn’t realized that logger.exception() might be ambiguous. That's a fair point.

However, I don’t think passing exceptions directly to the logging method is an ideal approach. I feel that the primary argument of logging method should be the log message as a string, while other type arguments such as the exception should be provided as additional context. That’s why I think it's preferable to pass the exception as an argument of opt().

That being said, it might make sense for the exception in opt() to take precedence over the exception in exception(). Indeed, I can see how it might be counter-intuitive not to have to use logger.exception() while one wants to “log an exception".

On the other hand, introducing this precedence would add some logical complexity. The behavior of logger.exception() would become dependent on the external state of opt(). Currently, its behavior is extremely straightforward: it simply calls sys.exc_info() and logs the result.

Personally, I would rather lean toward improving the documentation for logger.exception(). I should clarify that it calls sys.exc_info() internally which results in None outside of a except clause, and advise to use logger.opt() instead. Regarding your use case, I would recommend to create a small log_exception() wrapper in your own code. Sorry if it’s not exactly the answer you were hoping for. :x

@karlicoss
Copy link
Author

Ah thanks for explaining!

The behavior of logger.exception() would become dependent on the external state of opt()

I feel like it's the whole point of .opt though! It contains a lot of parameters that define the behaviour of logging :)

I guess my view is that I'd rather make exception logging as effortless as possible -- otherwise remembering to use .opt(exception every time is a cognitive effort. But if you oppose it, I guess I'll just create a wrapper, yeah.

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

No branches or pull requests

2 participants