Better errors for bad return values by rwb27 · Pull Request #333 · labthings/labthings-fastapi

rwb27 · 2026-05-06T13:05:29Z

There are quite a few places in the codebase where values that come from downstream code are returned over HTTP. As a rule, these values are returned from endpoint functions, and FastAPI then attempts to serialise them using pydantic. If the value cannot be serialized or validated by the model (because we need a model instance to serialize from, and we can only get one by validating the input data), we get an ugly error. "ugly" in this case means there's an enormous stack trace, almost all of which refers to fastapi or starlette or pydantic and none of which tells you where in the code you actually wrote the problem arose.

This merge request attempts to address that problem, by taking control of how values are serialized. This applies to:

Reading the value of a property
Starting an action
Polling an action
Listing actions
Retrieving action outputs

The first change is that we now instantiate models ourselves, rather than relying on FastAPI to do it. This ensures we can catch and handle any validation errors.

The second change is that we now serialize those models to JSON and return a Response instead of returning a model instance. This allows us to catch serialization errors.

I've added some helper functions to keep this neat, and also to ensure we provide useful hints in the error message as to where in the downstream code the problem may have occurred - for example, actions will give the name of the action, and the file and line number where the function was defined.

All together, these changes mean that, when a bad value is passed back to a client, we respond with a helpful 500 HTTP error response and log a useful error on the server.

There are more places in LabThings where error handling could be improved: the biggest one I can think of is the thing description, where bad types or bad default values can lead to hard-to-debug errors. I have deliberately not swallowed all validation/serialization errors, just the ones for which we can obviously point to a cause.

barecheck · 2026-05-07T08:30:57Z

Barecheck - Code coverage report

Total: 96.46%

Your code coverage diff: -0.41% ▾

Uncovered files and lines

File	Lines
src/labthings_fastapi/actions.py	556-557, 570, 606-607, 778-780, 802, 970-972, 1003-1004, 1007, 1010, 1058
src/labthings_fastapi/client/__init__.py	62, 65-66, 114-115, 258-261, 368, 468
src/labthings_fastapi/exceptions.py	272, 279-280
src/labthings_fastapi/properties.py	494, 512, 609, 924, 928, 963-966, 1038, 1061, 1477
src/labthings_fastapi/server/__init__.py	259, 263, 319, 333-336
src/labthings_fastapi/utilities/__init__.py	159, 180, 194, 226, 234

rwb27 · 2026-05-12T16:23:43Z

This branch has fallen foul of separate input and output schemas in Pydantic. That seems to mean the OpenAPI docs are missing schemas for GET requests to properties.

Disabling schema-splitting fixes the problem, so I'll likely include that in this PR.

Action outputs are serialized using a pydantic model. We now create (and validate) this model instance in the action thread, rather than in the response handler returning it to the user. This means that validation errors will now be caught and logged, and there won't be loads of ASGI/routing/FastAPI stuff in the stack trace. It does not yet fix the problem of un-JSONable types being returned, because `pydantic` will allow `Any` to be wrapped in a `RootModel` which validates fine, and fails at serialisation time.

The stack trace is not useful when an action returns an invalid value, as it only tells us about LabThings code. The error is still logged, but there's no stack trace.

This commit: * Adds an error handler for PydanticSerializationError * Fixes a race condition in invocation responses that could result in an output being present before the response marks the invocation as complete * Handles HTTP errors when polling actions in ThingClient * Ensures that invocation models attach useful debug info to serialization errors (they add the invocation ID and action path).

Depending on whether the action fails before or after the initial response is sent to the client, we get a different exception (with the same message). This commit will accept either kind of failure so long as it includes the same message. This should prevent intermittent test failures: there's nothing to synchronise the action thread and the HTTP response, and it's not feasible to add that now.

This commit moves `wrap_plain_types_in_rootmodel` into the `RootModelWrapper` class as a class method, and adds better error handling. This now returns class and attribute references, or file and line number in the case of functions. Currently these errors are raised if unserializable types are declared. A future commit will raise similar errors at runtime if unserialisable values are found.

This commit moves both validation and serialisation of properties into LabThings code, so the exception may be handled properly. If this looks good, we may want to make a custom Response class.

This was causing properties to have no schema for their GET endpoint. It's detailed at https://fastapi.tiangolo.com/how-to/separate-openapi-schemas/#same-schema-for-input-and-output-models-in-docs

This commit removes the complicated wrapper-based error handling in favour of a function. This approach is specific (only catches ValidationError/SerializationError directly related to user code) and avoids our custom errors being re-wrapped by `pydantic`.

This makes a few changes to ensure we handle serialization errors better: 1. Invocations now only return their input, output, log, and links when accessed individually. This avoids the possibility of serialization errors in lists of invocations (and should give a decent performance boost). 2. Endpoints that return values from downstream code (property reads or action outputs) now return `Response` directly, and use `serialize_from_user_code` to apply sensible error handling. 3. Everywhere I've used `serialize_from_user_code` I've specified the model in the FastAPI decorator, and ensured the exception is handled, logged, and re-raised as an HTTPException. I've split up `response_from_user_code` to handle validation and serialization separately: that fits better with the way actions are structured.

These may be used by some clients to find the full model, so they're useful here.

This checks that we get the correct errors when accessing bad values over HTTP. Note that an invocation with an unserializable return value will raise an error if it is accessed individually, but won't cause the global endpoints to fail.

bprobert97 · 2026-05-20T13:21:51Z

+            # 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


Suggested change

# If there are multiple arguments, add this as a further one

# If there are multiple or no arguments, add this as a further one

bprobert97 · 2026-05-20T13:23:48Z

+
+
+class UnserializableTypeError(CausedByUserCodeError, TypeError):
+    r"""A type has been specified that can't be serialized to JSON.


Suggested change

r"""A type has been specified that can't be serialized to JSON.

r"""A type has been specified that can't be serialised to JSON.

Just to be consistent with the docstring for InvalidReturnValueError

bprobert97 · 2026-05-20T13:24:18Z

+    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


Suggested change

serialized to JSON. This might be an instance of a custom class, or another

serialised to JSON. This might be an instance of a custom class, or another

Just to be consistent with the docstring for InvalidReturnValueError

bprobert97 · 2026-05-20T13:25:23Z

+    def summary_model(self) -> InvocationSummary:
+        """Generate a summary of the invocation suitable for HTTP.
+
+        :return: a `InvocationSummary` representing this `Invocation`.


Suggested change

:return: a `InvocationSummary` representing this `Invocation`.

:return: an `InvocationSummary` representing this `Invocation`.

bprobert97 · 2026-05-20T13:27:32Z

+                output_model_instance = validate_from_user_code(
+                    model=action.output_model,
+                    value=ret,
+                    description=f"the output of '{self.thing.name}.{action.name}'",
+                    code=action.func,
+                )

+                with self._status_lock:
+                    self._output_model_instance = output_model_instance
+                    self._status = InvocationStatus.COMPLETED
+                    action.emit_changed_event(self.thing, self._status.value)


why has this been moved outside of the with action.context_for_func(thing): context?

bprobert97 · 2026-05-21T21:12:02Z



+class Unjsonable:
+    """A class that pydantic can't serialize."""


Suggested change

"""A class that pydantic can't serialize."""

"""A class that pydantic can't serialise."""

bprobert97 · 2026-05-21T21:12:11Z

+
+
+def test_bad_type():
+    """Test an obviously un-serializable type raises an error."""


Suggested change

"""Test an obviously un-serializable type raises an error."""

"""Test an obviously un-serialisable type raises an error."""

bprobert97 · 2026-05-21T21:13:16Z

+    class BrokenThing(lt.Thing):
+        broken: Unjsonable | None = lt.property(default=None)
+
+    with pytest.raises(UnserializableTypeError, match="BrokenThing.broken"):
+        _ = BrokenThing.properties["broken"].model


Can we add some comments explaining what's happening here in a bit more detail?

bprobert97 · 2026-05-21T21:14:01Z

+        ):
+            _ = tc.intprop
+
+        # The second property won't serialize


Suggested change

# The second property won't serialize

# The second property won't serialise

bprobert97 · 2026-05-21T21:15:48Z

+        assert response.status_code == 503  # There's no output as it failed.
+
+        # Here, the type hint is vague so it validates OK, but it can't
+        # serialize.


Suggested change

# serialize.

# serialise.

rwb27 added this to the v0.3.0 milestone May 7, 2026

rwb27 force-pushed the validate-return-values branch from 9c256a9 to e945edc Compare May 7, 2026 08:29

rwb27 force-pushed the validate-return-values branch from e945edc to 1a60cdd Compare May 7, 2026 11:09

rwb27 marked this pull request as ready for review May 14, 2026 12:14

rwb27 added 12 commits May 18, 2026 16:47

Improve error message and tidy logging

e155711

The stack trace is not useful when an action returns an invalid value, as it only tells us about LabThings code. The error is still logged, but there's no stack trace.

Take control of property serialisation

fa1bc94

This commit moves both validation and serialisation of properties into LabThings code, so the exception may be handled properly. If this looks good, we may want to make a custom Response class.

Disable FastAPI schema splitting

58ddb29

This was causing properties to have no schema for their GET endpoint. It's detailed at https://fastapi.tiangolo.com/how-to/separate-openapi-schemas/#same-schema-for-input-and-output-models-in-docs

Move links into summary invocation model.

c5c8704

These may be used by some clients to find the full model, so they're useful here.

Improve testing of invalid values via HTTP

05c757a

This checks that we get the correct errors when accessing bad values over HTTP. Note that an invocation with an unserializable return value will raise an error if it is accessed individually, but won't cause the global endpoints to fail.

Remove a no-longer-needed type: ignore

26852c6

rwb27 force-pushed the validate-return-values branch from 55f5a21 to 26852c6 Compare May 18, 2026 15:59

bprobert97 reviewed May 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better errors for bad return values#333

Better errors for bad return values#333
rwb27 wants to merge 12 commits into
mainfrom
validate-return-values

rwb27 commented May 6, 2026 •

edited

Loading

Uh oh!

barecheck Bot commented May 7, 2026 •

edited

Loading

Uh oh!

rwb27 commented May 12, 2026 •

edited

Loading

Uh oh!

bprobert97 May 20, 2026

Uh oh!

bprobert97 May 20, 2026

Uh oh!

bprobert97 May 20, 2026

Uh oh!

bprobert97 May 20, 2026

Uh oh!

bprobert97 May 20, 2026

Uh oh!

bprobert97 May 21, 2026

Uh oh!

bprobert97 May 21, 2026

Uh oh!

bprobert97 May 21, 2026

Uh oh!

bprobert97 May 21, 2026

Uh oh!

bprobert97 May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	# If there are multiple arguments, add this as a further one
	# If there are multiple or no arguments, add this as a further one



		class UnserializableTypeError(CausedByUserCodeError, TypeError):
		r"""A type has been specified that can't be serialized to JSON.

	serialized to JSON. This might be an instance of a custom class, or another
	serialised to JSON. This might be an instance of a custom class, or another

	:return: a `InvocationSummary` representing this `Invocation`.
	:return: an `InvocationSummary` representing this `Invocation`.



		class Unjsonable:
		"""A class that pydantic can't serialize."""

	"""A class that pydantic can't serialize."""
	"""A class that pydantic can't serialise."""



		def test_bad_type():
		"""Test an obviously un-serializable type raises an error."""

	"""Test an obviously un-serializable type raises an error."""
	"""Test an obviously un-serialisable type raises an error."""

	# The second property won't serialize
	# The second property won't serialise

Conversation

rwb27 commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

barecheck Bot commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Barecheck - Code coverage report

Uh oh!

rwb27 commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rwb27 commented May 6, 2026 •

edited

Loading

barecheck Bot commented May 7, 2026 •

edited

Loading

rwb27 commented May 12, 2026 •

edited

Loading