config module eats exceptions thrown lower down

[vapier]

i tried to instantiate a root whose make.profile was pointing to default/linux/arm64. pkgcore threw an error:

  ...
    domain = config.get_default('domain')
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 568, in get_default
    "Failed instantiating default %s %r" % (type_name, defaults[0][0])))
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 563, in get_default
    return defaults[0][1].instantiate()
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 146, in instantiate
    compatibility.raise_from(errors.InstantiationError(self.name))
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 142, in instantiate
    self._instance = self._instantiate()
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 214, in _instantiate
    "exception caught from %r" % (errors._identify_functor_source(self.type.callable),)))
  File "/usr/lib64/python2.7/site-packages/pkgcore/config/central.py", line 209, in _instantiate
    self._instance = callable_obj(*pargs, **configdict)
  File "/usr/lib64/python2.7/site-packages/pkgcore/ebuild/domain.py", line 355, in __init__
    wrapped_repo = repo.configure(*pargs)
  File "/usr/lib64/python2.7/site-packages/pkgcore/ebuild/repository.py", line 565, in __init__
    self.__class__,))
pkgcore.config.errors.ConfigurationError: Failed instantiating default domain 'livefs domain'

that error isn't terribly useful, but if we look at the source of the traceback, we see that it tried to throw a useful message:

        elif 'CHOST' not in domain_settings:
            raise errors.InitializationError(
                "%s requires the following settings: 'CHOST', not supplied" % (
                    self.__class__,))

the problem is that pkgcore/config/central.py catches Exception and then ignores the content of it, instead choosing to throw a plain error:

            compatibility.raise_from(errors.ConfigurationError(
                "Failed instantiating default %s %r" % (type_name, defaults[0][0])))

that module is full of such ignore-the-exception-and-throw-a-plain-one-instead cases

[radhermit]

Yeah, I'm not overly fond of the current config design in certain places. Perhaps @ferringb can explain some of the history/reasons behind this.

[radhermit]

I forgot to mention another point. Normally when using the various commandline tools such as pmerge, the exception chains created by raise_from() will be shown "properly" by dump_error() from pkgcore.util.commandline.

Really we should just set sys.excepthook to some custom function like dump_error so the chains are shown in all cases.

[ferringb]

Offhand, ConfigurationError should be thrown from those areas- any consuming code needs to be able to just catch one inheritance tree of execptions.

Regarding sys.excepthook, if that were done via the pkgcore.util.commandline (or pwrapper bits) I'd be +1 for that....

[radhermit]

How would doing sys.excepthook in pkgcore.util.commandline or pwrapper handle raw API usage cases, e.g. https://github.com/pkgcore/pkgcore/issues/82 ?

[ferringb]

Revisiting this 7 years later: the raise from logic is correct. Chaining errors is what should happen here since if we start summarizing/wrapping, we lose the error chain from the internals.

However, that doesn't mean the UI is displaying the errors in a sane way. From a library standpoint- which pkgcore is- it shouldn't fiddle w/ sys.excepthook, it should instead expect the invoker to be able to display exceptions properly.

I'm closing this out since I suspect no one would argue w/ that, or that the UI should be improved.

[vapier]

the library should be chaining the exception. it isn't. ergo the module still needs improving doesn't it ?

[arthurzam]

the library should be chaining the exception. it isn't. ergo the module still needs improving doesn't it ?

I will gladly revisit this issue and try to fix it, but I just wanted to check something with you: does passing --debug show the full traceback.

Also, a reproducer will be very helpful to understand and for me to fix the issue. The codebase have changed a lot since then, so the current code examples aren't helpful for me.

[ferringb]

the library should be chaining the exception. it isn't. ergo the module still needs improving doesn't it ?

Yeah, that wasn't obvious in the post, my bad. In the future if you can link the code- especially if the issue goes stale- it's good for quick triage.

I'm seeing a couple of spots in central.py that need updating for a raise from, so I'll take a stab at cleaning that up in a bit.

[ferringb]

the library should be chaining the exception. it isn't. ergo the module still needs improving doesn't it ?

Pardon, but at the time of this bug report- this was true. Looking at it now, I'm not finding any code that does a raise w/out a direct raise from.

@vapier do you have a raw repro? pconfig dump should encapsulate enough for me to fix + write tests. I fully acknowledge the original code had this flaw, but I can't find it now, thus I need a repro.

[ferringb]

As to the original bug report; we used a compatibility 'raise from' back in py2k which wasn't properly support for error reporting. I'm assuming this error report was from that era, albeit before someone (presumably @radhermit ) rewrote all of that code and removed the compat code. I reiterate, that code was removed, and I'm fairly sure the bug vapier is referring to was wiped in the process.

@vapier if you've got a repro or TB that at least tells us which line to look at, please share. Barring that I'm closing this out since I can't find anything different from the previous "check every raise" I did 24h ago ;)