check_module: Don't crash, exit with error instead

[hroncok]

This should avoid https://bugzilla.redhat.com/show_bug.cgi?id=1689507

I still don't know what makes this happen, or if a regular Exception is better here than AttributeError, but this is not the first time this was reported by Fedora automatic bug report.

See for example https://bugzilla.redhat.com/show_bug.cgi?id=1601763

I've asked the user what were they actually doing.

https://retrace.fedoraproject.org/faf/reports/2267286/ gives hint that this happens quite often.

[codecov-io]

Codecov Report

Merging #261 into master will decrease coverage by 0.71%.
The diff coverage is 37.5%.

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
- Coverage   88.03%   87.31%   -0.72%     
==========================================
  Files           6        6              
  Lines         660      670      +10     
==========================================
+ Hits          581      585       +4     
- Misses         79       85       +6

Impacted Files	Coverage Δ
argcomplete/_check_module.py	72.34% <37.5%> (-8.75%)	⬇️

---

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 5e4d3c6...dc84a4e. Read the comment docs.

[kislyuk]

@evanunderscore can you please take a look at this? Code in question was introduced in #204.

[evanunderscore]

I had assumed that find_spec would raise an exception if it could not find the module as find_module does, but I missed the part in the documentation that says it returns None. This fix will at least make the cause of the failure clearer, so I'm all for it.

Having said that, failing with an exception is something this script is designed to do in any number of error cases. I don't know anything about Fedora's automatic bug reporting - what exactly is triggering that? How does it decide which kind of failures need to be reported?

Looking at the user's description, I can see why this is happening in his particular case, and I can probably come up with a patch that checks how many arguments have been passed before trying to invoke some of these completion methods, but there are still other expected failure cases that will likely trigger this reporting mechanism, so I'd like to get to the bottom of that if possible. Can you provide any insight @hroncok?

[hroncok]

The Fedora's automatic bug reporting recognizes if a program crashes with a traceback on stderr and non-0 exit code. It usually means a problem. If exiting with non-0 is desired here, to avoid the report, the code would need to catch the exception and report the error via print to stdout or similar. I can send a PR, but I'd like to know first: how is this failure detected elsewhere?

[hroncok]

Something like this:

• convert all risen Exceptions to something more specific (RuntimeError maybe?)

try:
    main()
except RuntimeError as e:
    print(e, file=sys.stderr)
    sys.exit(1)

[evanunderscore]

how is this failure detected elsewhere?

This is an internal function that is run by the global completion function to check whether a given module is compatible with argcomplete. You can see where it is invoked here:

argcomplete/argcomplete/bash_completion.d/python-argcomplete.sh

Line 32 in 5e4d3c6

if "$executable" -m argcomplete._check_module "${COMP_WORDS[2]}" >/dev/null 2>&1; then

If it's the only way to avoid the automatic bug report, then yes, we'd have to catch the exceptions and explicitly exit with a non-zero error code instead. In your snipped above, sys.exit(e) will handle the printing part for you and exit with code 1. Rather than RuntimeError I'd prefer to define our own exception type and specifically throw/catch that to avoid catching something we don't mean to. The find_spec reimplementation would also need to handle the ImportError potentially raised by find_module and instead return None.

Would you be happy to implement that?

[hroncok]

I would! Thanks.

[hroncok]

Done.

[evanunderscore]

@hroncok Thanks for your work on this, the changes look good to me. Up to you whether you want to do anything about the remaining unresolved comments. Let me know when you're ready for me to merge.

[hroncok]

I will address the comments.

[kislyuk]

@hroncok Please update with the preferred wording for the error name. Otherwise, is this good to merge?

[kislyuk]

(Sorry about the delay in my response.)

[kislyuk]

Released in v1.9.5, please test.

[hroncok]

I'll update the Fedora package and hopefully those reports will stop.