From b161c01c48b4c353d7d1a47e9097391a8eb2e047 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 31 Aug 2014 18:51:55 +0200 Subject: [PATCH] Fixed #22920 -- Avoid masking some exceptions. If loading an application trigger an ImportError, the details of that error were lost in some cases. Thanks Ben Davis for the report. --- django/apps/config.py | 19 ++++++++++++------- tests/apps/failing_app/__init__.py | 1 + tests/apps/tests.py | 8 ++++++++ 3 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 tests/apps/failing_app/__init__.py diff --git a/django/apps/config.py b/django/apps/config.py index 9cfe4858aa..843013f30e 100644 --- a/django/apps/config.py +++ b/django/apps/config.py @@ -87,6 +87,10 @@ class AppConfig(object): module = import_module(entry) except ImportError: + # Track that importing as an app module failed. If importing as an + # app config class fails too, we'll trigger the ImportError again. + module = None + mod_path, _, cls_name = entry.rpartition('.') # Raise the original exception when entry cannot be a path to an @@ -104,8 +108,8 @@ class AppConfig(object): else: mod_path, _, cls_name = entry.rpartition('.') - # If we're reaching this point, we must load the app config class - # located at .. + # If we're reaching this point, we must attempt to load the app config + # class located at . # Avoid django.utils.module_loading.import_by_path because it # masks errors -- it reraises ImportError as ImproperlyConfigured. @@ -113,11 +117,12 @@ class AppConfig(object): try: cls = getattr(mod, cls_name) except AttributeError: - # Emulate the error that "from import " - # would raise when exists but not , with - # more context (Python just says "cannot import name ..."). - raise ImportError( - "cannot import name '%s' from '%s'" % (cls_name, mod_path)) + if module is None: + # If importing as an app module failed, that error probably + # contains the most informative traceback. Trigger it again. + import_module(entry) + else: + raise # Check for obvious errors. (This check prevents duck typing, but # it could be removed if it became a problem in practice.) diff --git a/tests/apps/failing_app/__init__.py b/tests/apps/failing_app/__init__.py new file mode 100644 index 0000000000..4ec7f18d40 --- /dev/null +++ b/tests/apps/failing_app/__init__.py @@ -0,0 +1 @@ +raise ImportError("Oops") diff --git a/tests/apps/tests.py b/tests/apps/tests.py index 2c7629ac97..513ed32039 100644 --- a/tests/apps/tests.py +++ b/tests/apps/tests.py @@ -166,6 +166,14 @@ class AppsTests(TestCase): with self.settings(INSTALLED_APPS=['apps.apps.RelabeledAppsConfig', 'apps']): pass + def test_import_exception_is_not_masked(self): + """ + App discovery should preserve stack traces. Regression test for #22920. + """ + with six.assertRaisesRegex(self, ImportError, "Oops"): + with self.settings(INSTALLED_APPS=['apps.failing_app']): + pass + def test_models_py(self): """ Tests that the models in the models.py file were loaded correctly.