Refactored error message handling so that SimpleItem's...

Refactored error message handling so that SimpleItem's raise_standardErrorMessage sniffs the response to see if it wants an html encoded error message or not.

If so, then standard_error_message is rendered as usual as the error value. Otherwise the error value text is passed through unchanged.

XML-RPC responses now request text/plain error format so that standard_error_message is not envoked to render the fault string. Because of this, the previous formatting code could be removed, although html tags are still stripped because error values themselves sometimes contain tags 8^/

doc/CHANGES.txt

@@ -5,6 +5,11 @@ Zope Changes
   file HISTORY.txt.
 
     Bugs Fixed
+    
+      - Collector #550: Exceptions in XML-RPC requests no longer envoke
+        standard_error_message. Plain text error messages are instead added to
+        the fault string. In debug mode, a full traceback is also included
+        since access to the error log is not a given for XML-RPC developers.
 
       - Collector #512,541: Fixed broken WebDAV compatiblity
         with Cadaver 0.20.X due to a missing Lock-Token header.

lib/python/OFS/SimpleItem.py

@@ -17,8 +17,8 @@ Aqueduct database adapters, etc.
 This module can also be used as a simple template for implementing new
 item types.
 
-$Id: SimpleItem.py,v 1.102 2002/08/14 21:42:56 mj Exp $'''
-__version__='$Revision: 1.102 $'[11:-2]
+$Id: SimpleItem.py,v 1.103 2002/08/30 18:29:57 caseman Exp $'''
+__version__='$Revision: 1.103 $'[11:-2]
 
 import re, sys, Globals, App.Management, Acquisition, App.Undo
 import AccessControl.Role, AccessControl.Owned, App.Common
@@ -190,37 +190,41 @@ class Item(Base, Resource, CopySource, App.Management.Tabs, Traversable,
             if client is None: client=self
             if not REQUEST: REQUEST=self.aq_acquire('REQUEST')
 
-            try:
-                if hasattr(client, 'standard_error_message'):
-                    s=getattr(client, 'standard_error_message')
-                else:
-                    client = client.aq_parent
-                    s=getattr(client, 'standard_error_message')
-                kwargs = {'error_type': error_type,
-                          'error_value': error_value,
-                          'error_tb': error_tb,
-                          'error_traceback': error_tb,
-                          'error_message': error_message,
-                          'error_log_url': error_log_url}
-
-                if isinstance(s, HTML):
-                    v = s(client, REQUEST, **kwargs)
-                elif callable(s):
-                    v = s(**kwargs)
-                else:
-                    v = HTML.__call__(s, client, REQUEST, **kwargs)
-            except:
-                LOG('OFS', BLATHER,
-                    'Exception while rendering an error message',
-                    error=sys.exc_info())
+            if REQUEST.RESPONSE._error_format == 'text/html':
                 try:
-                    strv = str(error_value)
+                    if hasattr(client, 'standard_error_message'):
+                        s=getattr(client, 'standard_error_message')
+                    else:
+                        client = client.aq_parent
+                        s=getattr(client, 'standard_error_message')
+                    kwargs = {'error_type': error_type,
+                              'error_value': error_value,
+                              'error_tb': error_tb,
+                              'error_traceback': error_tb,
+                              'error_message': error_message,
+                              'error_log_url': error_log_url}
+
+                    if isinstance(s, HTML):
+                        v = s(client, REQUEST, **kwargs)
+                    elif callable(s):
+                        v = s(**kwargs)
+                    else:
+                        v = HTML.__call__(s, client, REQUEST, **kwargs)
                 except:
-                    strv = '<unprintable %s object>' % str(type(error_value).__name__)
-                v = strv + (
-                    " (Also, an error occurred while attempting "
-                    "to render the standard error message.)")
-            raise error_type, v, tb
+                    LOG('OFS', BLATHER,
+                        'Exception while rendering an error message',
+                        error=sys.exc_info())
+                    try:
+                        strv = str(error_value)
+                    except:
+                        strv = ('<unprintable %s object>' % 
+                                str(type(error_value).__name__))
+                    v = strv + (
+                        " (Also, an error occurred while attempting "
+                        "to render the standard error message.)")
+                raise error_type, v, tb
+            else:
+                raise error_type, error_value, tb
         finally:
             if hasattr(self, '_v_eek'): del self._v_eek
             tb=None

lib/python/ZPublisher/xmlrpc.py

@@ -82,9 +82,11 @@ class Response:
     It's probably possible to improve the 'exception' method quite a bit.
     The current implementation, however, should suffice for now.
     """
+    
+    _error_format = 'text/plain' # No html in error values
 
     # Because we can't predict what kind of thing we're customizing,
-    # we have to use delegation, rather than inheritence to do the
+    # we have to use delegation, rather than inheritance to do the
     # customization.
 
     def __init__(self, real): self.__dict__['_real']=real
@@ -132,20 +134,17 @@ class Response:
         Fault=xmlrpclib.Fault
         f=None
         try:
-            # Strip HTML tags and format the error value
+            # Strip HTML tags from the error value
             v = str(v)
-            v = re.sub(r"<br\s*/?>", "\n", v)
             remove = [r"<[^<>]*>", r"&[A-Za-z]+;"]
             for pat in remove:
                 v = re.sub(pat, " ", v)
-            v = re.sub(r"\n(?:\s*\n)+", "\n\n", v)
-                
             from Globals import DevelopmentMode
             if DevelopmentMode:
                 from traceback import format_exception
-                value = ''.join(format_exception(t, v, tb))
+                value = '\n' + ''.join(format_exception(t, v, tb))
             else:
-                value = '%s\n\n%s' % (t, v)
+                value = '%s - %s' % (t, v)
                 
             if isinstance(v, Fault):
                 f=v