Edgewall Software
Modify

Opened 7 years ago

Last modified 6 months ago

#11463 new enhancement

Strip markup from log messages

Reported by: Ryan J Ollos Owned by:
Priority: normal Milestone: next-stable-1.4.x
Component: general Version:
Severity: normal Keywords:
Cc: Jun Omae Branch:
Release Notes:
API Changes:
Internal Changes:

Description

It was previously mentioned in comment:27:ticket:11189 and comment:3:ticket:11369 that it might be better to strip the markup from log messages to make them more readable. It occurred to me while reviewing the patch in #11462 that this might be as easy as wrapping the message in a call to plaintext.

For example, currently we get a log message for the case of a ConfigurationError (newlines have been added to make this more readable):

Trac[chrome] ERROR: Error during check of EMAIL_VIEW:
ConfigurationError: Cannot find implementation(s) of the <tt>IPermissionPolicy</tt> interface named <tt>ReadonlyWikiPolicy</tt>.
Please check that the Component is enabled or update the option <tt>[trac] permission_policies</tt> in trac.ini.

If the exception message is passed to plaintext before being passed to the logger, we get:

Trac[chrome] ERROR: Error during check of EMAIL_VIEW:
ConfigurationError: Cannot find implementation(s) of the IPermissionPolicy interface named ReadonlyWikiPolicy.
Please check that the Component is enabled or update the option [trac] permission_policies in trac.ini.
  • trac/web/chrome.py

    diff --git a/trac/web/chrome.py b/trac/web/chrome.py
    index f627ae5..864d569 100644
    a b class Chrome(Component):  
    866866            # simply log the exception here, as we might already be rendering
    867867            # the error page
    868868            self.log.error("Error during check of EMAIL_VIEW: %s",
    869                            exception_to_unicode(e))
     869                           plaintext(exception_to_unicode(e)))
    870870            show_email_addresses = False
    871871
    872872        def pretty_dateinfo(date, format=None, dateonly=False):

To be investigated is whether the best approach is to wrap the exception message that is passed to the logger everywhere that the exceptions are trapped, or if there is a better approach.

Attachments (0)

Change History (7)

comment:1 by Jun Omae, 7 years ago

Cc: Jun Omae added

Good idea.

We expect exception_to_unicode convert an exception to unicode instance. Also we often use it with logger. Therefore, I think we should use plaintext function in exception_to_unicode. It might be good to add exception_to_fragment like to_fragment if needed.

>>> from trac.config import ConfigurationError
>>> from trac.util.html import exception_to_fragment
>>> from trac.util.text import exception_to_unicode
>>> from trac.util.translation import tag_
>>> from genshi.builder import tag
>>> print exception_to_unicode(ConfigurationError(tag_('See %(opt)s', opt=tag.tt('[wiki] max_size'))))
ConfigurationError: See [wiki] max_size
>>> exception_to_fragment(ConfigurationError(tag_('See %(opt)s', opt=tag.tt('[wiki] max_size'))))
<Fragment>
>>> print exception_to_fragment(ConfigurationError(tag_('See %(opt)s', opt=tag.tt('[wiki] max_size'))))
ConfigurationError: See <tt>[wiki] max_size</tt>
  • trac/util/text.py

    diff --git a/trac/util/text.py b/trac/util/text.py
    index 269b488..304b2de 100644
    a b import textwrap  
    2727from urllib import quote, quote_plus, unquote
    2828from unicodedata import east_asian_width
    2929
     30from trac.core import TracError
    3031from trac.util.translation import _
    3132
    3233
    def exception_to_unicode(e, traceback=False):  
    8485    In addition to `to_unicode`, this representation of the exception
    8586    also contains the class name and optionally the traceback.
    8687    """
    87     message = '%s: %s' % (e.__class__.__name__, to_unicode(e))
     88    if isinstance(e, TracError):
     89        from trac.util.html import plaintext, to_fragment
     90        message = plaintext(to_fragment(e))
     91    else:
     92        message = to_unicode(e)
     93    message = '%s: %s' % (e.__class__.__name__, message)
    8894    if traceback:
    8995        from trac.util import get_last_traceback
    9096        traceback_only = get_last_traceback().split('\n')[:-2]
  • trac/util/html.py

    diff --git a/trac/util/html.py b/trac/util/html.py
    index c345bd1..88a5c1d 100644
    a b def to_fragment(input):  
    351351    if isinstance(input, Fragment):
    352352        return input
    353353    return tag(to_unicode(input))
     354
     355
     356def exception_to_fragment(e):
     357    """Convert an `Exception` to a `Fragment` object."""
     358
     359    return tag('%s: ' % e.__class__.__name__, to_fragment(e))

comment:2 by Ryan J Ollos, 7 years ago

Milestone: next-stable-1.0.x1.0.3

comment:3 by Ryan J Ollos, 6 years ago

Small change in [5c1303b0/rjollos.git] that I'll save from committing until this ticket is implemented.

comment:4 by Ryan J Ollos, 6 years ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:5 by Ryan J Ollos, 6 years ago

Milestone: 1.0.3next-stable-1.0.x
Owner: Ryan J Ollos removed
Status: assignednew

comment:6 by Ryan J Ollos, 4 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

comment:7 by Ryan J Ollos, 6 months ago

Milestone: next-stable-1.2.xnext-stable-1.4.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.