Textualize / rich

Rich is a Python library for rich text and beautiful formatting in the terminal.
https://rich.readthedocs.io/en/latest/
MIT License
49.65k stars 1.73k forks source link

`RichHandler` does not handle log formatting exceptions #3536

Open snejus opened 1 month ago

snejus commented 1 month ago

Describe the bug

RichHandler does not handle exceptions raised while formatting the log message, unlike default builtin Python handlers.

Simple example

Below I set up the logger with RichHandler and provide bad arguments to the log.warning call.

>>> import logging
>>> from rich.logging import RichHandler

>>> log = logging.getLogger(__name__)
>>> log.handlers = [RichHandler()]

>>> log.warning("{bad}", 123)
... print("logic continues")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[12], line 1
----> 1 log.warning("{bad}", 123)
      2 print("logic continues")

...

File ~/.local/share/pyenv/versions/3.8.19/lib/python3.8/logging/__init__.py:373, in LogRecord.getMessage(self)
    371 msg = str(self.msg)
    372 if self.args:
--> 373     msg = msg % self.args
    374 return msg

TypeError: not all arguments converted during string formatting

>>>

I expected the logic to continue running with message logic continues printed. Compare this with the default behaviour:

>>> import logging

>>> log = logging.getLogger(__name__)
>>> log.handlers = [logging.StreamHandler()]

>>> log.warning("{bad}", 123)
... print("logic continues")
--- Logging error ---
Traceback (most recent call last):
  File "/home/sarunas/.local/share/pyenv/versions/3.8.19/lib/python3.8/logging/__init__.py", line 1085, in emit
    msg = self.format(record)

  ...

  File "<ipython-input-7-ee55d4d922a6>", line 1, in <module>
    log.warning("{bad}", 123)
Message: '{bad}'
Arguments: (123,)
logic continues

>>>

Here the exception got handled and my logic continued running with the message being printed.

The cause

I had a quick glance at RichHandler implementation and compared it with StreamHandler. I think the issue lies in emit method implementation:

class StreamHandler(Handler):
    ...
    def emit(self, record):
        try:
            msg = self.format(record)
            stream = self.stream
            # issue 35046: merged two stream.writes into one.
            stream.write(msg + self.terminator)
            self.flush()
        except RecursionError:  # See issue 36272
            raise
        except Exception:
            self.handleError(record)

and

class RichHandler(Handler):
    ...
    def emit(self, record: LogRecord) -> None:
        """Invoked by logging."""
        message = self.format(record)
        traceback = None
        ...

In StreamHandler, the self.format(record) call is tucked under the try statement while in RichHandler it is not.

github-actions[bot] commented 1 month ago

We found the following entry in the FAQ which you may find helpful:

Feel free to close this issue if you found an answer in the FAQ. Otherwise, please give us a little time to review.

This is an automated reply, generated by FAQtory

jakub-mrow commented 1 week ago

Hi, I have created a PR for this issue, would love to contribute! PR