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.

Sign up for GitHub

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

BUG-3536 - fix RichHandler not handling exception #3557

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

BUG-3536 - fix RichHandler not handling exception #3557

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2935d2d
BUG-3536 - fix RichHandler not handling exception
jakub-mrow Nov 10, 2024
226a424
BUG-3536 - wrap emit with try except
jakub-mrow Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion rich/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ def get_level_text(self, record: LogRecord) -> Text:

def emit(self, record: LogRecord) -> None:
"""Invoked by logging."""
message = self.format(record)
try:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's a fair bit of logic below which may also fail, I'd lean towards wrapping the entire method implementation within the try block, similar to the approach used in StreamHandler:

    def emit(self, record):
        """
        Emit a record.

        If a formatter is specified, it is used to format the record.
        The record is then written to the stream with a trailing newline.  If
        exception information is present, it is formatted using
        traceback.print_exception and appended to the stream.  If the stream
        has an 'encoding' attribute, it is used to determine how to do the
        output to the stream.
        """
        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)

I've just tested your solution with log.warning("hello {}", 123) and found that another exception is raised from the following line:

        message_renderable = self.render_message(record, message)

To keep the original implementation clean, you may want to consider something like this:

    ...
    def _emit(self, record: LogRecord) -> None:
        ...
        # original emit implementation
        ...

    def emit(self, record: LogRecord) -> None:
        try:
            return self._emit(record)
        except Exception:
            self.handleError(record)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I am gonna apply the changes

message = self.format(record)
except Exception:
self.handleError(record)

traceback = None
if (
self.rich_tracebacks
Expand Down