Invalid import inside plugin code can result in partially loaded plugin

[dgarros]

Environment

• Python version: 3.7.7
• NetBox version: 2.8.3

Steps to Reproduce

1. Install a plugin that is importing a non valid object, for example in views.py
2. Start NetBox
3. Navigate to the plugin section in the admin page (/admin/plugins/installed-plugins/)

Expected Behavior

If a plugin code is not valid an error message should be generated when NetBox is starting and the plugin should be ignored/not installed.

Ideally the error message should provide some information to troubleshoot the issue.

Observed Behavior

When the code of a plugin is not valid, there is no error message visible when NetBox start and the plugin is partially loaded.
The plugin will be listed under /admin/plugins/installed-plugins/
Depending on what is broken and what features the plugin is using it could result in a broken UI too.
In my case the import error was in the views.py file and the plugin included a navigation.py file to add a menu.
The UI was not working anymore because the namespace of the plugin was not properly defined.

NoReverseMatch at /
'netbox_onboarding' is not a registered namespace inside 'plugins'

[lampwins]

I'm not sure what we can really do here as this is simply a side effect of the interpreted nature of Python coupled with some runtime specifics of Django. The fact is, the view module is not imported by Django until the URL path is actually evaluated which occurs on a reverse lookup. In other words, Django doesn't know there is a problem until it actually tries to resolve the URL and I am not aware of a way to force that at startup. I think this really just comes down to the testing burden on the plugin developer.

[glennmatthews]

At least one place that logging and/or more extensive error handling for plugins would be useful is in extras/plugins/urls.py:

# Register base/API URL patterns for each plugin
for plugin_path in settings.PLUGINS:
    plugin_name = plugin_path.split('.')[-1]
    app = apps.get_app_config(plugin_name)     # GFM: an error in the plugin's base __init__.py will not be caught here, and NetBox will abort entirely
    base_url = getattr(app, 'base_url') or app.label

    # Check if the plugin specifies any base URLs
    try:
        urlpatterns = import_string(f"{plugin_path}.urls.urlpatterns")
        plugin_patterns.append(
            path(f"{base_url}/", include((urlpatterns, app.label)))
        )
    except ImportError:
        pass   # GFM: an ImportError here will be silently ignored; an error message would be preferable
    # GFM:  note that any other exception in loading the plugin urls (SyntaxError, for example) will cause NetBox to abort

    # Check if the plugin specifies any API URLs
    try:
        urlpatterns = import_string(f"{plugin_path}.api.urls.urlpatterns")
        plugin_api_patterns.append(
            path(f"{base_url}/", include((urlpatterns, f"{app.label}-api")))
        )
    except ImportError:
        pass   # GFM: same comments here

It would be good to log something in the ImportError case instead of a simple pass.