classification
Title: subprocess should include filename in FileNotFoundError exception
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: Christian H, akira, berker.peksag, charpov, gregory.p.smith, ronaldoussoren, travis.thieman, tshepang, vstinner
Priority: normal Keywords: patch

Created on 2014-10-02 00:49 by charpov, last changed 2017-09-06 20:40 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
bug.py charpov, 2014-10-02 00:49
22536-subprocess-exception-filename.patch travis.thieman, 2014-11-15 21:42 review
22536-subprocess-exception-filename-2.patch travis.thieman, 2014-11-30 03:03 review
Pull Requests
URL Status Linked Edit
PR 3194 merged gregory.p.smith, 2017-08-23 20:55
PR 3202 merged gregory.p.smith, 2017-08-25 00:12
PR 3305 merged gregory.p.smith, 2017-09-04 20:55
Messages (13)
msg228147 - (view) Author: Michel Charpentier (charpov) Date: 2014-10-02 00:49
FileNotFoundError should contain a 'filename' information, as per its specification.  It's 'None' after a failure to execute a subprocess.
msg231221 - (view) Author: Travis Thieman (travis.thieman) * Date: 2014-11-15 21:42
The attached patch includes the first element in args in _execute_child to the OSError exception subclass. This correctly populates the 'filename' field on the resulting exception. A test is also included that fails without the patch.
msg231293 - (view) Author: Akira Li (akira) * Date: 2014-11-17 20:55
I can confirm that without the patch the filename attribute is None
despite being mentioned in strerror.

Travis, you should use `orig_executable` instead of `args[0]` to cover:

  subprocess.call("exit 0", shell=True, executable='/nonexistent bash')

case. 

And use `cwd` if `child_exec_never_called`, to be consistent with the error message (see if/else statements above the raise statement).

It seems appropriate to set filename even if errno is not ENOENT
but to be conservative, you could provide filename iff `err_msg` is also changed i.e., iff errno is ENOENT.
msg231297 - (view) Author: Akira Li (akira) * Date: 2014-11-17 21:28
If the_oserror.filename is not None then str(the_oserror) appends the
filename twice:

  [Errno 2] No such file or directory: 'nonexistent': 'nonexistent'

You could remove `err_msg += ':' ...` statements to avoid the
repeatition. It may break the code that uses strerror attribute. But
exception error messages are explicitly excluded from backward
compatibility considirations therefore it might be ok to break it
here. I can't find the reference so it should probably be resolved as a
new issue (independent from providing the filename attribute value).
msg231392 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-11-19 16:39
You should only add the filename if the error if a FileNotFound exception, not for any OSError, and only if exec() was called. It looks like the variable child_exec_never_called indicates if exec() was called.
msg231400 - (view) Author: Akira Li (akira) * Date: 2014-11-19 22:38
It would be inconsitent to provide filename only if exec is called e.g.:

   >>> import subprocess
   subprocess.call("not used", cwd="nonexistent")
   FileNotFoundError: [Errno 2] No such file or directory: 'nonexistent'

The error message shows the filename (cwd) despite exec not being 
called therefore it would be logical to provide non-None `filename` 
attribute here too.

If we ignore the backward compatibility issue I've mentioned in msg231297 
then the current code:

                        if errno_num == errno.ENOENT:
                            if child_exec_never_called:
                                # The error must be from chdir(cwd).
                                err_msg += ': ' + repr(cwd)
                            else:
                                err_msg += ': ' + repr(orig_executable)
                    raise child_exception_type(errno_num, err_msg)

could be replaced with:
                        
                        if errno_num == errno.ENOENT:
                            if child_exec_never_called:
                                # The error must be from chdir(cwd).
                                filename = cwd
                            else:
                                filename = orig_executable
                            raise child_exception_type(errno_num, err_msg, filename)
                    raise child_exception_type(errno_num, err_msg)


[1] https://hg.python.org/cpython/file/23ab1197df0b/Lib/subprocess.py#l1443
msg231414 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-11-20 06:38
I wnated to say that args[0] is not the right filename if exec() was not
called. Yes, there is also cwd for example. The logic to choose the
filename should be done in the child.
msg231881 - (view) Author: Travis Thieman (travis.thieman) * Date: 2014-11-30 03:03
Thank you all for the helpful comments. A revised attempt is attached as -2.patch, with improved behavior around using cwd if the child is never called and orig_executable if it is. I opted not to fix the issue with the redundancy in the error message as I agree that should be handled as a separate issue.
msg300751 - (view) Author: Christian H (Christian H) Date: 2017-08-23 13:33
I was also bitten by this bug, and would like to see it merged. The patch 22536-subprocess-exception-filename-2.patch looks fine to me.
msg300808 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-08-24 21:58
New changeset 8621bb5d93239316f97281826461b85072ff6db7 by Gregory P. Smith in branch 'master':
bpo-22536: Set the filename in FileNotFoundError. (#3194)
https://github.com/python/cpython/commit/8621bb5d93239316f97281826461b85072ff6db7
msg300814 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-24 23:37
Your change makes test_subprocess f1iling on Windows. Example:
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/891/steps/test/logs/stdio
msg300815 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-08-25 00:06
right, the test needs to be excluded there.  fixing... (bummer windows wasn't in the CI)
msg301266 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-09-04 21:23
New changeset 1dba3789e335f06e3b01cdc84070f2e828c9b861 by Gregory P. Smith in branch '3.6':
bpo-22536 [3.6] Set filename in FileNotFoundError  (#3305)
https://github.com/python/cpython/commit/1dba3789e335f06e3b01cdc84070f2e828c9b861
History
Date User Action Args
2017-09-06 20:40:35gregory.p.smithsetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.6, Python 3.7, - Python 3.4, Python 3.5
2017-09-04 21:23:25gregory.p.smithsetmessages: + msg301266
2017-09-04 20:55:33gregory.p.smithsetpull_requests: + pull_request3335
2017-08-25 00:12:42gregory.p.smithsetpull_requests: + pull_request3241
2017-08-25 00:06:48gregory.p.smithsetmessages: + msg300815
2017-08-24 23:37:19vstinnersetmessages: + msg300814
2017-08-24 21:58:27gregory.p.smithsetmessages: + msg300808
2017-08-23 21:11:32gregory.p.smithsetassignee: gregory.p.smith
2017-08-23 20:55:49gregory.p.smithsetpull_requests: + pull_request3233
2017-08-23 13:33:42Christian Hsetnosy: + Christian H
messages: + msg300751
2014-11-30 03:03:35travis.thiemansetfiles: + 22536-subprocess-exception-filename-2.patch

messages: + msg231881
2014-11-20 06:38:51vstinnersetmessages: + msg231414
2014-11-19 22:38:01akirasetmessages: + msg231400
2014-11-19 16:39:25vstinnersetnosy: + vstinner
messages: + msg231392
2014-11-17 21:28:04akirasetmessages: + msg231297
2014-11-17 20:55:23akirasetnosy: + akira
messages: + msg231293
2014-11-16 05:50:11berker.peksagsetnosy: + berker.peksag
stage: patch review

versions: + Python 3.4
2014-11-15 21:42:45travis.thiemansetfiles: + 22536-subprocess-exception-filename.patch

nosy: + travis.thieman
messages: + msg231221

keywords: + patch
2014-10-03 16:20:17tshepangsetnosy: + tshepang
2014-10-02 06:13:11ned.deilysetnosy: + gregory.p.smith
title: Missing filename in FileNotFoundError -> subprocess should include filename in FileNotFoundError exception
assignee: ronaldoussoren -> (no value)
versions: + Python 3.5, - Python 3.4
components: + Library (Lib), - Interpreter Core, macOS
2014-10-02 00:49:37charpovcreate