FIX: ExceptionFormatter infinite recursion traceback

[ismagilli]

This is my attempt to fix #1044. I'm not sure how good the choice of variable names is, but I couldn't think of a better one. If there are any comments, I am ready to correct them.

[ismagilli]

At the moment, there are several checks errors. I've fixed them, but I'd like to hear the comments on the implementation first and then prepare the commit.

[Delgan]

Hi @ismagilli, thanks a lot for your efforts to resolve this bug!

The solution you proposed is a clever way to circumvent the infinite recursion. However, I was able to craft a code example that exhibits an unexpected behavior:

from loguru import logger
import sys

if __name__ == "__main__":

    def sink(_):
        raise ValueError

    logger.remove()
    logger.add(sink, catch=False)

    catcher = logger.catch(reraise=False)

    class Foo:
        def __repr__(self):
            with catcher:
                raise ValueError

    print("Before: ", catcher._already_logging_exception)  # False (expected).

    try:
        foo = Foo()
        repr(foo)
    except Exception:
        pass

    print("After: ", catcher._already_logging_exception)  # True (unexpected).

    logger.remove()
    logger.add(sys.stderr)

    # The "ValueError" won't be captured by Loguru, instead of that
    # a "ExceptionFormatterRecursionError" will immediately be raised
    # and the program will exit.
    with catcher:
        raise ValueError

This is because if logger._log() raises an Exception, then self._already_logging_exception = False is never called.

You call easily fix it by adding a finally clause:

diff --git a/loguru/_logger.py b/loguru/_logger.py
index 84ac4cb..c26b6c5 100644
--- a/loguru/_logger.py
+++ b/loguru/_logger.py
@@ -1242,9 +1242,11 @@ class Logger:

                 if not self._already_logging_exception:
                     self._already_logging_exception = True
-                    catch_options = [(type_, value, traceback_), depth, True] + options
-                    logger._log(level, from_decorator, catch_options, message, (), {})
-                    self._already_logging_exception = False
+                    try:
+                        catch_options = [(type_, value, traceback_), depth, True] + options
+                        logger._log(level, from_decorator, catch_options, message, (), {})
+                    finally:
+                        self._already_logging_exception = False
                 else:
                     raise ExceptionFormatterRecursionError()

Unfortunately, there is another problem that can't be fixed as easily. The following code still causes infinite recursion:

from loguru import logger

if __name__ == "__main__":

    class Foo:
        def __repr__(self):
            with logger.catch(reraise=True):
                raise ValueError

    foo = Foo()
    repr(foo)

Since a new Catcher instance is created each time __repr__ is called, there is no way to detect the recursion with the suggested implementation. :/

There are also thread-safety concerns. I don't have a suitable solution in mind yet. It's a tricky problem to solve.

[ismagilli]

I completely agree with you. It only comes to mind to try to identify recursion by looking for repetitions in the call stack. But this will lead to false positives and false negatives.

[ismagilli]

Since the above code does not fix the existing problem and I have no idea how to fix the code, there is no need to leave this pull request open.

[Delgan]

Thanks for giving it a shot anyway. I may use your solution as inspiration for finalizing the fix.