Skip to content

Conversation

@rwb27
Copy link
Collaborator

@rwb27 rwb27 commented Dec 14, 2025

This uses a WrapValidator to improve the handling of errors in an ImportString.

The problem, identified in #222, is that TypeError and ValueError are treated specially by pydantic. This means that if either of those exceptions are raised during module import, we get a confusing validation error.

My wrap validator ensures that we get either a sensible import-related error (e.g. module not found, name not found in module) or a single clear error stating that an error occurred during module import.

This does not dump a traceback, because doing so during validation tends to lead to very confusing messages. However, it should make it obvious that the problem is an unimportable module, and point someone in the right direction.

Had this been implemented last week, I hope it might have saved @julianstirling some head-scratching.

For example:

>       ThingConfig(cls="tests.unimportable.valueerror:SomeClass")
E       pydantic_core._pydantic_core.ValidationError: 1 validation error for ThingConfig
E       cls
E         An exception was raised when importing 'tests.unimportable.valueerror:SomeClass'. [type=import_error, input_value='tests.unimportable.valueerror:SomeClass', input_type=str]

This does not come across quite as nicely if we use the shorthand:

>       ThingServerConfig(things={"thing":"tests.unimportable.valueerror:SomeClass"})
E       pydantic_core._pydantic_core.ValidationError: 2 validation errors for ThingServerConfig
E       things.thing.ThingConfig
E         Input should be a valid dictionary or instance of ThingConfig [type=model_type, input_value='tests.unimportable.valueerror:SomeClass', input_type=str]
E           For further information visit https://errors.pydantic.dev/2.10/v/model_type
E       things.thing.function-wrap[contain_import_errors()]
E         An exception was raised when importing 'tests.unimportable.valueerror:SomeClass'. [type=import_error, input_value='tests.unimportable.valueerror:SomeClass', input_type=str]

The first error is due to the way pydantic handles union types. It might be possible to improve on that, though I'm not 100% sure how - possibly by annotating the union, rather than just the ImportString.

This uses a WrapValidator to improve the handling of errors in an ImportString.

The problem, identified in #222, is that `TypeError` and `ValueError` are treated specially by `pydantic`.
This means that if either of those exceptions are raised during module import, we get a confusing validation error.

My wrap validator ensures that we get either a sensible import-related error (e.g. moule not found, name not found in module) or a single clear error stating that an error occurred during module import.

This does not dump a traceback, because doing so during validation tends to lead to very confusing messages.
However, it should make it obvious that the problem is an unimportable module, and point someone in the right direction.
@barecheck
Copy link

barecheck bot commented Dec 14, 2025

Barecheck - Code coverage report

Total: 95.09%

Your code coverage diff: 0.02% ▴

✅ All code changes are covered

@julianstirling
Copy link
Contributor

This is slightly less bad, but it still doesn't point to the actual error, which means I need to then start trying to manually import to regenerate the stack to find the error.

If it doesn't point to a line in our code then it is not a traceback that can be used to isolate the actual error and fix it. In these dummy examples the error is obvious. If I recreate the error I have changing one line from:

downsampled_array_factor: int = lt.property(default=2)

to

downsampled_array_factor: int = lt.property(2, default=2)

This happens to be in base camera. So I get this as my report:

Traceback (most recent call last):
  File "/home/js3214/Documents/gitstuffs/openflexure-microscope-server/src/openflexure_microscope_server/server/__init__.py", line 93, in serve_from_cli
    server = lt.ThingServer.from_config(config)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/js3214/Documents/gitstuffs/openflexure-microscope-server/.venv/lib/python3.11/site-packages/labthings_fastapi/server/__init__.py", line 111, in from_config
    return cls(**dict(config))
           ^^^^^^^^^^^^^^^^^^^
  File "/home/js3214/Documents/gitstuffs/openflexure-microscope-server/.venv/lib/python3.11/site-packages/labthings_fastapi/server/__init__.py", line 85, in __init__
    self._config = ThingServerConfig(things=things, settings_folder=settings_folder)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/js3214/Documents/gitstuffs/openflexure-microscope-server/.venv/lib/python3.11/site-packages/pydantic/main.py", line 214, in __init__
    validated_self = self.__pydantic_validator__.validate_python(data, self_instance=self)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.ValidationError: 6 validation errors for ThingServerConfig
things.camera.ThingConfig
  Input should be a valid dictionary or instance of ThingConfig [type=model_type, input_value='openflexure_microscope_s...ulation:SimulatedCamera', input_type=str]
    For further information visit https://errors.pydantic.dev/2.10/v/model_type
things.camera.function-wrap[contain_import_errors()]
  An exception was raised when importing 'openflexure_microscope_server.things.camera.simulation:SimulatedCamera'. [type=import_error, input_value='openflexure_microscope_s...ulation:SimulatedCamera', input_type=str]
things.autofocus.ThingConfig
  Input should be a valid dictionary or instance of ThingConfig [type=model_type, input_value='openflexure_microscope_s...utofocus:AutofocusThing', input_type=str]
    For further information visit https://errors.pydantic.dev/2.10/v/model_type
things.autofocus.function-wrap[contain_import_errors()]
  An exception was raised when importing 'openflexure_microscope_server.things.autofocus:AutofocusThing'. [type=import_error, input_value='openflexure_microscope_s...utofocus:AutofocusThing', input_type=str]
things.camera_stage_mapping.ThingConfig
  Input should be a valid dictionary or instance of ThingConfig [type=model_type, input_value='openflexure_microscope_s...pping:CameraStageMapper', input_type=str]
    For further information visit https://errors.pydantic.dev/2.10/v/model_type
things.camera_stage_mapping.function-wrap[contain_import_errors()]
  An exception was raised when importing 'openflexure_microscope_server.things.camera_stage_mapping:CameraStageMapper'. [type=import_error, input_value='openflexure_microscope_s...pping:CameraStageMapper', input_type=str]

This error isn't clear, and will require significant manual importing to find which simply isn't reasonable for a single simple error. We need to be able to isolate the ValueError and raise it traceback

@julianstirling julianstirling closed this pull request by merging all changes into main in 8d4340e Dec 15, 2025
@julianstirling julianstirling deleted the better_import_errors branch December 15, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants