From e4ebd4f68ce19f29d319750793b9475e068922eb Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sat, 18 Jul 2026 16:46:38 +0300 Subject: [PATCH 1/5] gh-153970: Fix CalledProcessError.__str__ crash when returncode is None CalledProcessError.__str__ fell through to a branch that formats the return code with %d, which raises TypeError when returncode is None. Handle the None case explicitly and return a message reporting an unknown exit status. --- Lib/subprocess.py | 4 +++- Lib/test/test_subprocess.py | 4 ++++ .../Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 6fe2ec98fb4088..82d9da444fc809 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -143,7 +143,9 @@ def __init__(self, returncode, cmd, output=None, stderr=None): self.stderr = stderr def __str__(self): - if self.returncode and self.returncode < 0: + if self.returncode is None: + return "Command %r returned an unknown exit status." % (self.cmd,) + elif self.returncode < 0: try: return "Command %r died with %r." % ( self.cmd, signal.Signals(-self.returncode)) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d066ae85dfc51a..14c67f15960f6f 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2449,6 +2449,10 @@ def test_CalledProcessError_str(self): err = subprocess.CalledProcessError(-9876543, "fake cmd") self.assertEqual(str(err), "Command 'fake cmd' died with unknown signal 9876543.") + # unknown return code (must not raise from __str__ itself) + err = subprocess.CalledProcessError(None, "fake cmd") + self.assertEqual(str(err), "Command 'fake cmd' returned an unknown exit status.") + def test_preexec(self): # DISCLAIMER: Setting environment variables is *not* a good use # of a preexec_fn. This is merely a test. diff --git a/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst b/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst new file mode 100644 index 00000000000000..5431a7d75feb1c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst @@ -0,0 +1,3 @@ +Calling :func:`str` on a :exc:`subprocess.CalledProcessError` whose +:attr:`!returncode` is ``None`` no longer raises :exc:`TypeError`. It now +returns a message reporting an unknown exit status. From 716206645a2e1ed8ca7b817dda66b0b1e57798e5 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 19 Jul 2026 02:49:54 +0300 Subject: [PATCH 2/5] change implementation per comment by vstinner --- Lib/subprocess.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 82d9da444fc809..df23fb18a518b7 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -143,9 +143,7 @@ def __init__(self, returncode, cmd, output=None, stderr=None): self.stderr = stderr def __str__(self): - if self.returncode is None: - return "Command %r returned an unknown exit status." % (self.cmd,) - elif self.returncode < 0: + if self.returncode and self.returncode < 0: try: return "Command %r died with %r." % ( self.cmd, signal.Signals(-self.returncode)) @@ -153,8 +151,8 @@ def __str__(self): return "Command %r died with unknown signal %d." % ( self.cmd, -self.returncode) else: - return "Command %r returned non-zero exit status %d." % ( - self.cmd, self.returncode) + return (f"Command {self.cmd!r} returned non-zero " + f"exit status {self.returncode}.") @property def stdout(self): From fa1a5d61d80d91c405ef86224b8e4904e02a3f4b Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 19 Jul 2026 02:55:57 +0300 Subject: [PATCH 3/5] gh-153970: Align test and NEWS with the f-string message The implementation now formats a None returncode with an f-string ("returned non-zero exit status None.") rather than a dedicated branch, so update the test assertion and the NEWS wording to match. --- Lib/test/test_subprocess.py | 4 ++-- .../Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 14c67f15960f6f..db5d254d5310af 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2449,9 +2449,9 @@ def test_CalledProcessError_str(self): err = subprocess.CalledProcessError(-9876543, "fake cmd") self.assertEqual(str(err), "Command 'fake cmd' died with unknown signal 9876543.") - # unknown return code (must not raise from __str__ itself) + # None return code (must not raise TypeError from __str__ itself) err = subprocess.CalledProcessError(None, "fake cmd") - self.assertEqual(str(err), "Command 'fake cmd' returned an unknown exit status.") + self.assertEqual(str(err), "Command 'fake cmd' returned non-zero exit status None.") def test_preexec(self): # DISCLAIMER: Setting environment variables is *not* a good use diff --git a/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst b/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst index 5431a7d75feb1c..d57e0f6afdbb2e 100644 --- a/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst +++ b/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst @@ -1,3 +1,2 @@ -Calling :func:`str` on a :exc:`subprocess.CalledProcessError` whose -:attr:`!returncode` is ``None`` no longer raises :exc:`TypeError`. It now -returns a message reporting an unknown exit status. +Calling :func:`str` on a :exc:`subprocess.CalledProcessError` with a +:attr:`!returncode` of ``None`` no longer raises :exc:`TypeError`. From f3bd9910e88eb239df6c0554585d1452c31c6a40 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Wed, 22 Jul 2026 11:12:10 +0300 Subject: [PATCH 4/5] gh-153970: Document that CalledProcessError.returncode is an integer The earlier fix changed CalledProcessError.__str__ to tolerate a None returncode, but that only handled one non-integer value and would still fail on others, such as a returncode set to a string. The returncode attribute is meant to hold a process exit status, which subprocess always sets to an integer, so there is no real bug in __str__. This drops the code change and the extra test, and instead notes in the docs that returncode is always an integer. --- Doc/library/subprocess.rst | 4 ++-- Lib/subprocess.py | 4 ++-- Lib/test/test_subprocess.py | 4 ---- .../Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst | 2 -- 4 files changed, 4 insertions(+), 10 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index fe64daa3291d67..b929eb3e77c7ab 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -236,8 +236,8 @@ underlying :class:`Popen` interface can be used directly. .. attribute:: returncode - Exit status of the child process. If the process exited due to a - signal, this will be the negative signal number. + Exit status of the child process, always an integer. If the process + exited due to a signal, this will be the negative signal number. .. attribute:: cmd diff --git a/Lib/subprocess.py b/Lib/subprocess.py index df23fb18a518b7..6fe2ec98fb4088 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -151,8 +151,8 @@ def __str__(self): return "Command %r died with unknown signal %d." % ( self.cmd, -self.returncode) else: - return (f"Command {self.cmd!r} returned non-zero " - f"exit status {self.returncode}.") + return "Command %r returned non-zero exit status %d." % ( + self.cmd, self.returncode) @property def stdout(self): diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index db5d254d5310af..d066ae85dfc51a 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2449,10 +2449,6 @@ def test_CalledProcessError_str(self): err = subprocess.CalledProcessError(-9876543, "fake cmd") self.assertEqual(str(err), "Command 'fake cmd' died with unknown signal 9876543.") - # None return code (must not raise TypeError from __str__ itself) - err = subprocess.CalledProcessError(None, "fake cmd") - self.assertEqual(str(err), "Command 'fake cmd' returned non-zero exit status None.") - def test_preexec(self): # DISCLAIMER: Setting environment variables is *not* a good use # of a preexec_fn. This is merely a test. diff --git a/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst b/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst deleted file mode 100644 index d57e0f6afdbb2e..00000000000000 --- a/Misc/NEWS.d/next/Library/2026-07-18-16-46-25.gh-issue-153970.Vn4pR8.rst +++ /dev/null @@ -1,2 +0,0 @@ -Calling :func:`str` on a :exc:`subprocess.CalledProcessError` with a -:attr:`!returncode` of ``None`` no longer raises :exc:`TypeError`. From 19a86c29a001d63e23b9887e940d0849e7110d5e Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Wed, 22 Jul 2026 15:06:00 +0300 Subject: [PATCH 5/5] gh-153970: Validate the returncode type in CalledProcessError Calling str() on a CalledProcessError whose returncode was None raised TypeError, because the __str__ else branch formats the value with %d. A returncode of another type, such as a string, failed even earlier at the self.returncode < 0 comparison. The constructor now raises a clear TypeError when returncode is neither an integer nor None, so __str__ can never be handed a value it cannot format. The else branch also uses an f-string, so a None returncode renders as a message instead of crashing. One test that built the exception with an empty-string returncode is updated to pass an integer, which is what a real failed command produces. --- Doc/library/subprocess.rst | 6 +++++- Lib/subprocess.py | 10 ++++++++-- Lib/test/test_script_helper.py | 2 +- Lib/test/test_subprocess.py | 14 ++++++++++++++ .../2026-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst | 4 ++++ 5 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index b929eb3e77c7ab..5470d05bb11f91 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -236,7 +236,7 @@ underlying :class:`Popen` interface can be used directly. .. attribute:: returncode - Exit status of the child process, always an integer. If the process + Exit status of the child process, an integer. If the process exited due to a signal, this will be the negative signal number. .. attribute:: cmd @@ -260,6 +260,10 @@ underlying :class:`Popen` interface can be used directly. .. versionchanged:: 3.5 *stdout* and *stderr* attributes added + .. versionchanged:: 3.16 + The constructor now raises :exc:`TypeError` if *returncode* is + neither an integer nor ``None``. + .. _frequently-used-arguments: diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 6fe2ec98fb4088..7853413aff6f55 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -137,12 +137,18 @@ class CalledProcessError(SubprocessError): cmd, returncode, stdout, stderr, output """ def __init__(self, returncode, cmd, output=None, stderr=None): + if returncode is not None and not isinstance(returncode, int): + raise TypeError( + "returncode must be an integer or None, not " + f"{type(returncode).__name__}") self.returncode = returncode self.cmd = cmd self.output = output self.stderr = stderr def __str__(self): + # A None returncode is falsy, so it skips the '< 0' comparison + # below and is formatted by the final branch instead. if self.returncode and self.returncode < 0: try: return "Command %r died with %r." % ( @@ -151,8 +157,8 @@ def __str__(self): return "Command %r died with unknown signal %d." % ( self.cmd, -self.returncode) else: - return "Command %r returned non-zero exit status %d." % ( - self.cmd, self.returncode) + return (f"Command {self.cmd!r} returned non-zero " + f"exit status {self.returncode}.") @property def stdout(self): diff --git a/Lib/test/test_script_helper.py b/Lib/test/test_script_helper.py index eeea6c4842b488..3c5763b0b8374d 100644 --- a/Lib/test/test_script_helper.py +++ b/Lib/test/test_script_helper.py @@ -86,7 +86,7 @@ def tearDown(self): def test_interpreter_requires_environment_true(self, mock_check_call): with mock.patch.dict(os.environ): os.environ.pop('PYTHONHOME', None) - mock_check_call.side_effect = subprocess.CalledProcessError('', '') + mock_check_call.side_effect = subprocess.CalledProcessError(1, 'cmd') self.assertTrue(script_helper.interpreter_requires_environment()) self.assertTrue(script_helper.interpreter_requires_environment()) self.assertEqual(1, mock_check_call.call_count) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index d066ae85dfc51a..d3368a745fc374 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2449,6 +2449,20 @@ def test_CalledProcessError_str(self): err = subprocess.CalledProcessError(-9876543, "fake cmd") self.assertEqual(str(err), "Command 'fake cmd' died with unknown signal 9876543.") + # None return code (must not raise TypeError from __str__ itself) + err = subprocess.CalledProcessError(None, "fake cmd") + self.assertEqual(str(err), "Command 'fake cmd' returned non-zero exit status None.") + + def test_CalledProcessError_returncode_type(self): + # returncode must be an integer or None; other types are rejected + # at construction so that __str__ cannot fail later on. + subprocess.CalledProcessError(1, "fake cmd") + subprocess.CalledProcessError(None, "fake cmd") + for returncode in ("1", 1.5, [1], object()): + with self.subTest(returncode=returncode): + with self.assertRaises(TypeError): + subprocess.CalledProcessError(returncode, "fake cmd") + def test_preexec(self): # DISCLAIMER: Setting environment variables is *not* a good use # of a preexec_fn. This is merely a test. diff --git a/Misc/NEWS.d/next/Library/2026-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst b/Misc/NEWS.d/next/Library/2026-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst new file mode 100644 index 00000000000000..044982326adc53 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst @@ -0,0 +1,4 @@ +Calling :func:`str` on a :exc:`subprocess.CalledProcessError` whose +:attr:`!returncode` is ``None`` no longer raises :exc:`TypeError`. The +constructor now also raises :exc:`TypeError` when *returncode* is neither +an integer nor ``None``.