Merge 1c300ed into 7c98fbb

CHANGES.rst

@@ -4,7 +4,8 @@ Changes
 4.2.3 (unreleased)
 ------------------
 
-- Nothing changed yet.
+- Simplify exception chaining and nested exception error messages.
+  See `issue 43 <https://github.com/zopefoundation/zope.configuration/issues/43>`_.
 
 
 4.2.2 (2018-09-27)

docs/narr.rst

@@ -1162,16 +1162,15 @@ redefine our directives:
 .. doctest::
 
    >>> from zope.configuration.xmlconfig import string
-   >>> from zope.configuration.xmlconfig import ZopeXMLConfigurationError
-   >>> try:
-   ...    v = string(
+   >>> from zope.configuration.exceptions import ConfigurationError
+   >>> v = string(
    ...      '<text xmlns="http://sample.namespaces.zope.org/schema" name="x" />',
    ...      context)
-   ... except ZopeXMLConfigurationError as e:
-   ...   v = e
-   >>> print(v)
-   File "<string>", line 1.0
-       ConfigurationError: The directive ('http://sample.namespaces.zope.org/schema', 'text') cannot be used in this context
+   Traceback (most recent call last):
+   ...
+   zope.configuration.exceptions.ConfigurationError: The directive ('http://sample.namespaces.zope.org/schema', 'text') cannot be used in this context
+     File "<string>", line 1.0
+
 
 Let's see what happens if we declare duplicate fields:
 
@@ -1187,7 +1186,7 @@ Let's see what happens if we declare duplicate fields:
    ...      </schema>
    ...      ''',
    ...      context)
-   ... except ZopeXMLConfigurationError as e:
+   ... except ConfigurationError as e:
    ...   v = e
    >>> print(v)
    File "<string>", line 5.7-5.24

src/zope/configuration/config.py

@@ -27,6 +27,7 @@
 from zope.schema import ValidationError
 
 from zope.configuration.exceptions import ConfigurationError
+from zope.configuration.exceptions import ConfigurationWrapperError
 from zope.configuration.interfaces import IConfigurationContext
 from zope.configuration.interfaces import IGroupingContext
 from zope.configuration.fields import GlobalInterface
@@ -41,7 +42,6 @@
     'ConfigurationContext',
     'ConfigurationAdapterRegistry',
     'ConfigurationMachine',
-    'ConfigurationExecutionError',
     'IStackItem',
     'SimpleStackItem',
     'RootStackItem',
@@ -74,10 +74,14 @@
 
 
 class ConfigurationContext(object):
-    """Mix-in that implements IConfigurationContext
+    """
+    Mix-in for implementing.
+    :class:`zope.configuration.interfaces.IConfigurationContext`.
 
-    Subclasses provide a ``package`` attribute and a ``basepath``
-    attribute.  If the base path is not None, relative paths are
+    Note that this class itself does not actually declare that it
+    implements that interface; the subclass must do that. In addition,
+    subclasses must provide a ``package`` attribute and a ``basepath``
+    attribute. If the base path is not None, relative paths are
     converted to absolute paths using the the base path. If the
     package is not none, relative imports are performed relative to
     the package.
@@ -91,34 +95,38 @@ class ConfigurationContext(object):
     attribute.
 
     The include path is appended to each action and is used when
-    resolving conflicts among actions.  Normally, only the a
+    resolving conflicts among actions. Normally, only the a
     ConfigurationMachine provides the actions attribute. Decorators
     simply use the actions of the context they decorate. The
-    ``includepath`` attribute is a tuple of names.  Each name is
+    ``includepath`` attribute is a tuple of names. Each name is
     typically the name of an included configuration file.
 
     The ``info`` attribute contains descriptive information helpful
-    when reporting errors.  If not set, it defaults to an empty string.
-
-    The actions attribute is a sequence of dictionaries where each dictionary
-    has the following keys:
+    when reporting errors. If not set, it defaults to an empty string.
 
-      - ``discriminator``, a value that identifies the action. Two actions
-        that have the same (non None) discriminator conflict.
+    The actions attribute is a sequence of dictionaries where each
+    dictionary has the following keys:
 
-      - ``callable``, an object that is called to execute the action,
+        - ``discriminator``, a value that identifies the action. Two
+          actions that have the same (non None) discriminator
+          conflict.
 
-      - ``args``, positional arguments for the action
+        - ``callable``, an object that is called to execute the
+          action,
 
-      - ``kw``, keyword arguments for the action
+        - ``args``, positional arguments for the action
 
-      - ``includepath``, a tuple of include file names (defaults to ())
+        - ``kw``, keyword arguments for the action
 
-      - ``info``, an object that has descriptive information about
-        the action (defaults to '')
+        - ``includepath``, a tuple of include file names (defaults to
+          ())
 
+        - ``info``, an object that has descriptive information about
+          the action (defaults to '')
     """
 
+    # pylint:disable=no-member
+
     def __init__(self):
         super(ConfigurationContext, self).__init__()
         self._seen_files = set()
@@ -666,8 +674,8 @@ class ConfigurationMachine(ConfigurationAdapterRegistry, ConfigurationContext):
     info = ''
 
     #: These `Exception` subclasses are allowed to be raised from `execute_actions`
-    #: without being re-wrapped into a `ConfigurationExecutionError`. (`BaseException`
-    #: instances are never wrapped.)
+    #: without being re-wrapped into a `~.ConfigurationError`. (`BaseException`
+    #: and other `~.ConfigurationError` instances are never wrapped.)
     #:
     #: Users of instances of this class may modify this before calling `execute_actions`
     #: if they need to propagate specific exceptions.
@@ -727,9 +735,8 @@ def execute_actions(self, clear=True, testing=False):
             [('f', (1,), {}), ('f', (2,), {})]
 
         If the action raises an error, we convert it to a
-        ConfigurationExecutionError.
+        `~.ConfigurationError`.
 
-            >>> from zope.configuration.config import ConfigurationExecutionError
             >>> output = []
             >>> def bad():
             ...    bad.xxx
@@ -739,22 +746,35 @@ def execute_actions(self, clear=True, testing=False):
             ...   (2, f, (2,)),
             ...   (3, bad, (), {}, (), 'oops')
             ...   ]
-            >>> try:
-            ...    v = context.execute_actions()
-            ... except ConfigurationExecutionError as e:
-            ...    v = e
-            >>> lines = str(v).splitlines()
-            >>> 'AttributeError' in lines[0]
-            True
-            >>> lines[0].endswith("'function' object has no attribute 'xxx'")
-            True
-            >>> lines[1:]
-            ['  in:', '  oops']
+
+            >>> context.execute_actions()
+            Traceback (most recent call last):
+            ...
+            zope.configuration.config.ConfigurationExecutionError: oops
+                AttributeError: 'function' object has no attribute 'xxx'
 
         Note that actions executed before the error still have an effect:
 
             >>> output
             [('f', (1,), {}), ('f', (2,), {})]
+
+        If the exception was already a `~.ConfigurationError`, it is raised
+        as-is with the action's ``info`` added.
+
+            >>> def bad():
+            ...     raise ConfigurationError("I'm bad")
+            >>> context.actions = [
+            ...   (1, f, (1,)),
+            ...   (1, f, (11,), {}, ('x', )),
+            ...   (2, f, (2,)),
+            ...   (3, bad, (), {}, (), 'oops')
+            ...   ]
+            >>> context.execute_actions()
+            Traceback (most recent call last):
+            ...
+            zope.configuration.exceptions.ConfigurationError: I'm bad
+                oops
+
         """
         pass_through_exceptions = self.pass_through_exceptions
         if testing:
@@ -769,29 +789,23 @@ def execute_actions(self, clear=True, testing=False):
                 info = action['info']
                 try:
                     callable(*args, **kw)
+                except ConfigurationError as ex:
+                    ex.add_details(info)
+                    raise
                 except pass_through_exceptions:
                     raise
                 except Exception:
-                    t, v, tb = sys.exc_info()
-                    try:
-                        reraise(ConfigurationExecutionError(t, v, info),
-                                None, tb)
-                    finally:
-                        del t, v, tb
-
+                    # Wrap it up and raise.
+                    reraise(ConfigurationExecutionError(info, sys.exc_info()[1]),
+                            None, sys.exc_info()[2])
         finally:
             if clear:
                 del self.actions[:]
 
-class ConfigurationExecutionError(ConfigurationError):
+class ConfigurationExecutionError(ConfigurationWrapperError):
     """
     An error occurred during execution of a configuration action
     """
-    def __init__(self, etype, evalue, info):
-        self.etype, self.evalue, self.info = etype, evalue, info
-
-    def __str__(self): # pragma: no cover
-        return "%s: %s\n  in:\n  %s" % (self.etype, self.evalue, self.info)
 
 ##############################################################################
 # Stack items
@@ -1671,15 +1685,16 @@ def toargs(context, schema, data):
             try:
                 args[str(name)] = field.fromUnicode(s)
             except ValidationError as v:
-                reraise(ConfigurationError("Invalid value for", n, str(v)),
+                reraise(ConfigurationError("Invalid value for %r" % (n)).add_details(v),
                         None, sys.exc_info()[2])
         elif field.required:
             # if the default is valid, we can use that:
             default = field.default
             try:
                 field.validate(default)
-            except ValidationError:
-                raise ConfigurationError("Missing parameter:", n)
+            except ValidationError as v:
+                reraise(ConfigurationError("Missing parameter: %r" % (n,)).add_details(v),
+                        None, sys.exc_info()[2])
             args[str(name)] = default
 
     if data:
@@ -1801,17 +1816,19 @@ def bypath(ainfo):
 class ConfigurationConflictError(ConfigurationError):
 
     def __init__(self, conflicts):
+        super(ConfigurationConflictError, self).__init__()
         self._conflicts = conflicts
 
-    def __str__(self): #pragma NO COVER
+    def _with_details(self, opening, detail_formatter):
         r = ["Conflicting configuration actions"]
         for discriminator, infos in sorted(self._conflicts.items()):
             r.append("  For: %s" % (discriminator, ))
             for info in infos:
                 for line in text_type(info).rstrip().split(u'\n'):
                     r.append(u"    " + line)
 
-        return "\n".join(r)
+        opening = "\n".join(r)
+        return super(ConfigurationConflictError, self)._with_details(opening, detail_formatter)
 
 
 ##############################################################################

src/zope/configuration/exceptions.py

@@ -14,10 +14,57 @@
 """Standard configuration errors
 """
 
+import traceback
+
 __all__ = [
     'ConfigurationError',
 ]
 
 class ConfigurationError(Exception):
     """There was an error in a configuration
     """
+
+    # A list of strings or things that can be converted to strings,
+    # added by append_details as we walk back up the call/include stack.
+    # We will print them in reverse order so that the most recent detail is
+    # last.
+    _details = ()
+
+    def add_details(self, info):
+        if isinstance(info, BaseException):
+            info = traceback.format_exception_only(type(info), info)
+            # Trim trailing newline
+            info[-1] = info[-1].rstrip()
+            self._details += tuple(info)
+        else:
+            self._details += (info,)
+        return self
+
+    def _with_details(self, opening, detail_formatter):
+        lines = ['    ' + detail_formatter(detail) for detail in self._details]
+        lines.append(opening)
+        lines.reverse()
+        return '\n'.join(lines)
+
+    def __str__(self):
+        s = super(ConfigurationError, self).__str__()
+        return self._with_details(s, str)
+
+    def __repr__(self):
+        s = super(ConfigurationError, self).__repr__()
+        return self._with_details(s, repr)
+
+
+class ConfigurationWrapperError(ConfigurationError):
+
+    USE_INFO_REPR = False
+
+    def __init__(self, info, exception):
+        super(ConfigurationWrapperError, self).__init__(repr(info) if self.USE_INFO_REPR else info)
+        self.add_details(exception)
+
+        # This stuff is undocumented and not used but we store
+        # for backwards compatibility
+        self.info = info
+        self.etype = type(exception)
+        self.evalue = exception

src/zope/configuration/tests/test_config.py

@@ -1789,7 +1789,18 @@ class ISchema(Interface):
         with self.assertRaises(ConfigurationError) as exc:
             self._callFUT(context, ISchema, {})
         self.assertEqual(exc.exception.args,
-                         ('Missing parameter:', 'no_default'))
+                         ("Missing parameter: 'no_default'",))
+
+        # It includes the details of any validation failure;
+        # The rendering of the nested exception varies by Python version,
+        # sadly.
+        exception_str = str(exc.exception)
+        self.assertTrue(exception_str.startswith(
+            "Missing parameter: 'no_default'\n"
+        ), exception_str)
+        self.assertTrue(exception_str.endswith(
+            "RequiredMissing: no_default"
+        ), exception_str)
 
     def test_w_field_missing_but_default(self):
         from zope.interface import Interface
@@ -1810,8 +1821,16 @@ class ISchema(Interface):
         with self.assertRaises(ConfigurationError) as exc:
             self._callFUT(context, ISchema, {'count': '-1'})
         self.assertEqual(exc.exception.args,
-                         ('Invalid value for', 'count', '(-1, 0)'))
+                         ("Invalid value for 'count'",))
 
+        for meth in str, repr:
+            exception_str = meth(exc.exception)
+            self.assertIn(
+                "Invalid value for",
+                exception_str)
+            self.assertIn(
+                "TooSmall: (-1, 0)",
+                exception_str)
 
 class Test_expand_action(unittest.TestCase):
 
@@ -1974,6 +1993,14 @@ def _b():
             "For: ('a', 1)\n    conflict!\n    conflict2!",
             str(exc.exception))
 
+        exc.exception.add_details('a detail')
+
+        self.assertEqual(
+            "Conflicting configuration actions\n  "
+            "For: ('a', 1)\n    conflict!\n    conflict2!\n"
+            "    a detail",
+            str(exc.exception))
+
     def test_wo_discriminators_final_sorting_order(self):
         from zope.configuration.config import expand_action
         def _a():