ENH: get_status_dict - add exit_code if CommandError

[yarikoptic]

Immediate use-case is foreach since it feels worthwhile channeling
upstairs the details of the failed execution, but I can imagine
it being useful for other cases too.

WDYT?

TODO:

• test

[codecov]

Codecov Report

Merging #5642 (d32d905) into master (fff474c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           master    #5642    +/-   ##
========================================
  Coverage   89.71%   89.72%            
========================================
  Files         318      320     +2     
  Lines       41652    41808   +156     
========================================
+ Hits        37367    37511   +144     
- Misses       4285     4297    +12     

Impacted Files	Coverage Δ
datalad/interface/results.py	96.26% <100.00%> (+0.11%)	⬆️
datalad/interface/tests/test_results.py	100.00% <100.00%> (ø)
datalad/support/exceptions.py	83.33% <100.00%> (+0.32%)	⬆️
datalad/local/clean.py	82.85% <0.00%> (ø)
datalad/local/tests/test_clean.py	100.00% <0.00%> (ø)
datalad/interface/clean.py	83.78% <0.00%> (+0.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fff474c...d32d905. Read the comment docs.

[kyleam]

Looks useful to me.

[yarikoptic]

Thank you @kyleam . Other @datalad/developers - any feedback before I proceed?

[mih]

My 2ct: There is very little cost to adding useful information to a result record. The only downside is that with #6054 it is the wild-west re naming the fields. Field names will have to be static forever, because hooks depend on them, once written.

[mih]

After having looked into this, we need to change the approach to make this happen. As discovered in #6167 (comment) and documented in #6171 get_status_dict() can no longer handle CommandError exceptions -- only CapturedException.

@bpoldrack can you please advice how to handle this case reliably, ie. check that the captured exception was CommandError (possible via .name), and then extract the original .code property? I believe this is not possible (by design), but I might be wrong. And if I am correct, which approach would be a suitable alternative (excluding a long list of special cases for particular captured exception types)?

[bpoldrack]

I believe this is not possible (by design), but I might be wrong. And if I am correct, which approach would be a suitable alternative (excluding a long list of special cases for particular captured exception types)?

I think you are right. There are two ways, I can see:

1. The "ugly" one:
   When capturing the exception get that property to be able to put in a result record directly rather than "reconstructing" it after the fact within get_status_dict.

2. The cleaner, generic way in my view:
   Enhance CapturedException to capture attributes of the original exception and make them available (via __getattr__ or something, I guess). In that case the PR would work as is. After all CapturedException is supposed to capture the relevant information about it (w/o keeping references to stack frames, etc). I didn't think of such attributes, but arguably they simpy should be part of what is captured.

   Edit: Probably woud need to be CapturedException.exc.generic_attribute_from_original_exc, though, in order to not have the namespaces collide and be truly universal. So, CapturedException.exc.code in this case.

[mih]

I think I do not like either scenario. Any additional logic that is put into CapturedException will slows things down. I think I will RF this stack and rearrange functionality:

• there is no real need for get_status_dict() to require CapturedException, only that it doesn't know how to format a traceback without it -- but this can change
• take CapturedException.format_oneline_tb() and turn it into a standalone helper that is also accessible to get_status_dict()
• install a switch in get_status_dict()that uses this new helper properly with any passed exception
• put the change in this PR on top of it

[mih]

I pushed my proposal to this PR.

[mih]

This is now complete with a test and ready to merge.

[mih]

Thx @bpoldrack