starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Improve handling of plugin loading errors #3820

New Issue
Closed
opened 2025-12-29 18:31:22 +01:00 by adam · 2 comments
adam commented 2025-12-29 18:31:22 +01:00
Owner
Copy Link

Originally created by @glennmatthews on GitHub (Jun 30, 2020).

Environment

  • Python version: 3.7.7
  • NetBox version: v2.8.4

Proposed Functionality

Currently, when developing a plugin, errors in the plugin code under development can cause undesirable NetBox behavior. If the plugin code is severely broken (such as a SyntaxError, then NetBox as a whole will die when attempting to load the plugin. In other cases, such as where the plugin code throws an ImportError due to a missing or incorrectly specified dependency, NetBox will silently ignore the error, the admin "Installed Plugins" page will still show the broken plugin, but attempts to access the plugin later (via its URLs, views, etc.) will report a confusing error such as NoReverseMatch at / 'plugin_name' is not a registered namespace inside 'plugins'.

See also #4628.

Currently NetBox loads plugins via three primary functions: importlib.import_module(), django.apps.apps.get_app_config() and django.utils.module_loading.import_string().

importlib.import_module()

This is called in netbox/netbox/settings.py:

for plugin_name in PLUGINS:

    # Import plugin module
    try:
        plugin = importlib.import_module(plugin_name)
    except ImportError:
        raise ImproperlyConfigured(
            "Unable to import plugin {}: Module not found. Check that the plugin module has been installed within the "
            "correct Python environment.".format(plugin_name)
        )

In the case of an ImportError, this should re-raise an ImproperlyConfigured error, but in practice it appears that Django is silently ignoring that exception. (Possible NetBox bug of some sort?) This then causes errors downstream when get_app_config is called - see below.

In the case of any other exception being thrown by the plugin, the entire Django startup process aborts.

Proposal: add a blanket except Exception to this try block, and log the exception and continue without the plugin, so that NetBox itself remains operational.

django.apps.apps.get_app_config()

This is called in netbox/extras/plugins/urls.py and netbox/extras/plugins/views.py, and will raise a LookupError if the requested plugin is not present (such as due to an earlier error in settings.py calling import_module() above), which is not handled by NetBox, causing the entire application to abort.

Proposal: catch the LookupError in each case, print an appropriate error message to the Django console, and skip over the affected plugin. This will fix the case where broken plugins are still listed under the admin "Installed Plugins" view and the api/plugins/installed-plugins/ API view.

Proposal: A possible further enhancement would be for these two views to, rather than simply skipping broken plugins, separately report any plugins that are configured but could not be loaded successfully.

django.utils.module_loading.import_string()

This method raises an ImportError if:

  1. the specified string is not a valid dotted path specification (module.submodule.object)
  2. importing the specified module/submodule raises an ImportError, because either:
    a. it does not exist
    b. it exists but fails to load, perhaps because it itself contains an import statement that fails (the case described in this issue)
  3. the specified module/submodule exists but does not contain the specified named object.

In the case of plugin loading where we are using import_string (extras/plugins/__init__.py, extras/plugins/views.py, extras/plugins/urls.py) we avoid case 1. by properly constructing the string ourselves. We want case 2.a. (and 3.?) to fail silently because, for example, a plugin very well may not implement a plugin_name.urls module (or have a urlpatterns declaration in said module?).

Proposal: Case 2.b. should result in a reported error message, and possibly also this failure to be reported in some fashion under the /admin/plugins/installed-plugins/ view. This could probably be accomplished by rolling our own implementation of import_string, which is a relatively simple function.

Use Cases

  1. Plugin developers will be more readily able to diagnose and repair errors in the plugin code.
  2. Deployments of NetBox will be more resilient to errors in plugin code that are outside NetBox's direct control.

Database Changes

None are proposed.

External Dependencies

None identified.

Originally created by @glennmatthews on GitHub (Jun 30, 2020). ### Environment * Python version: 3.7.7 * NetBox version: v2.8.4 ### Proposed Functionality Currently, when developing a plugin, errors in the plugin code under development can cause undesirable NetBox behavior. If the plugin code is severely broken (such as a `SyntaxError`, then NetBox as a whole will die when attempting to load the plugin. In other cases, such as where the plugin code throws an `ImportError` due to a missing or incorrectly specified dependency, NetBox will silently ignore the error, the admin "Installed Plugins" page will still show the broken plugin, but attempts to access the plugin later (via its URLs, views, etc.) will report a confusing error such as `NoReverseMatch at / 'plugin_name' is not a registered namespace inside 'plugins'`. See also #4628. Currently NetBox loads plugins via three primary functions: `importlib.import_module()`, `django.apps.apps.get_app_config()` and `django.utils.module_loading.import_string()`. #### importlib.import_module() This is called in `netbox/netbox/settings.py`: ```python for plugin_name in PLUGINS: # Import plugin module try: plugin = importlib.import_module(plugin_name) except ImportError: raise ImproperlyConfigured( "Unable to import plugin {}: Module not found. Check that the plugin module has been installed within the " "correct Python environment.".format(plugin_name) ) ``` In the case of an `ImportError`, this should re-raise an `ImproperlyConfigured` error, but in practice it appears that Django is silently ignoring that exception. (Possible NetBox bug of some sort?) This then causes errors downstream when `get_app_config` is called - see below. In the case of any other exception being thrown by the plugin, the entire Django startup process aborts. *Proposal*: add a blanket `except Exception` to this `try` block, and log the exception and continue without the plugin, so that NetBox itself remains operational. #### django.apps.apps.get_app_config() This is called in `netbox/extras/plugins/urls.py` and `netbox/extras/plugins/views.py`, and will raise a `LookupError` if the requested plugin is not present (such as due to an earlier error in `settings.py` calling `import_module()` above), which is not handled by NetBox, causing the entire application to abort. *Proposal*: catch the LookupError in each case, print an appropriate error message to the Django console, and skip over the affected plugin. This will fix the case where broken plugins are still listed under the admin "Installed Plugins" view and the `api/plugins/installed-plugins/` API view. *Proposal*: A possible further enhancement would be for these two views to, rather than simply skipping broken plugins, separately report any plugins that are configured but could not be loaded successfully. #### django.utils.module_loading.import_string() This method raises an `ImportError` if: 1. the specified string is not a valid dotted path specification (`module.submodule.object`) 2. importing the specified module/submodule raises an `ImportError`, because either: a. it does not exist b. it exists but fails to load, perhaps because it itself contains an `import` statement that fails (the case described in this issue) 3. the specified module/submodule exists but does not contain the specified named object. In the case of plugin loading where we are using `import_string` (`extras/plugins/__init__.py`, `extras/plugins/views.py`, `extras/plugins/urls.py`) we avoid case 1. by properly constructing the string ourselves. We want case 2.a. (and 3.?) to fail silently because, for example, a plugin very well may not implement a `plugin_name.urls` module (or have a `urlpatterns` declaration in said module?). *Proposal*: Case 2.b. should result in a reported error message, and possibly also this failure to be reported in some fashion under the /admin/plugins/installed-plugins/ view. This could probably be accomplished by rolling our own implementation of `import_string`, which is a relatively simple function. ### Use Cases 1. Plugin developers will be more readily able to diagnose and repair errors in the plugin code. 2. Deployments of NetBox will be more resilient to errors in plugin code that are outside NetBox's direct control. ### Database Changes None are proposed. ### External Dependencies None identified.
adam added the status: acceptedtype: feature labels 2025-12-29 18:31:22 +01:00
adam closed this issue 2025-12-29 18:31:22 +01:00
adam commented 2025-12-29 18:31:22 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Jun 30, 2020):

If the plugin code is severely broken (such as a SyntaxError), then NetBox as a whole will die when attempting to load the plugin.

What would be the benefit of allowing NetBox to run with an incorrectly installed plugin? A configuration which references a missing or broken plugin should cause the NetBox process to fail and report the error, calling the user's attention to the problem. Suppressing the error will only lead to confusion, especially if the user does not have adequate logging configured.

In the case of an ImportError, this should re-raise an ImproperlyConfigured error, but in practice it appears that Django is silently ignoring that exception. (Possible NetBox bug of some sort?)

I cannot reproduce this on NetBox v2.8.6. Attempting to import an invalid plugin, or intentionally raising an ImportError within a valid plugin, raises an ImproperlyConfigured with the expected error message.

In the case of plugin loading where we are using import_string [...] we avoid case 1. by properly constructing the string ourselves.

There's no requirement that a plugin must define these modules: Even though the path is correct, the file may not necessarily exist.

I agree that in "case 2b" it would be ideal to raise ImportErrors resulting from the plugin itself, however that will require introducing separate logic to check for the existence of each optional plugin module (template_extensions.py, urlpatterns.py, etc.) prior to attempting to import it. For example, instead of:

try:
    template_extensions = import_string(f"{self.__module__}.{self.template_extensions}")
    register_template_extensions(template_extensions)
except ImportError:
    pass

We'd do something like:

spec = importlib.util.find_spec(self.template_extensions, package=self.__module__)
if spec is not None:
    template_extensions = importlib.util.module_from_spec(spec)
    register_template_extensions(template_extensions)

This avoids intercepting any ImportErrors. However, it should be noted that exceptions remain the responsibility of the plugin author: If they make it to NetBox, they will cause NetBox to fail just as they do in core.

@jeremystretch commented on GitHub (Jun 30, 2020): > If the plugin code is severely broken (such as a `SyntaxError`), then NetBox as a whole will die when attempting to load the plugin. What would be the benefit of allowing NetBox to run with an incorrectly installed plugin? A configuration which references a missing or broken plugin _should_ cause the NetBox process to fail and report the error, calling the user's attention to the problem. Suppressing the error will only lead to confusion, especially if the user does not have adequate logging configured. > In the case of an `ImportError`, this should re-raise an `ImproperlyConfigured` error, but in practice it appears that Django is silently ignoring that exception. (Possible NetBox bug of some sort?) I cannot reproduce this on NetBox v2.8.6. Attempting to import an invalid plugin, or intentionally raising an `ImportError` within a valid plugin, raises an `ImproperlyConfigured` with the expected error message. > In the case of plugin loading where we are using import_string [...] we avoid case 1. by properly constructing the string ourselves. There's no requirement that a plugin _must_ define these modules: Even though the path is correct, the file may not necessarily exist. I agree that in "case 2b" it would be ideal to raise `ImportErrors` resulting from the plugin itself, however that will require introducing separate logic to check for the existence of each optional plugin module (`template_extensions.py`, `urlpatterns.py`, etc.) prior to attempting to import it. For example, instead of: ```python try: template_extensions = import_string(f"{self.__module__}.{self.template_extensions}") register_template_extensions(template_extensions) except ImportError: pass ``` We'd do something like: ```python spec = importlib.util.find_spec(self.template_extensions, package=self.__module__) if spec is not None: template_extensions = importlib.util.module_from_spec(spec) register_template_extensions(template_extensions) ``` This avoids intercepting any ImportErrors. However, it should be noted that exceptions remain the responsibility of the plugin author: If they make it to NetBox, they will cause NetBox to fail just as they do in core.
adam commented 2025-12-29 18:31:22 +01:00
Author
Owner
Copy Link

@glennmatthews commented on GitHub (Jun 30, 2020):

Fair enough, I can understand where you're coming from as far as wanting to fail fast if a plugin error is present. That honestly simplifies the solution a fair bit.

I cannot reproduce this on NetBox v2.8.6. Attempting to import an invalid plugin, or intentionally raising an ImportError within a valid plugin, raises an ImproperlyConfigured with the expected error message.

It's very weird. In my installation, only this specific ImproperlyConfigured appears to be being ignored - if I raise one earlier in settings.py, it gets reported as expected; if I change this to a RuntimeError it gets reported as expected; but as implemented, an ImproperlyConfigured raised at this specific point just causes the execution of settings.py to end at this point with no error being reported. I'll assume it's just something bizarre about my particular setup.

There's no requirement that a plugin must define these modules: Even though the path is correct, the file may not necessarily exist.

Understood and agreed.

I agree that in "case 2b" it would be ideal to raise ImportErrors resulting from the plugin itself, however that will require introducing separate logic to check for the existence of each optional plugin module (template_extensions.py, urlpatterns.py, etc.) prior to attempting to import it.

Agreed. That's what I think we would benefit from here.

@glennmatthews commented on GitHub (Jun 30, 2020): Fair enough, I can understand where you're coming from as far as wanting to fail fast if a plugin error is present. That honestly simplifies the solution a fair bit. > I cannot reproduce this on NetBox v2.8.6. Attempting to import an invalid plugin, or intentionally raising an ImportError within a valid plugin, raises an ImproperlyConfigured with the expected error message. It's very weird. In my installation, only this specific `ImproperlyConfigured` appears to be being ignored - if I raise one earlier in `settings.py`, it gets reported as expected; if I change this to a `RuntimeError` it gets reported as expected; but as implemented, an `ImproperlyConfigured` raised at this specific point just causes the execution of `settings.py` to end at this point with no error being reported. I'll assume it's just something bizarre about my particular setup. >There's no requirement that a plugin must define these modules: Even though the path is correct, the file may not necessarily exist. Understood and agreed. > I agree that in "case 2b" it would be ideal to raise ImportErrors resulting from the plugin itself, however that will require introducing separate logic to check for the existence of each optional plugin module (template_extensions.py, urlpatterns.py, etc.) prior to attempting to import it. Agreed. That's what I think we would benefit from here.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label status: accepted type: feature
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#3820
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?