Close #15425: Eliminate more importlib related traceback noise

Lib/importlib/_bootstrap.py

@@ -1472,7 +1472,7 @@ def _find_and_load_unlocked(name, import_):
     parent = name.rpartition('.')[0]
     if parent:
         if parent not in sys.modules:
-            import_(parent)
+            _recursive_import(import_, parent)
         # Crazy side-effects!
         if name in sys.modules:
             return sys.modules[name]
@@ -1550,6 +1550,12 @@ def _gcd_import(name, package=None, level=0):
     _lock_unlock_module(name)
     return module
 
+def _recursive_import(import_, name):
+    """Common exit point for recursive calls to the import machinery
+
+    This simplifies the process of stripping importlib from tracebacks
+    """
+    return import_(name)
 
 def _handle_fromlist(module, fromlist, import_):
     """Figure out what __import__ should return.
@@ -1569,7 +1575,8 @@ def _handle_fromlist(module, fromlist, import_):
                 fromlist.extend(module.__all__)
         for x in fromlist:
             if not hasattr(module, x):
-                import_('{}.{}'.format(module.__name__, x))
+                _recursive_import(import_,
+                                  '{}.{}'.format(module.__name__, x))
     return module
 
 

Lib/test/test_import.py

@@ -844,6 +844,74 @@ class ImportTracebackTests(unittest.TestCase):
             self.fail("ZeroDivisionError should have been raised")
         self.assert_traceback(tb, [__file__, 'foo.py', 'bar.py'])
 
+    # A few more examples from issue #15425
+    def test_syntax_error(self):
+        self.create_module("foo", "invalid syntax is invalid")
+        try:
+            import foo
+        except SyntaxError as e:
+            tb = e.__traceback__
+        else:
+            self.fail("SyntaxError should have been raised")
+        self.assert_traceback(tb, [__file__])
+
+    def _setup_broken_package(self, parent, child):
+        pkg_name = "_parent_foo"
+        def cleanup():
+            rmtree(pkg_name)
+            unload(pkg_name)
+        os.mkdir(pkg_name)
+        self.addCleanup(cleanup)
+        # Touch the __init__.py
+        init_path = os.path.join(pkg_name, '__init__.py')
+        with open(init_path, 'w') as f:
+            f.write(parent)
+        bar_path = os.path.join(pkg_name, 'bar.py')
+        with open(bar_path, 'w') as f:
+            f.write(child)
+        importlib.invalidate_caches()
+        return init_path, bar_path
+
+    def test_broken_submodule(self):
+        init_path, bar_path = self._setup_broken_package("", "1/0")
+        try:
+            import _parent_foo.bar
+        except ZeroDivisionError as e:
+            tb = e.__traceback__
+        else:
+            self.fail("ZeroDivisionError should have been raised")
+        self.assert_traceback(tb, [__file__, bar_path])
+
+    def test_broken_from(self):
+        init_path, bar_path = self._setup_broken_package("", "1/0")
+        try:
+            from _parent_foo import bar
+        except ZeroDivisionError as e:
+            tb = e.__traceback__
+        else:
+            self.fail("ImportError should have been raised")
+        self.assert_traceback(tb, [__file__, bar_path])
+
+    def test_broken_parent(self):
+        init_path, bar_path = self._setup_broken_package("1/0", "")
+        try:
+            import _parent_foo.bar
+        except ZeroDivisionError as e:
+            tb = e.__traceback__
+        else:
+            self.fail("ZeroDivisionError should have been raised")
+        self.assert_traceback(tb, [__file__, init_path])
+
+    def test_broken_parent_from(self):
+        init_path, bar_path = self._setup_broken_package("1/0", "")
+        try:
+            from _parent_foo import bar
+        except ZeroDivisionError as e:
+            tb = e.__traceback__
+        else:
+            self.fail("ZeroDivisionError should have been raised")
+        self.assert_traceback(tb, [__file__, init_path])
+
     @cpython_only
     def test_import_bug(self):
         # We simulate a bug in importlib and check that it's not stripped

Misc/NEWS

@@ -10,6 +10,9 @@ What's New in Python 3.3.0 Beta 2?
 Core and Builtins
 -----------------
 
+- Issue #15425: Eliminated traceback noise from more situations involving
+  importlib
+
 - Issue #14578: Support modules registered in the Windows registry again.
 
 - Issue #15466: Stop using TYPE_INT64 in marshal, to make importlib.h

Python/import.c

@@ -1154,14 +1154,27 @@ remove_importlib_frames(void)
 {
     const char *importlib_filename = "<frozen importlib._bootstrap>";
     const char *exec_funcname = "_exec_module";
+    const char *get_code_funcname = "get_code";
+    const char *recursive_import = "_recursive_import";
     int always_trim = 0;
+    int trim_get_code = 0;
     int in_importlib = 0;
     PyObject *exception, *value, *base_tb, *tb;
     PyObject **prev_link, **outer_link = NULL;
 
     /* Synopsis: if it's an ImportError, we trim all importlib chunks
-       from the traceback.  Otherwise, we trim only those chunks which
-       end with a call to "_exec_module". */
+       from the traceback.  If it's a SyntaxError, we trim any chunks that
+       end with a call to "get_code", We always trim chunks
+       which end with a call to "_exec_module". */
+
+    /* Thanks to issue 15425, we also strip any chunk ending with
+     * _recursive_import. This is used when making a recursive call to the
+     * full import machinery which means the inner stack gets stripped early
+     * and the normal heuristics won't fire properly for outer frames. A
+     * more elegant mechanism would be nice, as this one can misfire if
+     * builtins.__import__ has been replaced with a custom implementation.
+     * However, the current approach at least gets the job done.
+     */
 
     PyErr_Fetch(&exception, &value, &base_tb);
     if (!exception || Py_VerboseFlag)
@@ -1169,6 +1182,9 @@ remove_importlib_frames(void)
     if (PyType_IsSubtype((PyTypeObject *) exception,
                          (PyTypeObject *) PyExc_ImportError))
         always_trim = 1;
+    if (PyType_IsSubtype((PyTypeObject *) exception,
+                         (PyTypeObject *) PyExc_SyntaxError))
+        trim_get_code = 1;
 
     prev_link = &base_tb;
     tb = base_tb;
@@ -1191,8 +1207,14 @@ remove_importlib_frames(void)
 
         if (in_importlib &&
             (always_trim ||
-             PyUnicode_CompareWithASCIIString(code->co_name,
-                                              exec_funcname) == 0)) {
+             (PyUnicode_CompareWithASCIIString(code->co_name,
+                                              exec_funcname) == 0) ||
+             (PyUnicode_CompareWithASCIIString(code->co_name,
+                                              recursive_import) == 0) ||
+             (trim_get_code &&
+              PyUnicode_CompareWithASCIIString(code->co_name,
+                                              get_code_funcname) == 0)
+           )) {
             PyObject *tmp = *outer_link;
             *outer_link = next;
             Py_XINCREF(next);