Don't hide true source of error when catching ImportError #4259

New Issue

adam · 2025-12-29T18:34:17+01:00

adam commented

2025-12-29 18:34:17 +01:00

Originally created by @candlerb on GitHub (Nov 9, 2020).

Originally assigned to: @jeremystretch on GitHub.

Environment

Python version: 3.6.9
NetBox version: 2.9.8

Proposed Functionality

There are a number of places which catch ImportError and replace with a simplified error message, e.g. in authentication.py:

        try:
            from netbox import ldap_config
        except ImportError:
            raise ImproperlyConfigured(
                "ldap_config.py does not exist"
            )

This can give an incorrect diagnosis of the problem, which leads to hard-to-debug problems, especially when trying to support users remotely.

I would propose either:

Don't catch the ImportError at all. The backtrace will still say ModuleNotFoundError: No module named 'ldap_config' which is pretty obvious.

Catch it, but include the underlying error:

        try:
            from netbox import ldap_config
        except ImportError as err:
            raise ImproperlyConfigured(
                "ldap_config.py does not exist or other problem: %s" % err
            )

(also it may be better to catch ModuleNotFoundError, which is a subclass of ImportError)

Catch it, but check for the exact condition before making a summary diagnosis

        try:
            from netbox import ldap_config
        except ModuleNotFoundError as err:
            if getattr(err, "name") == "ldap_config":
                raise ImproperlyConfigured(
                    "ldap_config.py does not exist"
                )
            raise

Use Case

A real-world case from the forum. A user created ldap_config.py which in turn contained an import error. However Netbox then insisted that the error is "ldap_config.py does not exist" which was not true, hiding the actual error. The user went on a wild goose chase of making additional copies of ldap_config.py in various places.

Database Changes

None

External Dependencies

None

Originally created by @candlerb on GitHub (Nov 9, 2020). Originally assigned to: @jeremystretch on GitHub. ### Environment * Python version: 3.6.9 * NetBox version: 2.9.8 ### Proposed Functionality There are a number of places which catch ImportError and replace with a simplified error message, e.g. in [authentication.py](https://github.com/netbox-community/netbox/blob/develop/netbox/netbox/authentication.py#L147-L152): ``` try: from netbox import ldap_config except ImportError: raise ImproperlyConfigured( "ldap_config.py does not exist" ) ``` This can give an incorrect diagnosis of the problem, which leads to hard-to-debug problems, especially when trying to support users remotely. I would propose either: 1. Don't catch the ImportError at all. The backtrace will still say `ModuleNotFoundError: No module named 'ldap_config'` which is pretty obvious. 2. Catch it, but include the underlying error: ``` try: from netbox import ldap_config except ImportError as err: raise ImproperlyConfigured( "ldap_config.py does not exist or other problem: %s" % err ) ``` (also it may be better to catch `ModuleNotFoundError`, which is a subclass of `ImportError`) 3. Catch it, but check for the exact condition before making a summary diagnosis ``` try: from netbox import ldap_config except ModuleNotFoundError as err: if getattr(err, "name") == "ldap_config": raise ImproperlyConfigured( "ldap_config.py does not exist" ) raise ``` ### Use Case A real-world case from the [forum](https://groups.google.com/d/topic/netbox-discuss/lXxMQM5w3Tw/discussion). A user created `ldap_config.py` which in turn contained an import error. However Netbox then insisted that the error is "ldap_config.py does not exist" which was not true, hiding the actual error. The user went on a wild goose chase of making additional copies of ldap_config.py in various places. ### Database Changes None ### External Dependencies None

adam added the status: accepted type: housekeeping labels 2025-12-29 18:34:17 +01:00

adam closed this issue

2025-12-29 18:34:17 +01:00

Sign in to join this conversation.

Branches Tags

main

update-changelog-comments-docs

feature-removal-issue-type

20911-dropdown

20239-plugin-menu-classes-mutable-state

21097-graphql-id-lookups

feature

fix_module_substitution

20923-dcim-templates

20044-elevation-stuck-lightmode

feature-ip-prefix-link

v4.5-beta1-release

20068-import-moduletype-attrs

20766-fix-german-translation-code-literals

20378-del-script

7604-filter-modifiers-v3

circuit-swap

12318-case-insensitive-uniqueness

20637-improve-device-q-filter

20660-script-load

19724-graphql

20614-update-ruff

14884-script

02496-max-page

19720-macaddress-interface-generic-relation

19408-circuit-terminations-export-templates

20203-openapi-check

fix-19669-api-image-download

7604-filter-modifiers

19275-fixes-interface-bulk-edit

fix-17794-get_field_value_return_list

11507-show-aggregate-and-rir-on-api

9583-add_column_specific_search_field_to_tables

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/netbox#4259