Title: Argparse.parse_args exits on unrecognized option with exit_on_error=False
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.9
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: bigbird, joshmeranda, mhughes, paul.j3, rhettinger, xtreak
Priority: normal Keywords: patch

Created on 2020-07-09 08:40 by mhughes, last changed 2021-09-11 05:39 by paul.j3.

File name Uploaded Description Edit
exit_on_error_tests.patch mhughes, 2020-07-09 13:59
Pull Requests
URL Status Linked Edit
PR 27295 open joshmeranda, 2021-07-22 21:43
Messages (10)
msg373382 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 08:40
>>> import argparse
    >>> parser = argparse.ArgumentParser(exit_on_error=False)
    >>> parser.parse_args(["--unknown"])
    usage: [-h]
    : error: unrecognized arguments: --unknown

The docs 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.
msg373383 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2020-07-09 09:05
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 GH-15362. 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"
msg373385 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 09:51
> 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_on_error=False, ArgumentError is _not_ being raised, but SystemExit is.
msg373405 - (view) Author: Matthew Hughes (mhughes) * Date: 2020-07-09 13:59
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?
msg373437 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 03:01
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.
msg373440 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 04:39
For custom handling of unrecognized arguments, use parser_known_args().  You don't need this new parameter.
msg373470 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2020-07-10 16:41
The docs could change 

"catch errors manually"


"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,

There wasn't much discussion about the scope of this change, or about the documentation wording.  My only comment was in 2013,

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,
msg396345 - (view) Author: Peter V (bigbird) Date: 2021-06-22 14:59
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:
            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.
msg397975 - (view) Author: Josh Meranda (joshmeranda) * Date: 2021-07-22 05:15
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'
            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.
msg401629 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2021-09-11 05:39

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.
Date User Action Args
2021-09-11 05:39:59paul.j3setmessages: - msg401554
2021-09-11 05:39:07paul.j3setmessages: + msg401629
2021-09-10 04:42:35paul.j3setmessages: + msg401554
2021-07-22 21:43:27joshmerandasetstage: patch review
pull_requests: + pull_request25838
2021-07-22 05:15:06joshmerandasetnosy: + joshmeranda

messages: + msg397975
versions: + Python 3.9, - Python 3.10
2021-06-22 14:59:21bigbirdsetnosy: + bigbird
messages: + msg396345
2020-07-10 16:41:46paul.j3setmessages: + msg373470
2020-07-10 04:39:09paul.j3setmessages: + msg373440
2020-07-10 03:01:12paul.j3setmessages: + msg373437
2020-07-09 13:59:51mhughessetfiles: + exit_on_error_tests.patch
keywords: + patch
messages: + msg373405
2020-07-09 09:51:16mhughessetmessages: + msg373385
versions: - Python 3.9
2020-07-09 09:05:01xtreaksetnosy: + rhettinger, xtreak, paul.j3

messages: + msg373383
versions: + Python 3.9
2020-07-09 08:40:33mhughescreate