Raise the real import error when a thing cannot import

Author: julianstirling

src/labthings_fastapi/server/cli.py

@@ -27,7 +27,7 @@
 
 from . import ThingServer
 from . import fallback
-from .config_model import ThingServerConfig
+from .config_model import ThingServerConfig, ThingImportFailure
 
 
 def get_default_parser() -> ArgumentParser:
@@ -156,7 +156,7 @@ def serve_from_cli(
             app.labthings_error = e
             uvicorn.run(app, host=args.host, port=args.port)
         else:
-            if isinstance(e, ValidationError):
+            if isinstance(e, (ValidationError, ThingImportFailure)):
                 print(f"Error reading LabThings configuration:\n{e}")
                 sys.exit(3)
             else:

src/labthings_fastapi/server/config_model.py

@@ -6,20 +6,29 @@
 start servers based on configuration files or strings.
 """
 
+from importlib import import_module
+import re
 from pydantic import (
     BaseModel,
     Field,
     ImportString,
     AliasChoices,
     field_validator,
-    ValidationError,
     ValidatorFunctionWrapHandler,
     WrapValidator,
 )
-from pydantic_core import PydanticCustomError
 from typing import Any, Annotated, TypeAlias
 from collections.abc import Mapping, Sequence, Iterable
 
+PYTHON_EL_RE_STR = r"[a-zA-Z_][a-zA-Z0-9_]*"
+IMPORT_REGEX = re.compile(
+    rf"^{PYTHON_EL_RE_STR}(?:\.{PYTHON_EL_RE_STR})*:{PYTHON_EL_RE_STR}$"
+)
+
+
+class ThingImportFailure(BaseException):
+    """Failed to import Thing. Raise with import traceback."""
+
 
 def contain_import_errors(value: Any, handler: ValidatorFunctionWrapHandler) -> Any:
     """Prevent errors during import from causing odd validation errors.
@@ -39,17 +48,35 @@ def contain_import_errors(value: Any, handler: ValidatorFunctionWrapHandler) ->
     """
     try:
         return handler(value)
-    except Exception as e:
-        # ImportErrors get wrapped as ValidationErrors, and that's fine:
-        # it results in a sensible error message.
-        if isinstance(e, ValidationError):
-            if all(err["type"] == "import_error" for err in e.errors()):
-                raise
-        raise PydanticCustomError(
-            "import_error",
-            "An exception was raised when importing '{name}'.",
-            {"name": str(value), "error": e},
-        ) from e
+    except Exception:
+        # In the case where this is a matching import rule.
+        if isinstance(value, str) and IMPORT_REGEX.match(value):
+            # Try to import the module again
+            module_name = value.split(":")[0]
+            thing_name = value.split(":")[1]
+            try:
+                module = import_module(module_name)
+            except Exception as import_err:  # noqa: BLE001
+                # Capture the import exception and raise as a ThingImportFailure which
+                # is a subclass of BaseException.
+                msg = f"[{type(import_err).__name__}] {import_err}"
+                exc = ThingImportFailure(msg)
+                # Raise from None so the traceback is just the clear import traceback.
+                raise exc.with_traceback(import_err.__traceback__) from None
+
+            # If check the Thing is there and if not raise the ThingImportFailure
+            # wrapping an ImportError.
+            if not hasattr(module, thing_name):
+                msg = (
+                    f"[ImportError] cannot import name '{thing_name}' from "
+                    f"'{module_name}'"
+                )
+                # Raise from None so the traceback is just the clear import traceback.
+                raise ThingImportFailure(msg) from None
+
+        # If this was the wrong type, didn't match the regex, or somehow imported fine
+        # then re-raise the original error.
+        raise
 
 
 ThingImportString = Annotated[

tests/test_server_config_model.py

@@ -2,7 +2,11 @@
 
 from pydantic import ValidationError
 import pytest
-from labthings_fastapi.server.config_model import ThingConfig, ThingServerConfig
+from labthings_fastapi.server.config_model import (
+    ThingConfig,
+    ThingServerConfig,
+    ThingImportFailure,
+)
 import labthings_fastapi.example_things
 from labthings_fastapi.example_things import MyThing
 
@@ -40,7 +44,6 @@ def test_ThingConfig():
     {},
     {"foo": "bar"},
     {"class": MyThing, "kwargs": 1},
-    "missing.module:object",
     4,
     None,
     False,
@@ -98,22 +101,42 @@ def test_ThingServerConfig():
             ThingServerConfig(things={name: MyThing})
 
 
-def test_unimportable_module():
+def test_unimportable_modules():
     """Test that unimportable modules raise errors as expected."""
-    expected_message = "exception was raised when importing 'tests.unimportable"
-    with pytest.raises(ValidationError, match=expected_message):
+
+    with pytest.raises(
+        ThingImportFailure, match="\[ModuleNotFoundError\] No module named 'missing'"
+    ):
+        ThingConfig(cls="missing.module:object")
+
+    with pytest.raises(
+        ThingImportFailure,
+        match="\[RuntimeError\] This module should not be importable!",
+    ):
         # This checks RuntimErrors get reported with a single error
         ThingConfig(cls="tests.unimportable.runtimeerror:SomeClass")
-    with pytest.raises(ValidationError, match=expected_message):
+
+    with pytest.raises(
+        ThingImportFailure,
+        match="\[ValueError\] This module should not be importable due to ValueError!",
+    ):
         # This checks ValueErrors get reported with a single error
         # rather than getting swallowed by a ValidationError
         ThingConfig(cls="tests.unimportable.valueerror:SomeClass")
+
     with pytest.raises(
-        ValidationError,
-        match="No module named 'tests.unimportable.missing'",
+        ThingImportFailure,
+        match="\[ModuleNotFoundError\] No module named 'tests.unimportable.missing'",
     ):
         # This checks normal ImportErrors get reported as usual
         ThingConfig(cls="tests.unimportable.missing:SomeClass")
-    with pytest.raises(ValidationError, match="cannot import name 'MissingClass'"):
+
+    with pytest.raises(
+        ThingImportFailure,
+        match=(
+            "\[ImportError\] cannot import name 'MissingClass' from "
+            "'tests.unimportable'"
+        ),
+    ):
         # This checks normal ImportErrors get reported as usual
         ThingConfig(cls="tests.unimportable:MissingClass")