Argparse.parse_args exits on unrecognized option with exit_on_error=False

[00dcd123-3f50-4394-96eb-3e7b1086583c]

BPO	41255
Nosy	@rhettinger, @tirkarthi, @matthewhughes934, @jacobtylerwalls, @bigbirdcode, @joshmeranda
PRs	python/cpython#27295 python/cpython#30832
Files	exit_on_error_tests.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.9'] title = 'Argparse.parse_args exits on unrecognized option with exit_on_error=False' updated_at = user = 'https://github.com/matthewhughes934' ``` bugs.python.org fields: ```python activity = actor = 'jacobtylerwalls' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'mhughes' dependencies = [] files = ['49310'] hgrepos = [] issue_num = 41255 keywords = ['patch'] message_count = 10.0 messages = ['373382', '373383', '373385', '373405', '373437', '373440', '373470', '396345', '397975', '401629'] nosy_count = 7.0 nosy_names = ['rhettinger', 'paul.j3', 'xtreak', 'mhughes', 'jacobtylerwalls', 'bigbird', 'joshmeranda'] pr_nums = ['27295', '30832'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue41255' versions = ['Python 3.9'] ```

[00dcd123-3f50-4394-96eb-3e7b1086583c]

>>> import argparse
    >>> parser = argparse.ArgumentParser(exit_on_error=False)
    >>> parser.parse_args(["--unknown"])
    usage: [-h]
    : error: unrecognized arguments: --unknown

The docs https://docs.python.org/3.10/library/argparse.html#exit-on-error say:

Normally, when you pass an invalid argument list to the parse_args() method of an ArgumentParser, it will exit with error info. If the user would like catch errors manually, the feature can be enable by setting exit_on_error to False:

This description _appears_ to be at odds with the observed behavior.

[tirkarthi]

I guess the docs by manually mean that ArgumentError will be raised when exit_on_error is False that can be handled. By default with exit_on_error being True self.error() which raises SystemExit and catching SystemExit can mask other errors. This was added in bpo-9938 with python/cpython#59567. There is also a typo in the docs that it should have used enabled instead of enable in "the feature can be enable by setting exit_on_error to False"

[00dcd123-3f50-4394-96eb-3e7b1086583c]

typo in the docs that it should have used enabled instead of enable

Well spotted, I'll happily fix this up.

I guess the docs by manually mean that ArgumentError will be raised when exit_on_error is False that can be handled.

To be clear, in this case, even with exit_onerror=False, ArgumentError is _not being raised, but SystemExit is.

[00dcd123-3f50-4394-96eb-3e7b1086583c]

I've attached a patch containing tests showing the current behavior, namely that exit_on_error does not change the behavior of argparse.ArgumentParser.parse_args in either case:

• An unrecognized option is given
• A required option is not given

Should the docs be updated to note this?

[7a064fe6-c535-4d80-a11f-a04ed39056c5]

I didn't pay attention to the patch that added this "exit_on_error" parameter. So I don't know exactly what error handling it was supposed to handle or why. But given the location of the code change, it does not handle every possible error.

Specifically it's in parser.parse_known_args() where it calls _parse_known_args(). With this parameter True, a argparse.ArgumentError is caught and converted to parse.error() call. That's the original behavior.

With False, there's no special handling for ArgumentError. Catching that is left to the developer, as illustrated in the docs.

In the documented example, it's a 'type' error. 'choices' would also behave this way. 'nargs' errors also. But not all errors are handled like this.

Inside _parse_known_args(), self.error() is called several times, once for 'required' arguments failure, and for a required mutually_exclusive_group error. I count 9 self.error() calls; exit_on_error only changes one of those.

The error highlighted in this issue is called in parser.parse_args(). This calls parse_known_args(), and raises an error if there are 'extras', unrecognized strings.

So clearly the new docs is is describing this new parameter in overly broad terms. It only changes the handling of a subset of parser.error() calls. Off hand I can't think of clean way of refining the description without getting overly technical about the error handling.

Developers already had the documented option of changing the parse.error() and parse.exit() methods.

[7a064fe6-c535-4d80-a11f-a04ed39056c5]

For custom handling of unrecognized arguments, use parser_known_args(). You don't need this new parameter.

[7a064fe6-c535-4d80-a11f-a04ed39056c5]

The docs could change

"catch errors manually"

to

"catch ArgumentError manually"

But while 'argparse.ArgumentError' is imported, it is not documented. We have to study the code to learn when it is raised.

Its class def:

    def __init__(self, argument, message):

shows it's tied to a specific 'argument', an Action object. Most commonly it is produced by reraising a ValueError, TypeError or ArgumentTypeError during the check_values step.

Unrecognized arguments, and missing required arguments errors aren't produced while processing an argument, but rather while checking things after parsing. So they can't raise an ArgumentError, and aren't handled by this new parameter.

I found a old issue that discusses this https://bugs.python.org/issue9938, https://github.com/python/cpython/pull/15362

There wasn't much discussion about the scope of this change, or about the documentation wording. My only comment was in 2013, https://bugs.python.org/msg192147

Until we iron out the wording I think this patch should be reverted.

While exploring other problems, I thought it would be a good idea of refactor parse_known_args and _parse_known_args. Specifically I'd move the 'required' testing and self.error() calls out of _parse_known_args, allowing a developer to write their own versions of parse_known_args. The goal was to make it easier to test for mixes of seen and unseen arguments.

In light of the current issue, we might want to look into consolidating all (or at least most) of the calls to self.error() in one function. Until then, the documented idea of modifying the error() method itself is the best user/developer tool, https://docs.python.org/3.10/library/argparse.html#exiting-methods

[b33cfc3a-3200-4fb5-ba1b-de7f13700717]

I'm new to Python bugtracker and I may misunderstand the discussion. But I think this is a real bug in argparse, not a documentation problem.

My usecase was that I wanted to add argparse to a GUI application where print and exit is a wrong option. So I set argparse.ArgumentParser(exit_on_error=False) but I failed with that, system exited whatever I tried. Exit in this case is not an option, I expected a dedicated exception after that.

Digging a bit deeper I found that I can create a custom class and overwrite the ArgumentParser.exit(status=0, message=None) but still, as a simple user of the standard library why should I do that?

This would be a minimalist example how this can be solved in argparse code:

    def exit(self, status=0, message=None):
        if message:
            self._print_message(message, _sys.stderr)
        if self.exit_on_error:
            _sys.exit(status)
        else:
            raise ArgumentError(...)

But considering GUI or interactive usage this is still not enough, sys.stdout and sys.stderr is written, that do not exists in GUI case, so these parts also need some re-design.

[c37a9ee8-4404-4fda-a826-0206b10b669a]

I agree with Bigbird and paul.j3.

But I think this is a real bug in argparse, not a documentation problem.

Off hand I can't think of clean way of refining the description without getting overly technical about the error handling.

It seems like a reasonable conclusion to make that, "If the user would like to catch errors manually, the feature can be enabled by setting exit_on_error to False" indicates that wrapping any call to parser.parse_args() or parser.parse_known_args() will catch any known error that may raised. So outside of adding the workaround of subclassing ArgumentParser to the documentation, this probably needs a patch to the code.

Any solution will probably also need to implement a new error type to be able to handle these cases since they can be caused by multiple arguments being included / excluded, which is not something that ArgumentError can adequately describe by referencing only a single argument. Something like:

class MultipleArgumentError(ArgumentError):

    def __init__(self, arguments, message):
        self.argument_names = filter([_get_action_name(arg) for arg in arguments], lambda name: name)
        self.message = message

    def __str__(self):
        if self.argument_names is None:
            format = '%(message)s'
        else:
            format = 'argument %(argument_names): %(message)s'
        return format % dict(message=self.message,
                             argument_names=', '.join(self.argument_name))

I'm not sure I like the idea of changing the exit or error methods since they have a a clear purpose and don't need to be repurposed to also include error handling. It seems to me that adding checks to self.exit_on_error in _parse_known_args to handle the missing required arguments and in parse_args to handle unknown arguments is probably a quick and clean solution.

[7a064fe6-c535-4d80-a11f-a04ed39056c5]

In

https://stackoverflow.com/questions/69108632/unable-to-catch-exception-error-for-argparse

it looked like exit_on_error does not work when using subparsers. On on further thought, I realized that it has to included in the definition of the subparser. As with other ArgumentParser parameters, the subparsers does not inherit from the main parser.

So it's basically a documentation issue. The add_parser method is described briefly as:

 which takes a command name and any ArgumentParser constructor 
 arguments, and returns an ArgumentParser object that can be 
 modified as usual.

But as my experience shows, its relevance is easily missed, even by an experienced users.

[ed-ij]

Just wanted to bump this after attempting to use the exit_on_error argument to prevent sys.exit() being called.

The documentation could at least be improved until a better implementation is available. Adding a note in the documentation for this function which links to the "Exiting methods" section would also be helpful in the interim although I agree with bigbirdcode and joshmeranda that you shouldn't have to override methods just to have control over error handling.

[serhiy-storchaka]

I wrote #121056 before discovering this issue. It is basically an extended version of #30832.

We now have 3 duplicate issues with 4 open PRs (and one incomplete PR was already merged). It is time to finish this.