Better errors for bad return values

src/labthings_fastapi/client/__init__.py

@@ -101,15 +101,24 @@ def poll_invocation(
     :param first_interval: sets how long we wait before the first
         polling request. Often, it makes sense for this to be a short
         interval, in case the action fails (or returns) immediately.
-
+    :raises ServerActionError: if an HTTP error is found during polling.
     :return: the completed invocation as a dictionary.
     """
     first_time = True
     while invocation["status"] in ACTION_RUNNING_KEYWORDS:
         time.sleep(first_interval if first_time else interval)
-        r = client.get(invocation_href(invocation))
-        r.raise_for_status()
-        invocation = r.json()
+        response = client.get(invocation_href(invocation))
+        if response.is_error:
+            try:
+                message = response.json()["detail"]
+            except KeyError:
+                message = response.text
+            raise ServerActionError(
+                f"The server returned error {response.status_code} while polling "
+                f"action '{invocation['action']}' with id '{invocation['id']}'. "
+                f"The error message was:\n{message}."
+            )
+        invocation = response.json()
         first_time = False
     return invocation
 

src/labthings_fastapi/exceptions.py

@@ -4,6 +4,8 @@
 # An __all__ for this module is less than helpful, unless we have an
 # automated check that everything's included.
 
+from collections.abc import Callable
+
 
 class NotConnectedToServerError(RuntimeError):
     """The Thing is not connected to a server.
@@ -254,6 +256,74 @@ class NoInvocationContextError(RuntimeError):
     """
 
 
+class CausedByUserCodeError(Exception):
+    """A mixin to allow exceptions to refer to downstream code."""
+
+    def _append_to_args(self, message: str) -> None:
+        """Add a message to the exception's arguments.
+
+        :param message: the message to append.
+        """
+        if len(self.args) == 1:
+            # If there's only one string, assume it's a message and append
+            self.args = (self.args[0] + "\n" + message,)
+        else:
+            # If there are multiple arguments, add this as a further one
+            self.args += (message,)
+
+    def set_source_function(self, func: Callable) -> None:
+        """Add the location of a user-supplied function to the error message.
+
+        :param func: the function that caused this error.
+        """
+        code = func.__code__
+        self._append_to_args(
+            f"This was likely caused by function '{code.co_name}' "
+            f"at {code.co_filename}:{code.co_firstlineno}"
+        )
+
+    def set_source_class(self, cls: type, attr: str | None = None) -> None:
+        """Add a reference to a class (and optionally attribute).
+
+        :param cls: the class that caused this error.
+        :param attr: the attribute name that caused this error.
+        """
+        self._append_to_args(
+            f"This was likely caused by '{cls.__module__}.{cls.__qualname__}.{attr}"
+            if attr
+            else "'."
+        )
+
+
+class InvalidReturnValueError(CausedByUserCodeError, RuntimeError):
+    r"""The return value from a method cannot be serialised by LabThings.
+
+    This error is raised when an action returns a value that can't be serialised.
+    This usually means that either it doesn't match the declared return type of
+    the function, or the declared return type permits un-serialisable values.
+
+    If an action's return type is missing or `Any`\ , it's possible to return a
+    value that can't be serialised, which will cause this error.
+
+    The solution is usually to ensure that the return type of your action is
+    either a simple type that can be serialised to JSON, or a Pydantic model.
+    You should also check that the function's return value matches the declared
+    type, ideally by regularly running a type checker like `mypy` on your code.
+    """
+
+
+class UnserializableTypeError(CausedByUserCodeError, TypeError):
+    r"""A type has been specified that can't be serialized to JSON.
+
+    This error generally means a property or action has a type that cannot be
+    serialized to JSON. This might be an instance of a custom class, or another
+    datatype that doesn't have a ready representation using JSON-compatible types.
+
+    This error can often be fixed using `pydantic` annotations, or by using simple
+    Python types instead of custom ones.
+    """
+
+
 class LogConfigurationError(RuntimeError):
     """There is a problem with logging configuration.
 

src/labthings_fastapi/invocations.py

@@ -11,7 +11,11 @@
 from typing import Optional, Any, Sequence, TypeVar, Generic
 import uuid
 
-from pydantic import BaseModel, ConfigDict, model_validator
+from pydantic import (
+    BaseModel,
+    ConfigDict,
+    model_validator,
+)
 
 from labthings_fastapi.middleware.url_for import URLFor
 
@@ -80,11 +84,31 @@ def generate_message(cls, data: Any) -> Any:
         return data
 
 
+class InvocationSummary(BaseModel):
+    """A model to represent `.Invocation` objects over HTTP.
+
+    This version of the model does not include logs our action outputs, and is intended
+    for use in endpoints that might list several invocations.
+
+    See `GenericInvocationModel` for the full representation, to be used in
+    endpoints referring to one specific invocation.
+    """
+
+    status: InvocationStatus
+    id: uuid.UUID
+    action: str
+    href: URLFor
+    timeStarted: Optional[datetime]
+    timeRequested: Optional[datetime]
+    timeCompleted: Optional[datetime]
+    links: Links = None
+
+
 InputT = TypeVar("InputT")
 OutputT = TypeVar("OutputT")
 
 
-class GenericInvocationModel(BaseModel, Generic[InputT, OutputT]):
+class GenericInvocationModel(InvocationSummary, Generic[InputT, OutputT]):
     """A model to serialise `.Invocation` objects when they are polled over HTTP.
 
     The input and output models are generic parameters, to allow this model to
@@ -93,17 +117,9 @@ class GenericInvocationModel(BaseModel, Generic[InputT, OutputT]):
     are not known in advance.
     """
 
-    status: InvocationStatus
-    id: uuid.UUID
-    action: str
-    href: URLFor
-    timeStarted: Optional[datetime]
-    timeRequested: Optional[datetime]
-    timeCompleted: Optional[datetime]
     input: InputT
     output: OutputT
     log: Sequence[LogRecordModel]
-    links: Links = None
 
 
 InvocationModel = GenericInvocationModel[Any, Any]

src/labthings_fastapi/properties.py

@@ -62,7 +62,7 @@ class attribute. Documentation is in strings immediately following the
 from typing_extensions import Self, TypedDict
 from weakref import WeakSet
 
-from fastapi import Body, FastAPI
+from fastapi import Body, FastAPI, Response, HTTPException
 from pydantic import (
     BaseModel,
     ConfigDict,
@@ -81,9 +81,10 @@ class attribute. Documentation is in strings immediately following the
     PropertyOp,
 )
 from .utilities import (
-    LabThingsRootModelWrapper,
+    RootModelWrapper,
     labthings_data,
-    wrap_plain_types_in_rootmodel,
+    serialize_from_user_code,
+    validate_from_user_code,
 )
 from .utilities.introspection import return_type
 from .base_descriptor import (
@@ -93,10 +94,12 @@ class attribute. Documentation is in strings immediately following the
 )
 from .exceptions import (
     FeatureNotAvailableError,
+    InvalidReturnValueError,
     NotConnectedToServerError,
     PropertyRedefinitionError,
     ReadOnlyPropertyError,
     MissingTypeError,
+    UnserializableTypeError,
     UnsupportedConstraintError,
 )
 from .thing_class_settings import get_validate_properties_on_set
@@ -464,12 +467,19 @@ def model(self) -> type[BaseModel]:
         subclass, this returns it unchanged.
 
         :return: a Pydantic model for the property's type.
+        :raises UnserializableTypeError: if the property can't be serialized
+            by `pydantic` to JSON.
         """
         if self._model is None:
-            self._model = wrap_plain_types_in_rootmodel(
-                self.value_type,
-                constraints=self.constraints,
-            )
+            try:
+                self._model = RootModelWrapper.wrap_type(
+                    self.value_type,
+                    constraints=self.constraints,
+                    name=f"{self.name.title()}Value",
+                )
+            except UnserializableTypeError as e:
+                e.set_source_class(self.owning_class, self.name)
+                raise
         return self._model
 
     def get_default(self, obj: Owner | None) -> Value:
@@ -559,8 +569,25 @@ def set_property(body: Any) -> None:
             summary=self.title,
             description=f"## {self.title}\n\n{self.description or ''}",
         )
-        def get_property() -> Any:
-            return self.__get__(thing)
+        def get_property() -> Response:
+            try:
+                instance = validate_from_user_code(
+                    model=self.model,
+                    value=self.__get__(thing),
+                    description=f"{thing.name}.{self.name}",
+                    code=(self.owning_class, self.name),
+                )
+                return serialize_from_user_code(
+                    model_instance=instance,
+                    description=f"{thing.name}.{self.name}",
+                    code=(self.owning_class, self.name),
+                )
+            except InvalidReturnValueError as e:
+                thing.logger.error(e)
+                raise HTTPException(
+                    status_code=500,
+                    detail=str(e),
+                ) from e
 
         if self.is_resettable(thing):
 
@@ -1270,7 +1297,7 @@ def validate(self, value: Any) -> Value:
             with its value type. This should never happen.
         """
         try:
-            if issubclass(self.model, LabThingsRootModelWrapper):
+            if issubclass(self.model, RootModelWrapper):
                 # If a plain type has been wrapped in a RootModel, use that to validate
                 # and then set the property to the root value.
                 model = self.model.model_validate(value)
@@ -1283,7 +1310,7 @@ def validate(self, value: Any) -> Value:
                 return self.value_type.model_validate(value)
 
             # This should be unreachable, because `model` is a
-            # `LabThingsRootModelWrapper` wrapping the value type, or the value type
+            # `RootModelWrapper` wrapping the value type, or the value type
             # should be a BaseModel.
             msg = f"Property {self.name} has an inconsistent model. This is "
             msg += f"most likely a LabThings bug. {self.model=}, {self.value_type=}"

src/labthings_fastapi/server/__init__.py

@@ -9,6 +9,7 @@
 import warnings
 from fastapi.testclient import TestClient
 from pydantic import ValidationError
+from pydantic_core import PydanticSerializationError
 from typing import Any, AsyncGenerator, Optional, TypeVar, overload
 from fastapi.responses import JSONResponse
 from typing_extensions import Self
@@ -50,6 +51,9 @@
 ThingSubclass = TypeVar("ThingSubclass", bound=Thing)
 
 
+LOGGER = logging.getLogger(__name__)
+
+
 class ThingServer:
     """Use FastAPI to serve `~lt.Thing` instances.
 
@@ -141,7 +145,7 @@ def __init__(
             self._config = ThingServerConfig(**kwargs)
         if self._config.settings_folder is None:
             self._config.settings_folder = "./settings"
-        self.app = FastAPI(lifespan=self.lifespan)
+        self.app = FastAPI(lifespan=self.lifespan, separate_input_output_schemas=False)
         self._set_cors_middleware()
         self._set_url_for_middleware()
         self._add_exception_handlers()
@@ -248,6 +252,16 @@ async def global_lock_exception_handler(
                 content={"detail": repr(exc)},
             )
 
+        @self.app.exception_handler(PydanticSerializationError)
+        async def serialization_error_handler(
+            request: Request, exc: PydanticSerializationError
+        ) -> JSONResponse:
+            LOGGER.error(
+                f"Couldn't serialize response to {request.url} because of error: \n"
+                f"{exc}"
+            )
+            return JSONResponse(status_code=500, content={"detail": str(exc)})
+
     @property
     def debug(self) -> bool:
         """Whether the server is in debug mode."""