diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py --- a/Lib/importlib/_bootstrap.py +++ b/Lib/importlib/_bootstrap.py @@ -541,7 +541,7 @@ class FrozenImporter: """Load a frozen module.""" is_reload = fullname in sys.modules try: - m = _imp.init_frozen(fullname) + m = cls._exec_module(fullname) # Let our own module_repr() method produce a suitable repr. del m.__file__ return m @@ -568,6 +568,13 @@ class FrozenImporter: """Return if the frozen module is a package.""" return _imp.is_frozen_package(fullname) + @classmethod + def _exec_module(cls, fullname): + """Helper for load_module, allowing to isolate easily (when + looking at a traceback) whether an error comes from executing + an imported module's code.""" + return _imp.init_frozen(fullname) + class _LoaderBasics: @@ -644,9 +651,15 @@ class _LoaderBasics: else: module.__package__ = module.__package__.rpartition('.')[0] module.__loader__ = self - exec(code_object, module.__dict__) + self._exec_module(code_object, module.__dict__) return module + def _exec_module(self, code_object, module_dict): + """Helper for _load_module, allowing to isolate easily (when + looking at a traceback) whether an error comes from executing + an imported module's code.""" + exec(code_object, module_dict) + class SourceLoader(_LoaderBasics): @@ -869,7 +882,7 @@ class ExtensionFileLoader: """Load an extension module.""" is_reload = fullname in sys.modules try: - module = _imp.load_dynamic(fullname, self.path) + module = self._exec_module(fullname, self.path) _verbose_message('extension module loaded from {!r}', self.path) return module except: @@ -889,6 +902,12 @@ class ExtensionFileLoader: """Return None as extension modules have no source code.""" return None + def _exec_module(self, fullname, path): + """Helper for load_module, allowing to isolate easily (when + looking at a traceback) whether an error comes from executing + an imported module's code.""" + return _imp.load_dynamic(fullname, path) + class _NamespacePath: """Represents a namespace package's path. It uses the module name diff --git a/Lib/test/test_import.py b/Lib/test/test_import.py --- a/Lib/test/test_import.py +++ b/Lib/test/test_import.py @@ -768,6 +768,98 @@ class ImportlibBootstrapTests(unittest.T self.assertTrue(mod.__file__.endswith('_bootstrap.py'), mod.__file__) +class ImportTracebackTests(unittest.TestCase): + + def setUp(self): + os.mkdir(TESTFN) + self.old_path = sys.path[:] + sys.path.insert(0, TESTFN) + + def tearDown(self): + sys.path[:] = self.old_path + rmtree(TESTFN) + + def create_module(self, mod, contents): + with open(os.path.join(TESTFN, mod + ".py"), "w") as f: + f.write(contents) + self.addCleanup(unload, mod) + importlib.invalidate_caches() + + def assert_traceback(self, tb, files): + deduped_files = [] + while tb: + code = tb.tb_frame.f_code + fn = code.co_filename + if not deduped_files or fn != deduped_files[-1]: + deduped_files.append(fn) + tb = tb.tb_next + self.assertEqual(len(deduped_files), len(files), deduped_files) + for fn, pat in zip(deduped_files, files): + self.assertRegex(fn, pat) + + def test_nonexistent_module(self): + try: + # assertRaises() clears __traceback__ + import nonexistent_xyzzy + except ImportError as e: + tb = e.__traceback__ + else: + self.fail("ImportError should have been raised") + self.assert_traceback(tb, [__file__]) + + def test_nonexistent_module_nested(self): + self.create_module("foo", "import nonexistent_xyzzy") + try: + import foo + except ImportError as e: + tb = e.__traceback__ + else: + self.fail("ImportError should have been raised") + self.assert_traceback(tb, [__file__, 'foo.py']) + + def test_exec_failure(self): + self.create_module("foo", "1/0") + try: + import foo + except ZeroDivisionError as e: + tb = e.__traceback__ + else: + self.fail("ZeroDivisionError should have been raised") + self.assert_traceback(tb, [__file__, 'foo.py']) + + def test_exec_failure_nested(self): + self.create_module("foo", "import bar") + self.create_module("bar", "1/0") + try: + import foo + except ZeroDivisionError as e: + tb = e.__traceback__ + else: + self.fail("ZeroDivisionError should have been raised") + self.assert_traceback(tb, [__file__, 'foo.py', 'bar.py']) + + @cpython_only + def test_import_bug(self): + # We simulate a bug in importlib and check that it's not stripped + # away from the traceback. + self.create_module("foo", "") + importlib = sys.modules['_frozen_importlib'] + old_load_module = importlib.SourceLoader.load_module + try: + def load_module(*args): + 1/0 + importlib.SourceLoader.load_module = load_module + try: + import foo + except ZeroDivisionError as e: + tb = e.__traceback__ + else: + self.fail("ZeroDivisionError should have been raised") + self.assert_traceback(tb, [__file__, 'tb_next; + PyFrameObject *frame = traceback->tb_frame; + PyCodeObject *code = frame->f_code; + int now_in_importlib; + + assert(PyTraceBack_Check(tb)); + now_in_importlib = (PyUnicode_CompareWithASCIIString( + code->co_filename, + importlib_filename) == 0); + if (now_in_importlib && !in_importlib) { + /* This is the link to this chunk of importlib tracebacks */ + outer_link = prev_link; + } + in_importlib = now_in_importlib; + + if (in_importlib && + (always_trim || + PyUnicode_CompareWithASCIIString(code->co_name, + exec_funcname) == 0)) { + PyObject *tmp = *outer_link; + *outer_link = next; + Py_XINCREF(next); + Py_DECREF(tmp); + prev_link = outer_link; + } + else { + prev_link = (PyObject **) &traceback->tb_next; + } + tb = next; + } +done: + PyErr_Restore(exception, value, base_tb); +} + + PyObject * PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals, PyObject *locals, PyObject *given_fromlist, @@ -1693,6 +1757,8 @@ PyImport_ImportModuleLevelObject(PyObjec Py_XDECREF(package); Py_XDECREF(globals); Py_XDECREF(fromlist); + if (final_mod == NULL) + remove_importlib_frames(); return final_mod; }