classification
Title: invalid docs for subprocess exceptions with shell=True
Type: Stage: resolved
Components: Documentation, Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: cvrebert, docs@python, eric.araujo, iritkatriel, jack__d, miss-islington, ncoghlan, r.david.murray, techtonik
Priority: normal Keywords: easy, patch

Created on 2012-05-22 06:09 by techtonik, last changed 2021-07-22 18:26 by iritkatriel. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 26755 merged jack__d, 2021-06-16 14:03
PR 27288 merged miss-islington, 2021-07-22 18:23
PR 27289 merged miss-islington, 2021-07-22 18:23
Messages (9)
msg161329 - (view) Author: anatoly techtonik (techtonik) Date: 2012-05-22 06:09
http://docs.python.org/library/subprocess.html#exceptions documentation is wrong at least for the case when shell=True on Linux. An attempt to execute a non-existent file with:

  process = subprocess.Popen("sdfsdf", shell=True, stdout=subprocess.PIPE, stderr=errpipe)
  out, err = process.communicate()
  print out

Results in a message "/bin/sh: sfs: command not found" with no exceptions. Other statements may be invalid too.
msg161384 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-22 22:07
Eh, just needs clarification along the lines of:

    "Exceptions raised in the child ++Python++ process"

    "A ValueError will be raised if Popen is called with invalid arguments
    ++whose validity is not dependent upon the state of the environment++."
msg161385 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-22 22:24
I don't think that those additions make anything clearer, I'm afraid.

"child python process" would seem to imply raised in the new python running in the child process, which is clearly not true.

Hmm.  I seem to remember having this discussion before.  The current statement is in fact exactly correct.  When shell=True the new program being run is 'sh' (or similar).  So the error message is coming from the program being run successfully by subprocess.  End of story.

So, we could add a clarfication: "If shell=True, the 'new program' being run is the shell, which may generate its own errors based on the shell command passed to it (such as command not found).  These will not result in exceptions."
msg161394 - (view) Author: Chris Rebert (cvrebert) * Date: 2012-05-23 05:04
The term "invalid arguments" in "A ValueError will be raised if Popen is called with invalid arguments." is still vague. One could well argue that a nonexistent executable or bad command is "invalid". Anything resulting in an OSError can be considered "invalid" in a sense. The ValueError sentence should use a different descriptor or else include a qualifier.

+1 on calling out the shell=True case.
(My but how the `shell` argument complicates everything...)
msg161404 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-05-23 12:34
Heh.  Maybe what we ought to do is drop the shell argument and make everyone build their own shell invocations :)

Actually, an API refactor where a shell call looks like this might be kind of cool:

  Popen(shell_cmd('echo magic'))

where shell would return a list.

As for the ValueError, yeah, it's vague, but I think that the only information that sentence is trying to convey is that ValueError is one of the exceptions you might get when calling Popen.  Exactly what you get it for is pretty much an implementation detail.  If someone wants to grovel through the code and figure out all the things that can raise ValueError, we could see if there's any sensible way to turn that into a sentence, but it might not be easy.  Short of that, perhaps we could  drop the existing qualifier and just say "Popen may also raise ValueError."
msg164814 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-07-07 10:42
I would add a simple note to the exceptions section:

"Note, when "shell=True", OSError will be raised by the child only if the selected shell itself cannot be found. To determine if the shell failed to find the requested application, it is necessary to check the return code or output from the subprcess."
msg397991 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-07-22 18:23
New changeset 50ffbe3dafcae7826d114df61d56c7ac45a6358c by Jack DeVries in branch 'main':
bpo-14879: [doc] clarify how to check for errors from subprocess.Popen(..., shell=True) (GH-26755)
https://github.com/python/cpython/commit/50ffbe3dafcae7826d114df61d56c7ac45a6358c
msg397992 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-07-22 18:25
New changeset 5ffbb05f1c3c952588ba5c1927da0e8223f5aaa7 by Miss Islington (bot) in branch '3.9':
bpo-14879: [doc] clarify how to check for errors from subprocess.Popen(..., shell=True) (GH-26755) (GH-27289)
https://github.com/python/cpython/commit/5ffbb05f1c3c952588ba5c1927da0e8223f5aaa7
msg397993 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-07-22 18:26
New changeset d0c61753036dcae292d49aa8ddac555062da6b76 by Miss Islington (bot) in branch '3.10':
bpo-14879: [doc] clarify how to check for errors from subprocess.Popen(..., shell=True) (GH-26755) (GH-27288)
https://github.com/python/cpython/commit/d0c61753036dcae292d49aa8ddac555062da6b76
History
Date User Action Args
2021-07-22 18:26:37iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-07-22 18:26:01iritkatrielsetmessages: + msg397993
2021-07-22 18:25:34iritkatrielsetmessages: + msg397992
2021-07-22 18:23:45miss-islingtonsetpull_requests: + pull_request25831
2021-07-22 18:23:39miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request25830
2021-07-22 18:23:37iritkatrielsetnosy: + iritkatriel
messages: + msg397991
2021-06-16 14:03:07jack__dsetkeywords: + patch
nosy: + jack__d

pull_requests: + pull_request25340
stage: patch review
2021-06-16 13:19:28iritkatrielsetkeywords: + easy
versions: + Python 3.9, Python 3.10, Python 3.11, - Python 2.7, Python 3.2, Python 3.3
2012-07-07 10:42:18ncoghlansetmessages: + msg164814
2012-05-25 18:48:10eric.araujosetnosy: + ncoghlan, eric.araujo
2012-05-23 12:34:36r.david.murraysetmessages: + msg161404
2012-05-23 05:04:58cvrebertsetmessages: + msg161394
2012-05-22 22:24:33r.david.murraysetnosy: + r.david.murray

messages: + msg161385
versions: - Python 2.6, Python 3.1, Python 3.4
2012-05-22 22:07:14cvrebertsetnosy: + cvrebert
messages: + msg161384
2012-05-22 06:09:53techtonikcreate