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

gh-108791: Fix pdb CLI invalid argument handling #108816

Merged
merged 10 commits into from
Oct 16, 2023

Conversation

chgnrdv
Copy link
Contributor

@chgnrdv chgnrdv commented Sep 2, 2023

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.

`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.
@chgnrdv
Copy link
Contributor Author

chgnrdv commented Sep 2, 2023

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

Lib/pdb.py Outdated
except Exception:
traceback.print_exc()
except Exception as e:
print(f"{type(e).__name__}: {e}")
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

@gaogaotiantian gaogaotiantian left a 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.

@gaogaotiantian
Copy link
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.

@chgnrdv
Copy link
Contributor Author

chgnrdv commented Oct 7, 2023

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

@gaogaotiantian
Copy link
Member

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

@iritkatriel
Copy link
Member

LGTM.

Should this be backported?

@gaogaotiantian
Copy link
Member

Rare case, but safe and backward compatible. I think it should be back-ported.

@sunmy2019
Copy link
Member

Agreed

@iritkatriel iritkatriel added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Oct 16, 2023
@iritkatriel iritkatriel merged commit 162213f into python:main Oct 16, 2023
28 checks passed
@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 16, 2023
…8816)

(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 16, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Oct 16, 2023
@bedevere-app
Copy link

bedevere-app bot commented Oct 16, 2023

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

@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
…8816)

(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
chgnrdv added a commit to chgnrdv/cpython that referenced this pull request Oct 19, 2023
…onGH-108816)

(cherry picked from commit 162213f)

Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 19, 2023

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

@bedevere-app
Copy link

bedevere-app bot commented Oct 19, 2023

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

iritkatriel pushed a commit that referenced this pull request Oct 19, 2023
…111064)

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

Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
iritkatriel pushed a commit that referenced this pull request Oct 19, 2023
…111063)

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

Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pdb CLI doesn't handle incorrect arguments properly
6 participants