From e515e7c62df05a787a5869b2c1a83a25e997036c Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Sat, 2 Sep 2023 18:57:11 +0300 Subject: [PATCH 1/6] Fix pdb command line interface error handling `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. --- Lib/pdb.py | 13 ++++++++++--- Lib/test/test_pdb.py | 18 +++++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index 90f26a2eb99848..d6f8c1394af464 100755 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -138,6 +138,9 @@ def check(self): if not os.path.exists(self): print('Error:', self.orig, 'does not exist') sys.exit(1) + if os.path.isdir(self): + print('Error:', self.orig, 'is a directory') + sys.exit(1) # Replace pdb's dir with script's dir in front of module search path. sys.path[0] = os.path.dirname(self) @@ -164,8 +167,8 @@ class _ModuleTarget(str): def check(self): try: self._details - except Exception: - traceback.print_exc() + except Exception as e: + print(f"{type(e).__name__}: {e}") sys.exit(1) @functools.cached_property @@ -2057,7 +2060,11 @@ def help(): def main(): import getopt - opts, args = getopt.getopt(sys.argv[1:], 'mhc:', ['help', 'command=']) + try: + opts, args = getopt.getopt(sys.argv[1:], 'mhc:', ['help', 'command=']) + except getopt.GetoptError as e: + print(f"Error: {e}") + sys.exit(1) if not args: print(_usage) diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py index 734b5c83cdff7d..60e2222e81e8bc 100644 --- a/Lib/test/test_pdb.py +++ b/Lib/test/test_pdb.py @@ -2629,7 +2629,7 @@ def test_module_without_a_main(self): stdout, stderr = self._run_pdb( ['-m', module_name], "", expected_returncode=1 ) - self.assertIn("ImportError: No module named t_main.__main__", + self.assertIn("ImportError: No module named t_main.__main__; 't_main' is a package and cannot be directly executed", stdout.splitlines()) def test_package_without_a_main(self): @@ -2648,6 +2648,22 @@ def test_package_without_a_main(self): "'t_pkg.t_main' is a package and cannot be directly executed", stdout) + def test_nonexistent_module(self): + stdout, stderr = self._run_pdb(["-m", os_helper.TESTFN], "", expected_returncode=1) + self.assertIn(f"ImportError: No module named {os_helper.TESTFN}", stdout.splitlines()) + + def test_dir_as_script(self): + os.mkdir(os_helper.TESTFN) + stdout, stderr = self._run_pdb([os_helper.TESTFN], "", expected_returncode=1) + self.assertIn(f"Error: {os_helper.TESTFN} is a directory", stdout.splitlines()) + os.rmdir(os_helper.TESTFN) + + def test_invalid_cmd_line_options(self): + stdout, stderr = self._run_pdb(["-c"], "", expected_returncode=1) + self.assertIn(f"Error: option -c requires argument", stdout.splitlines()) + stdout, stderr = self._run_pdb(["--spam"], "", expected_returncode=1) + self.assertIn(f"Error: option --spam not recognized", stdout.splitlines()) + def test_blocks_at_first_code_line(self): script = """ #This is a comment, on line 2 From 601d2f49ebecfeca2116039be1684b55033e8916 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 2 Sep 2023 16:07:25 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2023-09-02-16-07-23.gh-issue-108791.fBcAqh.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2023-09-02-16-07-23.gh-issue-108791.fBcAqh.rst diff --git a/Misc/NEWS.d/next/Library/2023-09-02-16-07-23.gh-issue-108791.fBcAqh.rst b/Misc/NEWS.d/next/Library/2023-09-02-16-07-23.gh-issue-108791.fBcAqh.rst new file mode 100644 index 00000000000000..84a2cd589e10d5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-09-02-16-07-23.gh-issue-108791.fBcAqh.rst @@ -0,0 +1 @@ +Improved error handling in :mod:`pdb` command line interface, making it produce more concise error messages. From 256a9558bf85f2acbf29f83e9b347e08a2481efc Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Sat, 2 Sep 2023 19:25:03 +0300 Subject: [PATCH 3/6] fixed tests --- Lib/test/test_pdb.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py index 60e2222e81e8bc..c48b3860d7fa11 100644 --- a/Lib/test/test_pdb.py +++ b/Lib/test/test_pdb.py @@ -2650,19 +2650,18 @@ def test_package_without_a_main(self): def test_nonexistent_module(self): stdout, stderr = self._run_pdb(["-m", os_helper.TESTFN], "", expected_returncode=1) - self.assertIn(f"ImportError: No module named {os_helper.TESTFN}", stdout.splitlines()) + self.assertIn(f"ImportError: No module named {os_helper.TESTFN}", stdout) def test_dir_as_script(self): - os.mkdir(os_helper.TESTFN) - stdout, stderr = self._run_pdb([os_helper.TESTFN], "", expected_returncode=1) - self.assertIn(f"Error: {os_helper.TESTFN} is a directory", stdout.splitlines()) - os.rmdir(os_helper.TESTFN) + with os_helper.temp_dir() as temp_dir: + stdout, stderr = self._run_pdb([temp_dir], "", expected_returncode=1) + self.assertIn(f"Error: {temp_dir} is a directory", stdout) def test_invalid_cmd_line_options(self): stdout, stderr = self._run_pdb(["-c"], "", expected_returncode=1) - self.assertIn(f"Error: option -c requires argument", stdout.splitlines()) + self.assertIn(f"Error: option -c requires argument", stdout) stdout, stderr = self._run_pdb(["--spam"], "", expected_returncode=1) - self.assertIn(f"Error: option --spam not recognized", stdout.splitlines()) + self.assertIn(f"Error: option --spam not recognized", stdout) def test_blocks_at_first_code_line(self): script = """ From 77ef45147aded1b96191a5e1ac93aea77301cddf Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Sat, 2 Sep 2023 20:40:26 +0300 Subject: [PATCH 4/6] changed test_module_without_a_main --- Lib/test/test_pdb.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py index c48b3860d7fa11..a434f1bdd914a6 100644 --- a/Lib/test/test_pdb.py +++ b/Lib/test/test_pdb.py @@ -2629,8 +2629,7 @@ def test_module_without_a_main(self): stdout, stderr = self._run_pdb( ['-m', module_name], "", expected_returncode=1 ) - self.assertIn("ImportError: No module named t_main.__main__; 't_main' is a package and cannot be directly executed", - stdout.splitlines()) + self.assertIn("ImportError: No module named t_main.__main__;", stdout) def test_package_without_a_main(self): pkg_name = 't_pkg' From 7f4c6497eb0e8fda662e727258d57f2f7cfed0fe Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Sat, 2 Sep 2023 22:10:18 +0300 Subject: [PATCH 5/6] changed _ModuleTarget.check to handle only ImportError and print traceback otherwise --- Lib/pdb.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/pdb.py b/Lib/pdb.py index d6f8c1394af464..9487c8f3184207 100755 --- a/Lib/pdb.py +++ b/Lib/pdb.py @@ -167,8 +167,11 @@ class _ModuleTarget(str): def check(self): try: self._details - except Exception as e: - print(f"{type(e).__name__}: {e}") + except ImportError as e: + print(f"ImportError: {e}") + sys.exit(1) + except Exception: + traceback.print_exc() sys.exit(1) @functools.cached_property From e6cdfce4f01abfda58ea5f4504473ecbef7b32ef Mon Sep 17 00:00:00 2001 From: Radislav Chugunov Date: Wed, 11 Oct 2023 14:46:57 +0300 Subject: [PATCH 6/6] assert in test_nonexistent_module that module doesn't exist --- Lib/test/test_pdb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py index 70c9debddcdf55..705cb5274375f6 100644 --- a/Lib/test/test_pdb.py +++ b/Lib/test/test_pdb.py @@ -2777,6 +2777,7 @@ def test_package_without_a_main(self): stdout) def test_nonexistent_module(self): + assert not os.path.exists(os_helper.TESTFN) stdout, stderr = self._run_pdb(["-m", os_helper.TESTFN], "", expected_returncode=1) self.assertIn(f"ImportError: No module named {os_helper.TESTFN}", stdout)