Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argparse.ArgumentParser.parses_args does not honor exit_on_error=False when given unrecognized arguments #121018

Closed
blhsing opened this issue Jun 26, 2024 · 5 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@blhsing
Copy link
Contributor

blhsing commented Jun 26, 2024

Bug report

Bug description:

As reported in Discourse, even though the documentation on the exit_on_error argument for argparse.ArgumentParser says:

exit_on_error - Determines whether or not ArgumentParser exits with error info when an error occurs. (default: True)

The parse_args method would still exit with error info when there are unrecognized arguments:

import argparse
parser = argparse.ArgumentParser(exit_on_error=False)
try:
    parser.parse_args('invalid arguments'.split())
except argparse.ArgumentError:
    print('ArgumentError caught.')

which outputs:

usage: test.py [-h]
test.py: error: unrecognized arguments: invalid arguments

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Linux, Windows

Linked PRs

@blhsing blhsing added the type-bug An unexpected behavior, bug, or error label Jun 26, 2024
serhiy-storchaka pushed a commit that referenced this issue Jun 26, 2024
…raises instead of exiting when given unrecognized arguments (GH-121019)
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 26, 2024
…False raises instead of exiting when given unrecognized arguments (pythonGH-121019)

(cherry picked from commit 0654336)

Co-authored-by: blhsing <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 26, 2024
…False raises instead of exiting when given unrecognized arguments (pythonGH-121019)

(cherry picked from commit 0654336)

Co-authored-by: blhsing <[email protected]>
serhiy-storchaka pushed a commit that referenced this issue Jun 26, 2024
…=False raises instead of exiting when given unrecognized arguments (GH-121019) (GH-121032)

(cherry picked from commit 0654336)

Co-authored-by: blhsing <[email protected]>
serhiy-storchaka pushed a commit that referenced this issue Jun 26, 2024
…=False raises instead of exiting when given unrecognized arguments (GH-121019) (GH-121031)

(cherry picked from commit 0654336)

Co-authored-by: blhsing <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 26, 2024
…rror=False

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at higher level and error() is called if
  exit_on_error is true.
@serhiy-storchaka
Copy link
Member

Actually, this is more complex issue. error() is called in many other places, and it does not honor exit_on_error=False.

In most cases it can be replaced with raise ArgumentError. It is caught at higher level and error() is called if exit_on_error is true.

In some cases ValueError may be more appropriate exception than ArgumentError, but this is another issue.

@blhsing
Copy link
Contributor Author

blhsing commented Jun 27, 2024

Actually, this is more complex issue. error() is called in many other places, and it does not honor exit_on_error=False.

In most cases it can be replaced with raise ArgumentError. It is caught at higher level and error() is called if exit_on_error is true.

In some cases ValueError may be more appropriate exception than ArgumentError, but this is another issue.

Ah yes good catch. I think ArgumentError is meant to be raised by invalid arguments given by the end user. In cases where the error is caused by an improper setup of a parser or an option from the developer, a ValueError would indeed be more appropriate.

@blhsing
Copy link
Contributor Author

blhsing commented Jun 27, 2024

On second thought, I think it may be cleaner to simply put the if self.exit_on_error: ... else: raise ArgumentError logics directly in the error method, and call self.error only when it is caused by the end user.

@serhiy-storchaka
Copy link
Member

There are already open issues for this bug: #85427 and #103498. It is my fault that I did not recognize duplicates.

Yes, putting the logic directly in the error method could simplify some cases, but we want exit_on_error=False to work even with user-overridden error method.

serhiy-storchaka added a commit that referenced this issue Jun 28, 2024
…alse (GH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
@blhsing
Copy link
Contributor Author

blhsing commented Jun 28, 2024

There are already open issues for this bug: #85427 and #103498. It is my fault that I did not recognize duplicates.

Ah you're right. I forgot to search for prior bug reports before filing mine. Sorry.

Yes, putting the logic directly in the error method could simplify some cases, but we want exit_on_error=False to work even with user-overridden error method.

Good point. Will keep the current approach then. Thanks!

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 28, 2024
…it_on_error=False (pythonGH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
(cherry picked from commit 81a654a)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jun 28, 2024
…it_on_error=False (pythonGH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
(cherry picked from commit 81a654a)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this issue Jun 28, 2024
…error=False (GH-121056) (GH-121129)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
(cherry picked from commit 81a654a)
serhiy-storchaka added a commit that referenced this issue Jun 28, 2024
…error=False (GH-121056) (GH-121128)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
(cherry picked from commit 81a654a)
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…False raises instead of exiting when given unrecognized arguments (pythonGH-121019)
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…rror=False (pythonGH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
colesbury added a commit that referenced this issue Jul 8, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…False raises instead of exiting when given unrecognized arguments (pythonGH-121019)
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…rror=False (pythonGH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…False raises instead of exiting when given unrecognized arguments (pythonGH-121019)
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…rror=False (pythonGH-121056)

* parse_intermixed_args() now raises ArgumentError instead of calling
  error() if exit_on_error is false.
* Internal code now always raises ArgumentError instead of calling
  error(). It is then caught at the higher level and error() is called if
  exit_on_error is true.
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
Status: Doc issues
Development

No branches or pull requests

2 participants