zope.configuration swallowing expat parser errors

[tseaver]

In https://bugs.launchpad.net/zope.configuration/+bug/558501, @zopyx reported:

See https://mail.zope.org/pipermail/zope-cmf/2010-April/029016.html

 GenericSetup: Install.py komplett abgelöst context.execute_actions()
  File "/home/ajung/sandboxes/hu/parts/zope2/lib/python/zope/configuration/config.py", line 612, in execute_actions
    callable(*args, **kw)
  File "/home/ajung/sandboxes/hu/eggs/Products.GenericSetup-1.4.5-py2.4.egg/Products/GenericSetup/registry.py", line 724, in registerProfile
    metadata = ProfileMetadata(path, product=product)()
  File "/home/ajung/sandboxes/hu/eggs/Products.GenericSetup-1.4.5-py2.4.egg/Products/GenericSetup/metadata.py", line 52, in __call__
    return self.parseXML( file.read() )
  File "/home/ajung/sandboxes/hu/eggs/Products.GenericSetup-1.4.5-py2.4.egg/Products/GenericSetup/utils.py", line 217, in parseXML
    dom = parseString(xml)
  File "/opt/python-2.4.6/lib/python2.4/xml/dom/minidom.py", line 1925, in parseString
    return expatbuilder.parseString(string)
  File "/opt/python-2.4.6/lib/python2.4/xml/dom/expatbuilder.py", line 940, in parseString
    return builder.parseString(string)
  File "/opt/python-2.4.6/lib/python2.4/xml/dom/expatbuilder.py", line 223, in parseString
    parser.Parse(string, True)
zope.configuration.config.ConfigurationExecutionError: xml.parsers.expat.ExpatError: mismatched tag: line 4, column 2
  in:
  File "/home/ajung/sandboxes/hu/src/Products.HUMoodle/Products/HUMoodle/configure.zcml", line 8.4-14.6
      <genericsetup:registerProfile
          name="default"
          title="Products.HUMoodle"
          directory="profiles/default"
          description="HU Moodle Integration"
          provides="Products.GenericSetup.interfaces.EXTENSION"
      />

ajung@ubuntu:~/sandboxes/hu$ cat /home/ajung/sandboxes/hu/src/Products.HUMoodle/Products/HUMoodle/configure.zcml
<configure xmlns="http://namespaces.zope.org/zope"
           xmlns:cmf="http://namespaces.zope.org/cmf"
           xmlns:genericsetup="http://namespaces.zope.org/genericsetup">

    <cmf:registerDirectory name="hu_moodle" />

    <genericsetup:registerProfile
        name="default"
        title="Products.HUMoodle"
        directory="profiles/default"
        description="HU Moodle Integration"
        provides="Products.GenericSetup.interfaces.EXTENSION"
    />

</configure>

@tseaver replied:

The code at line 612 of zope.configuration.config which re-wraps the exception seems to be deliberate policy. Reassigning to Christian for comment (and also so he can route the bug to the correct project: this one is not really for code bugs).

[jamadden]

xmlconfig.py catches xml.sax.SAXParseException and re-raises it as a ZopeSAXParseException. That class extends ConfigurationError, as expected. So directly parsing an XML file with the zope.configuration machinery raises a ConfigurationError, as expected.

But this is not talking about using the zope.configuration machinery to parse ZCML. Instead, one of the actions taken as a result of (successfully) parsing a ZCML file is raising a parse error. That's where config.py comes in. It very deliberately catches any exception and wraps it in a ConfigurationExecutionError; that error is a ConfigurationError. The net result is that all of the parsing and processing of ZCML can handle errors by catching ConfigurationError.

I think that makes sense.

If there were to be a change, I would just suggest adding ConfigurationError to the list of exceptions (currently just KeyboardInterrupt and SystemExit) that simply get re-raised as-is. No tests break if this is done. We could consider making that list a (class?) attribute of config.ConfigurationMachine so that users can customize what gets directly re-raised, if they have the need to care about that. (Likewise for xmlconfig:ConfigurationHandler.endElementNS.)

EDIT: On second thought, no, we probably don't want ConfigurationError to propagate directly. In both cases the wrapper adds additional information to help locate the underlying source of the problem. (This could also be done with __traceback_info__, but we have no guarantee that our exception rendered would be used.)