From e4ebd4f68ce19f29d319750793b9475e068922eb Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sat, 18 Jul 2026 16:46:38 +0300 Subject: [PATCH 1/6] 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 6fe2ec98fb40888..82d9da444fc8095 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 d066ae85dfc51a6..14c67f15960f6fd 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 000000000000000..5431a7d75feb1ce --- /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/6] 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 82d9da444fc8095..df23fb18a518b72 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/6] 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 14c67f15960f6fd..db5d254d5310afe 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 5431a7d75feb1ce..d57e0f6afdbb2e5 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/6] 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 fe64daa3291d670..b929eb3e77c7ab0 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 df23fb18a518b72..6fe2ec98fb40888 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 db5d254d5310afe..d066ae85dfc51a6 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 d57e0f6afdbb2e5..000000000000000 --- 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/6] 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 b929eb3e77c7ab0..5470d05bb11f91c 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 6fe2ec98fb40888..7853413aff6f554 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 eeea6c4842b4881..3c5763b0b8374dd 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 d066ae85dfc51a6..d3368a745fc374e 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 000000000000000..044982326adc530 --- /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``. From fdad6bd42e9b25ad14670a54f4c5a8b7f1dd8234 Mon Sep 17 00:00:00 2001 From: Vyron Vasileiadis Date: Sun, 26 Jul 2026 18:35:29 +0300 Subject: [PATCH 6/6] gh-153970: Make str() of CalledProcessError work for any returncode Per the review, the constructor no longer checks the returncode type. Instead __str__ guards the negative comparison with isinstance(), so a returncode that is not an integer is formatted by the f-string branch instead of raising TypeError. This keeps the full process state on the exception, which matters when a subprocess bug or a mocked Popen produces a returncode that is not an int. This drops the constructor check, the versionchanged note and the test_script_helper change that went with it. The type test is replaced by one that checks str() for non-integer returncodes. --- Doc/library/subprocess.rst | 4 ---- Lib/subprocess.py | 8 +------- Lib/test/test_script_helper.py | 2 +- Lib/test/test_subprocess.py | 20 ++++++++----------- ...-07-22-12-00-00.gh-issue-153970.Kq7Wn3.rst | 7 +++---- 5 files changed, 13 insertions(+), 28 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 5470d05bb11f91c..2a31213560c92d3 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -260,10 +260,6 @@ 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 7853413aff6f554..f0050fe05723cf3 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -137,19 +137,13 @@ 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: + if isinstance(self.returncode, int) and self.returncode < 0: try: return "Command %r died with %r." % ( self.cmd, signal.Signals(-self.returncode)) diff --git a/Lib/test/test_script_helper.py b/Lib/test/test_script_helper.py index 3c5763b0b8374dd..eeea6c4842b4881 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(1, 'cmd') + mock_check_call.side_effect = subprocess.CalledProcessError('', '') 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 d3368a745fc374e..e5e5b4870d43170 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -2449,19 +2449,15 @@ 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()): + # returncode which is not an integer, which happens for example when + # Popen is mocked: str() must not fail + for returncode in (None, "2", 2.5, [2]): with self.subTest(returncode=returncode): - with self.assertRaises(TypeError): - subprocess.CalledProcessError(returncode, "fake cmd") + err = subprocess.CalledProcessError(returncode, "fake cmd") + self.assertEqual( + str(err), + f"Command 'fake cmd' returned non-zero " + f"exit status {returncode}.") def test_preexec(self): # DISCLAIMER: Setting environment variables is *not* a good use 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 index 044982326adc530..def943b46b59420 100644 --- 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 @@ -1,4 +1,3 @@ -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``. +Calling :func:`str` on a :exc:`subprocess.CalledProcessError` no longer +raises :exc:`TypeError` when its :attr:`!returncode` is not an integer, such +as ``None``.