classification
Title: Traceback wrong on ImportError while executing a package
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, eric.araujo, eric.snow, martin.panter, ncoghlan, python-dev, robagar, schlamar
Priority: normal Keywords: patch

Created on 2012-03-13 06:50 by schlamar, last changed 2015-12-11 07:41 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
runpy-traceback.patch martin.panter, 2015-03-11 08:30 Tests but no actual fix review
finding-spec.patch martin.panter, 2015-03-11 08:41 Separate test for ImportError wrapping review
internal-error.patch martin.panter, 2015-12-02 03:47 Fix by using internal _Error review
init-ancestor.patch martin.panter, 2015-12-04 02:16 review
init-ancestor.2.patch martin.panter, 2015-12-06 02:30 Check ImportError.name review
init-ancestor.3.patch martin.panter, 2015-12-06 23:34 review
Messages (27)
msg155574 - (view) Author: Marc Schlaich (schlamar) * Date: 2012-03-13 06:50
It is very simple to reproduce this error. 
There is an executable package:

package/
    __init__.py
    __main__.py

The __init__ imports a missing module:

    import missing_module

And the __main__ imports from it:

    from . import missing_module

Now I get the following output which is not what I am expecting:

    C:\Python27\python.exe: No module named missing_module; 'package' is 
    a package and cannot be directly executed
msg223289 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-16 22:16
I've no idea what having a file called __main__.py is likely to do so can someone comment please.
msg223317 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-07-17 07:25
A file called “package/__main__.py” is executed as a script by “python -m package”. See <https://docs.python.org/dev/library/__main__.html>.

I’ve came across this issue myself. You don’t even need the __main__.py file to be doing anything special, as long as the __init__.py raises an ImportError, I think. On Python 3.4 the report is even more convoluted:

/sbin/python3: Error while finding spec for 'package.__main__' (<class 'ImportError'>: No module named 'missing_module'); 'package' is a package and cannot be directly executed

I dunno what “finding spec” means, and packages _can_ be directly executed if they have a __main__ module, so at least the last bit is definitely wrong.
msg226945 - (view) Author: Rob Agar (robagar) Date: 2014-09-16 08:43
The message also needs to include the file and line number of the ImportError.
msg236354 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-21 09:07
The relevant code is in the _get_module_details() function at Lib/runpy.py:101. There are a few of things going on:

1. The code is calling importlib.util.find_spec("<package>.__main__"), and handles various kinds of exceptions by wrapping them in an ImportError, adding the “Error while finding spec” message. The call apparently causes the package to be imported, so some exceptions triggered by running __init__.py are wrapped. It would be nice if exceptions raised by running __init__.py were not caught, but I have no idea if this is practical or how to do it. It seems the exception handler is there to raise ImportError if you do something like “python -m os.path.something”, which seems rather unlikely to me.

2. The logic for handling the __main__ module in a package seems to assume that any ImportError (such as raised from step 1) is due to the __main__ module not being present. Then it wraps that exception in another ImportError, adding the “. . . cannot be directly executed” bit. Again, it would be good to separate the __init__.py running from this exception handling.

3. Finally in _run_module_as_main(), the ImportError is caught and printed, without any traceback. It would be good to only do this for internal errors loading the module, and not if ImportError is raised when the module (or __init__.py) is run.

I’m not sure what the best way to fix this is. Perhaps add an internal ImportError subclass (or subclasses) that is only used for the “No module named . . .” errors, or only used for errors appropriate for final handler in step 3.
msg237830 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-03-11 00:40
Closely related: Issue 19771, which seems to be complaining about a the error message when __main__.py generates an ImportError.
msg237849 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-03-11 08:30
The patches at Issue 19771 should remove the part of the message that incorrectly says “. . . is a package and cannot be directly executed”. However that still leaves the problem of the suppressed traceback.

I am posting runpy-traceback.patch here which adds some tests to check if the traceback is suppressed. The offending test is marked @expectedFailure. It also points out where the exception is being incorrectly caught, but I haven’t thought of a nice way to fix the problem.

Other changes in the runpy-traceback.patch:

* Removed the exception message rewriting in _run_module_as_main(), because it seems to be redundant with the _get_main_module_details() function
* Fixed test_cmd_line_script.CmdLineTest.test_dash_m_error_code_is_one(), which was only checking the Python exit status, and not verifying that the specific failure was the one anticipated
msg237851 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-03-11 08:41
Posting finding-spec.patch which just has another test I wrote. It tests if AttributeError/ValueError/TypeError gets wrapped in the “finding spec” ImportError, though I’m not sure if this is a bug or a feature, hence I kept it separate. And again I’m not sure of a good way to fix it either.
msg255639 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-01 03:55
I suspect a proper solution of this bug would also help with Issue 16217 (provide only the relevant traceback for exceptions from user code).
msg255690 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-02 03:44
Now I have a deeper understanding I think this can be handled separately to Issue 16217.

This patch pulls together my previous two patches, and adds a fix. There are two aspects of my fix:

1. Import the package before calling find_spec() on the __main__ submodule. This means exceptions raised by the initialization code can be differentiated from exceptions from find_spec().

2. Change all the special ImportError exceptions raised inside runpy [and also one raised by InspectLoader.get_code()] to an internal _Error exception known only to runpy. Now I can be sure that all _Error exceptions are not caused by the initialization code, and I can stop catching ImportError, but still catch _Error and suppress the traceback. When runpy is invoked from the documented run_module() or run_path() APIs, _Error is not used, and it still raises ImportError to maintain backwards compatibility.

I think my patch should avoid the main problem in Issue 19771 as well.

Please review my patch. There are so many error possibilities; it is hard to be sure I have got them all right.
msg255692 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-02 05:24
Martin, your patch looks good to me, and is at the very least an improvement over the status quo (which clearly traps exceptions that it shouldn't), so I'd say go ahead and apply it.

Thanks for digging into this and figuring out a clean solution.
msg255707 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-02 11:09
Thanks for the review Nick. You removed Python 3.4 from the versions; do you think it is not worth the risk committing in 3.4? I understand the deadline for the final release of 3.4 is the end of this week.
msg255716 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-02 14:10
Right, while I agree this is a bug fix that makes sense to apply to 2.7 and 3.5, it's also a large enough change to runpy's control flow logic that I'm wary of including it in the final 3.4 binary release.
msg255799 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-03 01:54
New changeset 784a64a21fd0 by Martin Panter in branch '3.5':
Issue #14285: Do not catch __init__.py exceptions in runpy
https://hg.python.org/cpython/rev/784a64a21fd0

New changeset 01397c11ebb8 by Martin Panter in branch 'default':
Issue #14285: Merge runpy exception fix from 3.5
https://hg.python.org/cpython/rev/01397c11ebb8
msg255848 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-04 01:32
New changeset c4e950338e79 by Martin Panter in branch '2.7':
Issue #14285: Do not catch ImportError from __init__.py in runpy
https://hg.python.org/cpython/rev/c4e950338e79
msg255849 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-04 02:16
Even with my committed fix, tracebacks triggered by initializing a parent package are still suppressed:

$ ./python -m hello
Traceback (most recent call last):
  File "/media/disk/home/proj/python/cpython/Lib/runpy.py", line 158, in _run_module_as_main
    mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
  File "/media/disk/home/proj/python/cpython/Lib/runpy.py", line 116, in _get_module_details
    __import__(mod_name)  # Do not catch exceptions initializing package
  File "/media/disk/home/proj/python/cpython/hello/__init__.py", line 1, in <module>
    import random; random.dsgjdgj
AttributeError: module 'random' has no attribute 'dsgjdgj'
[Exit 1]
$ ./python -m hello.submodule
/media/disk/home/proj/python/cpython/python: Error while finding spec for 'hello.submodule' (<class 'AttributeError'>: module 'random' has no attribute 'dsgjdgj')
[Exit 1]

Fixing this in general might require a loop around the find_spec() call as in init-ancestor.patch. But I’m unsure if it is worth the extra complication.
msg255852 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-04 06:38
I'm wondering if there might be a simpler option: use rpartition() to strip off any trailing segment (whether that's "__main__" or not), and then always do a plain dynamic import of that package (if any). Something like the following at the start of _get_module_details():

    pkg_name, is_submodule, submodule = mod_name.rpartition(".")
    if is_submodule:
        __import__(pkg_name)

The key is that we *don't* want to be relying on the fact find_spec() will import parent packages implicitly.
msg255907 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-05 00:13
I think the problem with doing a blind import of the parent package is it would be hard to differentiate between the ImportError when the package (or an ancestor) is missing, versus a user-generated ImportError. Maybe you could inspect the exception as a workaround (untested):

try:
    __import__(pkg_name)
except ImportError as err:
    if err.name != pkg_name and not pkg_name.startswith(err.name + "."):
        raise
    raise error(format(err))
msg255957 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-05 14:41
That import will only import the parent package (if there is one), not the module itself. Imports from __main__ will still happen during the exec call later on.
msg255998 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-06 01:40
Yes, the package is all we need to import. I understand this bug is all about importing the parent package, and not __main__. If you read the original report, it is __init__.py that is trying to import a missing module.
msg256000 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-06 02:30
init-ancestor.2.patch implements the single __import__ with ImportError.name checking
msg256010 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-06 11:23
Ah, I think I understand the problem you're getting at now:

* if the parent package is missing entirely, we still want to suppress the traceback by throwing the runpy specific exception
* if the parent package is present, but one of the modules *that* tries to import is missing (or otherwise doesn't load), we want to let the traceback happen normally

The exception handler in the new code snippet addresses that by checking the name attribute on the ImportError, but could likely use a comment explaining the subtlety.

The way you've handled checking the result of the rpartition call also highlights a deficiency in the runpy docs, and likely also the test suite as well: runpy.run_module and the -m switch only work with absolute module names, but this isn't stated explicitly in the documentation, nor is there any early check in _get_module_details() to bail out if given a relative module name.
msg256036 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-06 23:34
The test suite does check run_module() with some relative names; see test_invalid_names() in test_runpy. But there does not appear to be a test for “python -m .relative”.

Changes in init-ancestor.3.patch:

* Added a comment explaining the exception handling
* Added check, tests and documentation for relative module names
msg256041 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-07 01:33
+1, latest iteration looks good to me.
msg256187 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-11 03:56
New changeset 3202d143a194 by Martin Panter in branch '3.5':
Issue #14285: Do not catch exceptions initializing any ancestor package
https://hg.python.org/cpython/rev/3202d143a194

New changeset a526ebcfd31d by Martin Panter in branch 'default':
Issue #14285: Merge runpy fix from 3.5
https://hg.python.org/cpython/rev/a526ebcfd31d
msg256190 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-12-11 04:22
I committed a slightly modified version to 3.5+. I stopped wrapping the new ImportError, and let the existing find_spec() handler wrap it instead. That way the existing error messages stay the same.

However I cannot figure out an easy way to do a similar fix for 2.7, where ImportError.name does not exist. Therefore I propose to leave 2.7 as it is, with just my first commit. This means that 2.7 still omits the traceback if a parent package fails to initialize:

$ ./python -m package.submodule
[. . .]/python: No module named missing_module

It also means we see an unnecessary traceback with broken *.pyc files, although the eventual error message is improved. This case was reported in <https://bugs.python.org/issue19771#msg248193>:

$ mkdir bad_pyc
$ : > bad_pyc/__init__.pyc  # Create empty file
$ python2 -m bad_pyc  # Original behaviour
/sbin/python2: Bad magic number in bad_pyc/__init__.pyc; 'bad_pyc' is a package and cannot be directly executed
$ ./python -m bad_pyc  # Current behaviour
Traceback (most recent call last):
  File "[. . .]/Lib/runpy.py", line 163, in _run_module_as_main
    mod_name, _Error)
  File "[. . .]/Lib/runpy.py", line 111, in _get_module_details
    __import__(mod_name)  # Do not catch exceptions initializing package
ImportError: Bad magic number in bad_pyc/__init__.pyc

I propose to leave these two cases as they currently are in 2.7, unless someone has any ideas how to improve them.
msg256205 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-12-11 07:41
Leaving Python 2.7 alone makes sense to me - runpy is heavily dependent on the import system, so there are going to be limits to the improvements that can be made there.
History
Date User Action Args
2015-12-11 07:41:04ncoghlansetstatus: open -> closed
resolution: fixed
messages: + msg256205

stage: patch review -> resolved
2015-12-11 04:22:31martin.pantersetmessages: + msg256190
2015-12-11 03:56:02python-devsetmessages: + msg256187
2015-12-07 01:33:22ncoghlansetmessages: + msg256041
2015-12-06 23:34:06martin.pantersetfiles: + init-ancestor.3.patch

messages: + msg256036
2015-12-06 11:23:06ncoghlansetmessages: + msg256010
2015-12-06 02:30:35martin.pantersetfiles: + init-ancestor.2.patch

messages: + msg256000
2015-12-06 01:40:40martin.pantersetmessages: + msg255998
2015-12-05 14:41:08ncoghlansetmessages: + msg255957
2015-12-05 00:13:13martin.pantersetmessages: + msg255907
2015-12-04 06:38:33ncoghlansetmessages: + msg255852
2015-12-04 02:16:14martin.pantersetfiles: + init-ancestor.patch

messages: + msg255849
2015-12-04 01:32:29python-devsetmessages: + msg255848
2015-12-03 01:54:03python-devsetnosy: + python-dev
messages: + msg255799
2015-12-02 14:10:43ncoghlansetmessages: + msg255716
2015-12-02 11:09:05martin.pantersetmessages: + msg255707
2015-12-02 05:24:55ncoghlansetmessages: + msg255692
versions: - Python 3.4
2015-12-02 03:53:32martin.panterlinkissue19771 dependencies
2015-12-02 03:47:57martin.pantersetfiles: + internal-error.patch
2015-12-02 03:44:44martin.pantersetstage: patch review
messages: + msg255690
components: + Library (Lib)
versions: + Python 3.5, Python 3.6
2015-12-01 03:55:21martin.pantersetmessages: + msg255639
2015-12-01 03:52:31martin.panterlinkissue25708 superseder
2015-03-11 08:41:21martin.pantersetfiles: + finding-spec.patch

messages: + msg237851
2015-03-11 08:30:58martin.pantersetfiles: + runpy-traceback.patch
keywords: + patch
messages: + msg237849
2015-03-11 00:40:01martin.pantersetmessages: + msg237830
2015-02-21 09:07:41martin.pantersetmessages: + msg236354
2014-09-16 08:43:37robagarsetmessages: + msg226945
2014-09-16 08:40:08robagarsetnosy: + robagar
2014-07-17 16:24:01berker.peksagsetnosy: + eric.snow
2014-07-17 07:25:34martin.pantersetnosy: + martin.panter

messages: + msg223317
versions: + Python 3.4
2014-07-16 22:16:01BreamoreBoysetnosy: + BreamoreBoy
messages: + msg223289
2013-02-01 22:11:09brett.cannonsetnosy: - brett.cannon
2012-03-14 02:08:07eric.araujosetnosy: + brett.cannon, ncoghlan, eric.araujo
2012-03-13 06:50:35schlamarcreate