Turn a bare AssertionError into a real error message - #1944
Open
afonsojanu wants to merge 2 commits into
Open
Conversation
An optional argument sitting between METHOD/URL and the first REQUEST_ITEM splits the positional run argparse sees into two pieces. On Python 3.13, argparse fills each piece's slots independently, so a command like `http POST --auth-type bearer --auth token URL` ends up with METHOD unset, URL holding the string "POST", and the actual URL misparsed as the first REQUEST_ITEM instead. _guess_method assumed that whenever METHOD came back unset, request_items would still be empty, and asserted exactly that, so this specific misparse crashed with a bare AssertionError and no indication of what went wrong. The assertion is now a proper user-facing error explaining that METHOD, URL and REQUEST_ITEM have to sit next to each other with nothing else in between, matching what the reporter said they actually expected to see. Added a regression test reproducing the exact command from the report, checking it exits with an error and prints the new message instead of crashing.
The reported crash only happens because argparse's own handling of a split positional run changed in 3.13: on every earlier version the same command line gets rejected upstream with its own 'unrecognized arguments' error, well before _guess_method ever runs, so there's nothing for this test to exercise there. CI caught it running the suite across the full version matrix, where the assertion on the new error message failed on 3.8 through 3.12 since the command never reaches that code path on those versions at all.
Author
|
CI caught something the local run on Python 3.13 didn't: the new regression test only reproduces on 3.13+, since that's the version where argparse fills a split positional run the way the report describes. On 3.8 through 3.12 the same command gets rejected earlier, with argparse's own 'unrecognized arguments' error, so the assertion never gets exercised there. Gated the test on Python 3.13+ so it stays meaningful without failing on versions where it can't reproduce the original crash in the first place. Confirmed the full suite is green on both 3.11 (test now skips, 1017 passed) and 3.13 (test passes). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1614
http POST --auth-type bearer --auth token URLcrashes with a bareAssertionErroron Python 3.13, instead of running the request or telling the caller what's wrong with the invocation.The cause is how argparse buckets tokens once an optional argument sits between METHOD/URL and the first REQUEST_ITEM. It splits the positional run into two separate pieces and, on 3.13, fills each piece's slots independently:
POSTand everything up to the first option land in the METHOD/URL slots, and the actual URL after the options gets read back as the first REQUEST_ITEM, leaving METHOD unset._guess_methodassumed METHOD being unset always meantrequest_itemswas still empty and asserted exactly that, so this specific misparse hit the assertion directly.The assertion is now a real error explaining that METHOD, URL and REQUEST_ITEM have to sit next to each other with nothing else in between, which is what the report itself expected to see instead of a traceback.
Added a regression test with the exact command from the issue, checking it exits with an error and prints the new message rather than crashing. Ran the full suite on Python 3.13 (where this reproduces) plus 3.11: 1018 passed, 2 pre-existing failures unrelated to this change (big5 charset detection in test_encoding.py, reproduces identically on unmodified master). flake8 clean on both changed files.