GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-108791: Fix `pdb` CLI invalid argument handling by chgnrdv · Pull Request #108816 · python/cpython

Conversation

@chgnrdv

@chgnrdv chgnrdv commented Sep 2, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #108791.
pdb module, if invoked with invalid command line arguments, produces large traceback and/or tries to run debugger on errorneus target, such as directory. This patch improves error handling in pdb CLI, making error messages more concise.

  • Issue: pdb CLI doesn't handle incorrect arguments properly #108791

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@chgnrdv
Fix pdb command line interface error handling
e515e7c 
`pdb` module, if invoked with invalid command line arguments, produces large traceback and/or tries to run debugger on errorneus target, such as directory. This patch improves error handling in pdb CLI, making error messages more concise.
@bedevere-bot bedevere-bot mentioned this pull request Sep 2, 2023
pdb CLI doesn't handle incorrect arguments properly #108791
Closed
2 tasks done
@bedevere-bot bedevere-bot added the awaiting review label Sep 2, 2023
blurb-it Bot and others added 2 commits September 2, 2023 16:07
@blurb-it
📜🤖 Added by blurb_it.
601d2f4
@chgnrdv
Merge branch 'main' into pdb-cli-fix-error-handling
d55710e
@chgnrdv

chgnrdv commented Sep 2, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I had to make changes in test_module_without_a_main, as it was taking error message from context exception.

Traceback (most recent call last):
  File "/home/radislav/projects/cpython/Lib/runpy.py", line 148, in _get_module_details
    return _get_module_details(pkg_main_name, error)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/radislav/projects/cpython/Lib/runpy.py", line 142, in _get_module_details
    raise error("No module named %s" % mod_name)
ImportError: No module named t_main.__main__

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/radislav/projects/cpython/Lib/pdb.py", line 166, in check
    self._details
  File "/home/radislav/projects/cpython/Lib/functools.py", line 1014, in __get__
    val = self.func(instance)
          ^^^^^^^^^^^^^^^^^^^
  File "/home/radislav/projects/cpython/Lib/pdb.py", line 174, in _details
    return runpy._get_module_details(self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/radislav/projects/cpython/Lib/runpy.py", line 152, in _get_module_details
    raise error(("%s; %r is a package and cannot " +
ImportError: No module named t_main.__main__; 't_main' is a package and cannot be directly executed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood
AlexWaygood requested a review from gaogaotiantian September 2, 2023 16:15
chgnrdv added 3 commits September 2, 2023 19:25
@chgnrdv
fixed tests
256a955
@chgnrdv
Merge branch 'pdb-cli-fix-error-handling' of github.com:chgnrdv/cpyth…
de830e3 
…on into pdb-cli-fix-error-handling
@chgnrdv
changed test_module_without_a_main
77ef451
sunmy2019
sunmy2019 reviewed Sep 2, 2023
View reviewed changes
Comment thread Lib/pdb.py Outdated
except Exception:
traceback.print_exc()
except Exception as e:
print(f"{type(e).__name__}: {e}")

@sunmy2019 sunmy2019 Sep 2, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make it something like

        try:
            self._details
        except ... : # Something you expect
            print(f"{type(e).__name__}: {e}")
        except Exception:
            traceback.print_exc()

to leave the general case untouched.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@chgnrdv chgnrdv Sep 2, 2023

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the runpy._get_module_details source, ImportError seems to be the only type of exception that can be raised here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@chgnrdv
changed _ModuleTarget.check to handle only ImportError and print trac…
7f4c649 
…eback otherwise
gaogaotiantian
gaogaotiantian approved these changes Sep 3, 2023
View reviewed changes

@gaogaotiantian gaogaotiantian left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the improvement is fine. The argument parser piece is a bit hacky, but I'm planning to convert that part to argparse anyway (getopt has not been updated for like 10 years) so it's okay.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
chgnrdv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Sep 3, 2023
@gaogaotiantian

gaogaotiantian commented Oct 7, 2023

Copy link
Copy Markdown
Member

Hi @chgnrdv , as I updated pdb to use argparse instead of getopt, the argument parsing piece might be fixed (more importantly, the code in this PR was definitely outdated). Could you resolve the conflicts in this PR, hopefully keep the relevant tests? I'll try to find the core dev to review this once it's fixed. Thanks.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@chgnrdv

chgnrdv commented Oct 7, 2023

Copy link
Copy Markdown
Contributor Author

@gaogaotiantian, thanks for notice, I'll take a look tomorrow.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@chgnrdv
adapted fix/test to changes in pdb CLI
506ec9d
@chgnrdv
chgnrdv requested a review from gaogaotiantian October 10, 2023 14:07
gaogaotiantian
gaogaotiantian approved these changes Oct 11, 2023
View reviewed changes
@gaogaotiantian

gaogaotiantian commented Oct 11, 2023

Copy link
Copy Markdown
Member

LGTM, @iritkatriel could you take a look? Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gaogaotiantian
gaogaotiantian requested a review from iritkatriel October 11, 2023 04:25
iritkatriel
iritkatriel reviewed Oct 11, 2023
View reviewed changes
Comment thread Lib/test/test_pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
chgnrdv added 2 commits October 11, 2023 14:25
@chgnrdv
Merge branch 'main' into pdb-cli-fix-error-handling
4339575
@chgnrdv
assert in test_nonexistent_module that module doesn't exist
e6cdfce
@chgnrdv
chgnrdv requested a review from iritkatriel October 15, 2023 19:31
iritkatriel
iritkatriel approved these changes Oct 15, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 15, 2023
@iritkatriel

iritkatriel commented Oct 15, 2023

Copy link
Copy Markdown
Member

LGTM.

Should this be backported?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gaogaotiantian

gaogaotiantian commented Oct 15, 2023

Copy link
Copy Markdown
Member

Rare case, but safe and backward compatible. I think it should be back-ported.
sunmy2019 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sunmy2019

sunmy2019 commented Oct 16, 2023

Copy link
Copy Markdown
Member

Agreed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sunmy2019
sunmy2019 approved these changes Oct 16, 2023
View reviewed changes
@iritkatriel iritkatriel added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 16, 2023
@iritkatriel
iritkatriel merged commit 162213f into python:main Oct 16, 2023
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 16, 2023
@miss-islington

miss-islington commented Oct 16, 2023

Copy link
Copy Markdown
Contributor

Thanks @chgnrdv for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 16, 2023
@chgnrdv @miss-islington
pythongh-108791: Fix pdb CLI invalid argument handling (pythonGH-10…
7e69c98 
…8816)

(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 16, 2023

Copy link
Copy Markdown

GH-110915 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 16, 2023
@bedevere-app

bedevere-app Bot commented Oct 16, 2023

Copy link
Copy Markdown

GH-110916 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Oct 16, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 16, 2023
@chgnrdv @miss-islington
pythongh-108791: Fix pdb CLI invalid argument handling (pythonGH-10…
4ac4b07 
…8816)

(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 19, 2023

Copy link
Copy Markdown

GH-111063 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Oct 19, 2023

Copy link
Copy Markdown

GH-111064 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel pushed a commit that referenced this pull request Oct 19, 2023
@chgnrdv
[3.12] gh-108791: Fix pdb CLI invalid argument handling (GH-108816) (#…
68a5640 
…111064)

* [3.12] gh-108791: Fix `pdb` CLI invalid argument handling (GH-108816)
(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <[email protected]>
iritkatriel pushed a commit that referenced this pull request Oct 19, 2023
@chgnrdv
[3.11] gh-108791: Fix pdb CLI invalid argument handling (GH-108816) (#…
26a0282 
…111063)

* [3.11] gh-108791: Fix `pdb` CLI invalid argument handling (GH-108816)
(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@chgnrdv @aisk
pythongh-108791: Fix pdb CLI invalid argument handling (python#108816)
72a98ad
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@chgnrdv @Glyphack
pythongh-108791: Fix pdb CLI invalid argument handling (python#108816)
643f3fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes
@gaogaotiantian gaogaotiantian gaogaotiantian approved these changes
@sunmy2019 sunmy2019 sunmy2019 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pdb CLI doesn't handle incorrect arguments properly

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@chgnrdv @gaogaotiantian @iritkatriel @sunmy2019 @miss-islington @bedevere-bot
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.