CLI toplevel exception reporting hides "cause"

[mih]

While this is not about a particular exception scenario, here is a concrete use case to help illustrate the problem: I have a command that performs input validation. If that fails a uniform ValueError is raised, and it is raised from the underlying exception representing the specific nature of the validation failure (e.g. TypeError, network down, water too cold, ...).

When such a ValueError reaches the CLI top-level handler, all users get to learn about the issue is:

[ERROR  ] Validation of parameter 'spec' failed (ValueError)

And that is despite the fact that exc.__cause__ is ValueError: URL is not a string at this point.

Two approaches would improve this:

1. CapturedException's .message would include information on a reported cause.
2. A patch like this:

diff --git a/datalad/cli/main.py b/datalad/cli/main.py
index c5364f4b3..49b09913a 100644
--- a/datalad/cli/main.py
+++ b/datalad/cli/main.py
@@ -212,7 +212,9 @@ def _run_with_exception_handler(cmdlineargs):
             exit_code = 3
         else:
             # some unforeseen problem
-            lgr.error('%s (%s)', ce.message, ce.name)
+            lgr.error('%s (%s)%s',
+                      ce.message, ce.name,
+                      f': {exc.__cause__}' if exc.__cause__ else '')
         sys.exit(exit_code)

In both cases the output to a user would be more like this:

[ERROR  ] Validation of parameter 'spec' failed (ValueError): URL is not a string

[bpoldrack]

FTR:

3. Additional method in CapturedException for more verbose messaging (essentially the patch). I believe we have similar code for special remote errors following the __cause__ chain, that could reuse this as well. Ultimately, I think, whether one wants the short, high-level message or a more verbose expression of causes, depends on how generic the piece of code is. So, having both available from CapturedException makes most sense to me.

[mih]

Wouldn't (3) require the exception to be kept alive/bound? Wasn't that one of the aims of CapturedException to avoid this?

[bpoldrack]

Wouldn't (3) require the exception to be kept alive/bound? Wasn't that one of the aims of CapturedException to avoid this?

Not required, no. CapturedException captures __cause__ and __context__ (textually) for that reason (currently only used in format_oneline_tb).

This is part of CapturedException.tb.

[bpoldrack]

It's the original idea to have the entire traceback including __cause__ and __context__ chains captured textually, but structured, so that different methods could "render" it in different ways. Eventually, I wanted to adjust log formatters/result renderers to that. So that we only ever pass the CapturedException object (rather than passing ce.message), and formatters/renderers can decide (based on switches/configs) what and how to show it.
Note also, that CapturedException.format_standard() is there as an example. It would give the formatted traceback output like python does by default, eventually allowing for debug-mode where formatter/renderer can give nice, human-readable tracebacks, rather than the short one-line variant of it.

[bpoldrack]

@mih: Proposal in #7210

[mih]

The PR does not have a demo on seeing the effect of the implementation on the CLI (and I cannot afford to cook one up, right now), but if it does fix this issue, I am happy regardless of the approach. Thx.

[yarikoptic-gitmate]

Issue fixed in 0.17.10