zopefoundation / zope.configuration

Extensible system for supporting various kinds of configurations
https://zopeconfiguration.readthedocs.io
Other
1 stars 6 forks source link

Simplify exception chaining and nested exception error messages. #45

Closed jamadden closed 5 years ago

jamadden commented 5 years ago

(Based on #40 since it touches many of the same things.)

Fixes #43

Given a/b/c/d.zcml included in that order with an error in d.zcml, previously executing python -c "from zope.configuration.xmlconfig import file; file('/tmp/a.zcml')" would get this:

Traceback (most recent call last):
   ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.schema.interfaces.InvalidDottedName: invalid dotted name

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
   ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.exceptions.ConfigurationError: ('Invalid value for', 'package', 'invalid dotted name')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
   ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "/tmp/d.zcml", line 2.4-2.58
    ConfigurationError: ('Invalid value for', 'package', 'invalid dotted name')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
   ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "/tmp/c.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/d.zcml", line 2.4-2.58
    ConfigurationError: ('Invalid value for', 'package', 'invalid dotted name')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
   ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "/tmp/b.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/c.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/d.zcml", line 2.4-2.58
    ConfigurationError: ('Invalid value for', 'package', 'invalid dotted name')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.xmlconfig.ZopeXMLConfigurationError: File "/tmp/a.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/b.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/c.zcml", line 2.4-2.29
    ZopeXMLConfigurationError: File "/tmp/d.zcml", line 2.4-2.58
    ConfigurationError: ('Invalid value for', 'package', 'invalid dotted name')

Now we get the simpler:

Traceback (most recent call last):
  ...
  File "/Users/jmadden/Projects/GithubSources/zope.schema/src/zope/schema/_field.py", line 670, in _validate
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.schema.interfaces.InvalidDottedName: invalid dotted name

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  ...
    raise InvalidDottedName(value).with_field_and_value(self, value)
zope.configuration.exceptions.ConfigurationError: Invalid value for 'package': InvalidDottedName('invalid dotted name')
    zope.schema.interfaces.InvalidDottedName: invalid dotted name
    File "/tmp/d.zcml", line 2.4-2.58
    File "/tmp/c.zcml", line 2.4-2.29
    File "/tmp/b.zcml", line 2.4-2.29
    File "/tmp/a.zcml", line 2.4-2.29

Yes, there's still a bit of repetition in that final case, but that seems OK to me?

This changed the constructors of two of the internal exceptions, but nobody should have been constructing those things anyway, right?

Looking for feedback.

WIP because I need to do some tweaks for coverage.

/cc @mgedmin

mgedmin commented 5 years ago

Yes, there's still a bit of repetition in that final case, but that seems OK to me?

But why? The traceback looks the same, and the displayed exception message is the same. What does the user gain from this duplication?

jamadden commented 5 years ago

But why? The traceback looks the same, and the displayed exception message is the same. What does the user gain from this duplication?

It's not quite the same. The extra info includes the module of the underlying exception, but the string doesn't. If that was all it is, I would drop the underlying exception from the top-level error message. But I think there's value in including most of the details on the single top-level line, mostly for line-oriented searches (possibly including some log analyzers).

I admit my arguments aren't especially strong here.

jamadden commented 5 years ago

But why? The traceback looks the same, and the displayed exception message is the same. What does the user gain from this duplication?

During the process of updating the other place that could raise a very similar exception, I changed my mind and dropped the duplication.

jamadden commented 5 years ago

@mgedmin How does this look now?

mgedmin commented 5 years ago

I'm sure I'm going to love it, but it's a 300-line diff and today's a Saturday.

jamadden commented 5 years ago

Thank you!