AppConfig.create swallows informative exceptions during import time

[Ben Davis]

If you have an INSTALLED_APPS entry that doesn't use an AppConfig, and that module has more than one level, eg "myproject.myapp", AppConfig.create will swallow any exceptions that occur during the import of that module, losing valuable traceback information. The create method makes some incorrect assumptions about the entry. Mainly it assumes that if the entry has more than one part and the import fails, that it must be an AppConfig class (which is incorrect).

I've submitted a PR with a fix for this. Basically the new algorithm is as follows:

1. Split the entry into mod_path and ending. If it's a single-level module, ending is empty.
2. Attempt to import mod_path as module, allowing exceptions to propagate (no try/catch)
3. Check if ending is an attribute of module.
   • If the attribute doesn't exist OR the attribute exists and is a module, import the full entry as module (again, allowing exceptions to propagate)
   • If the imported module does NOT a default_app_config attr, return with a default AppConfig instance.
   • Otherwise, import the module path from default_app_config entry as module (again, allowing exceptions to propagate), and assume the ending is a class name.
4. If ending was an attribute of module in step 3, we can assume it's an AppConfig class.
5. At this point we have a legitimate module and an object which should be a class, and can continue on to validate the AppConfig class.

[Ben Davis]

PR with tests: ​https://github.com/django/django/pull/2861

[Aymeric Augustin]

I believe the issue is valid and we should address it.

Can you confirm that the exception masking happens on Python 2 — due to the lack of exception chaining — in this section of the code?

        except AttributeError:
            # Emulate the error that "from <mod_path> import <cls_name>"
            # would raise when <mod_path> exists but not <cls_name>, with
            # more context (Python just says "cannot import name ...").
            raise ImportError(
                "cannot import name '%s' from '%s'" % (cls_name, mod_path))

I'm not very proud of that code and I'm not surprised someone came and flamed it ;-)

---

If I read your patch correctly, at some point, you redo an import that you know will fail in order to obtain the correct traceback. Two questions:

1 - Why wasn't it sufficient to use that trick in the piece of code I quoted above?
2 - Isn't it going to be confusing on Python 3, because of the exception chaining ?

I read your patch carefully but I found it hard to validate because it rewrites a section that is inherently difficult to test. Thanks for adding tests, though, that's very helpful in any case.

---

I consider this a minor bug because it doesn't affect functionality, only error reporting on projets that don't work anyway. Did I miss the reason why you marked it as a blocker?

[Aymeric Augustin]

bendavis78, did you have the opportunity to review my questions above?

[Aymeric Augustin]

I'm proposing a less invasive fix in ​PR 3144.

I've already rewritten that method entirely once to address issues I don't remember very well. I'd rather not restructure the logic drastically just to improve error reporting.

[Aymeric Augustin <aymeric.augustin@…>]

In b161c01c48b4c353d7d1a47e9097391a8eb2e047:

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.

[Aymeric Augustin <aymeric.augustin@…>]

In c981d9140d569d489d338741b6a3d878c4e712da:

[1.7.x] 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.

Backport of b161c01 from master