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

put record msg and args in history, remove message #135

Merged
merged 1 commit into from
Sep 22, 2016

Conversation

ezarowny
Copy link
Contributor

@ezarowny ezarowny commented Sep 22, 2016

Addresses #134.

Python's logging module can throw UnicodeDecodeErrors when the getMessage function attempts to format the log message. This can happen for a few reasons but it's almost always caused by a mismatch between encodings (ASCII vs UTF-8) and string types (str vs unicode) (see #134 for examples). For reference, the source for getMessage is as follows:

def getMessage(self):
    """
    Return the message for this LogRecord.

    Return the message for this LogRecord after merging any user-supplied
    arguments with the message.
    """
    if not _unicode: #if no unicode support...
        msg = str(self.msg)
    else:
        msg = self.msg
        if not isinstance(msg, basestring):
            try:
                msg = str(self.msg)
            except UnicodeError:
                msg = self.msg      #Defer encoding till later
    if self.args:
        msg = msg % self.args
    return msg

Pyrollbar's log handler stores the last few log records in thread locals to give the user some additional context to their log messages. When building a payload to send to Rollbar, getMessage is called on each record in the history in order to format the record's message.

However, records that cause exceptions when attempting to format can end up stored in pyrollbar's history. Additionally, because of where the exception occurs, the record is never purged from the history. This results in an exception every time a user tries to log because getMessage is called on the bad record.

We currently don't use the history data in Rollbar's UI other than just displaying it raw. Knowing this, a simple and safe fix is to store the unformatted record message and args instead of the formatted message.

@coryvirok
Copy link
Contributor

Nice. LGTM

@brianr
Copy link
Member

brianr commented Sep 22, 2016

👍 lgtm

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

Successfully merging this pull request may close these issues.

3 participants