Strip markup from log messages

[Ryan J Ollos]

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

@@ -866 +866 @@
             # simply log the exception here, as we might already be rendering
             # the error page
             self.log.error("Error during check of EMAIL_VIEW: %s",
-                           exception_to_unicode(e))
+                           plaintext(exception_to_unicode(e)))
             show_email_addresses = False
 
         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.

[Jun Omae]

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

@@ -27 +27 @@
 from urllib import quote, quote_plus, unquote
 from unicodedata import east_asian_width
 
+from trac.core import TracError
 from trac.util.translation import _
 
 
@@ -84 +85 @@
     In addition to `to_unicode`, this representation of the exception
     also contains the class name and optionally the traceback.
     """
-    message = '%s: %s' % (e.__class__.__name__, to_unicode(e))
+    if isinstance(e, TracError):
+        from trac.util.html import plaintext, to_fragment
+        message = plaintext(to_fragment(e))
+    else:
+        message = to_unicode(e)
+    message = '%s: %s' % (e.__class__.__name__, message)
     if traceback:
         from trac.util import get_last_traceback
         traceback_only = get_last_traceback().split('\n')[:-2]

trac/util/html.py

@@ -351 +351 @@
     if isinstance(input, Fragment):
         return input
     return tag(to_unicode(input))
+
+
+def exception_to_fragment(e):
+    """Convert an `Exception` to a `Fragment` object."""
+
+    return tag('%s: ' % e.__class__.__name__, to_fragment(e))

[Ryan J Ollos]

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

[Ryan J Ollos]

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.

[Ryan J Ollos]

Milestone renamed