Subprocess timeout causes output to be returned as bytes in text mode

[macdjord]

BPO	43431
Nosy	@giampaolo, @eryksun
Files	test_subprocess.py: Demonstration of issue

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-03-08.05:41:46.323>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = 'Subprocess timeout causes output to be returned as bytes in text mode'
updated_at = <Date 2021-03-30.18:57:45.244>
user = 'https://bugs.python.org/macdjord'

bugs.python.org fields:

activity = <Date 2021-03-30.18:57:45.244>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-03-08.05:41:46.323>
creator = 'macdjord'
dependencies = []
files = ['49856']
hgrepos = []
issue_num = 43431
keywords = []
message_count = 3.0
messages = ['388257', '388265', '388272']
nosy_count = 3.0
nosy_names = ['giampaolo.rodola', 'eryksun', 'macdjord']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43431'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[macdjord]

Passing the argument text=True to subprocess.run() is supposed to mean that any captured output of the called process is automatically decoded and retuned to the user as test instead of bytes.

However, if you give a timeout and that timeout expires, the raised subprocess.TimeoutExpired exception will have the captured output as as bytes even if text mode is enabled.

Test output:
bash-5.0$ python3 test_subprocess.py
Version and interpreter information: namespace(_multiarch='x86_64-linux-gnu', cache_tag='cpython-37', hexversion=50792432, name='cpython', version=sys.version_info(major=3, minor=7, micro=7, releaselevel='final', serial=0))
Completed STDOUT Type: <class 'str'>
Completed STDOUT Content: 'Start\nDone\n'
Timeout STDOUT Type: <class 'bytes'>
Timeout STDOUT Content: b'Start\n'

[eryksun]

communicate() is incomplete, so decoding the output may fail. For example, say the encoding is UTF-8, and the last multibyte character sequence (2-4 bytes) is incomplete. Maybe communicate() should always set stdout_bytes and stderr_bytes attributes on the timeout exception, and, in text mode, try to decode the output as stdout and/or stderr. If decoding fails, set the decoded value to None.

In Windows, run() tries to complete communication, which is dysfunctional in cases. I created bpo-43346 to propose changing the design in Windows, in order to address 3 cases that can cause subprocess.run() to ignore the given timeout. The proposed change also sets an incomplete read of stdout and stderr as bytes objects, regardless of text mode, because I was simply matching what POSIX does in this case.

[macdjord]

Eryk Sun: Well, I think step 1 should be to update the documentation for Python 3.7 through 3.10 on subprocess.run() and subprocess.TimeoutExpired to clearly state that TimeoutExpired.stdout and TimeoutExpired.stderr will be in bytes format even if text mode is set.

If we went with the model of having stdout_bytes and attempting to decode into stdout, we'd want an option to ignore a trailing decoding error.

[macdjord]

Note: If you use check=True and the command fails, .stdout on the resulting CalledProcessError is a str, not bytes, making things even more confusing and inconsistent.

[LewisGaul]

There's another related bug here: when there's no output before the timeout is hit then you get the stdout and stderr fields containing None rather than empty str/bytes. I've adressed both issues in #95579.

[gpshead]

Unfortunately this counts as an API change as people will have written code since timeouts were introduced in 3.3 that blindly assumes TimeoutExpired.stdout/stderr are bytes. So while it might be a bug, it's also a breaking change and thus feature. Meaning it can't be backported. And likely needing a deprecation period.

[erlend-aasland]

The current behaviour was documented in ...:

• gh-87597: Document TimeoutExpired.stdout & .stderr types. #97685

Unless there is a need for a breaking change, I suggest closing this as resolved.

[LewisGaul]

I don't think this should be closed, from my perspective this is just a bug, and should not have been documented as now changing it would be a breaking change (which would still be warranted in my opinion).

I raised a PR addressing this (#95579 - not sure why it was moved to draft state), and a discussion following the pushback (https://discuss.python.org/t/pr-review-request-decode-subprocess-output-in-text-mode-when-timeout-is-hit/19594/2) which didn't get any input.

This feels very much unfinished to me, and unfortunately remains an ugly wart for users who want to properly handle errors when using subprocess (especially in combination with mypy for type checking).

[gpshead]

I understand your frustration, but existing code already depends on the behavior given it has been this way for 11+ years now so it isn't "just a bug" in that sense - thus documenting the existing behavior to save everyone the trouble of discovering it the hard way.

I moved that PR to Draft as most PRs should really just start in Draft mode (a realtively new-to-github feature, we should use it more often) as that helps indicate that there are unresolved larger issues and it is unclear if the change as is, is even what we want. See my #95579 (comment) comment.

It sounds like there is a potential way forward based on the last comment from @zooba on the PR - keeping both the bytes and decoding to text on demand via a new TimeoutExpired attribute for the purpose.

I'm fine reopening this since you seem interested in working on it.