Skip to content

Raise better errors in observe_property, and test them. #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/labthings_fastapi/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,13 @@ class ReadOnlyPropertyError(AttributeError):
No setter has been defined for this `.FunctionalProperty`, so
it may not be written to.
"""


class PropertyNotObservableError(RuntimeError):
"""The property is not observable.

This exception is raised when `.Thing.observe_property` is called with a
property that is not observable. Currently, only data properties are
observable: functional properties (using a getter/setter) may not be
observed.
"""
12 changes: 8 additions & 4 deletions src/labthings_fastapi/thing.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@

from pydantic import BaseModel

from .properties import DataProperty, BaseSetting
from .properties import BaseProperty, DataProperty, BaseSetting
from .descriptors import ActionDescriptor
from .thing_description._model import ThingDescription, NoSecurityScheme
from .utilities import class_attributes
from .thing_description import validation
from .utilities.introspection import get_summary, get_docstring
from .websockets import websocket_endpoint
from .exceptions import PropertyNotObservableError


if TYPE_CHECKING:
Expand Down Expand Up @@ -241,7 +242,7 @@
for name in self._settings.keys():
value = getattr(self, name)
if isinstance(value, BaseModel):
value = value.model_dump()

Check warning on line 245 in src/labthings_fastapi/thing.py

View workflow job for this annotation

GitHub Actions / coverage

245 line is not covered with tests
setting_dict[name] = value
# Dumpy to string before writing so if this fails the file isn't overwritten
setting_json = json.dumps(setting_dict, indent=4)
Expand Down Expand Up @@ -299,7 +300,7 @@
and self._cached_thing_description[0] == path
and self._cached_thing_description[1] == base
):
return self._cached_thing_description[2]

Check warning on line 303 in src/labthings_fastapi/thing.py

View workflow job for this annotation

GitHub Actions / coverage

303 line is not covered with tests

properties = {}
actions = {}
Expand Down Expand Up @@ -346,10 +347,13 @@
:param stream: the stream used to send events.

:raise KeyError: if the requested name is not defined on this Thing.
:raise PropertyNotObservableError: if the property is not observable.
"""
prop = getattr(self.__class__, property_name)
if not isinstance(prop, DataProperty):
prop = getattr(self.__class__, property_name, None)
if not isinstance(prop, BaseProperty):
raise KeyError(f"{property_name} is not a LabThings Property")
if not isinstance(prop, DataProperty):
raise PropertyNotObservableError(f"{property_name} is not observable.")
prop._observers_set(self).add(stream)

def observe_action(self, action_name: str, stream: ObjectSendStream):
Expand All @@ -360,7 +364,7 @@

:raise KeyError: if the requested name is not defined on this Thing.
"""
action = getattr(self.__class__, action_name)
action = getattr(self.__class__, action_name, None)
if not isinstance(action, ActionDescriptor):
raise KeyError(f"{action_name} is not an LabThings Action")
observers = action._observers_set(self)
Expand Down
67 changes: 59 additions & 8 deletions src/labthings_fastapi/websockets.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,57 @@
import logging
from fastapi import WebSocket, WebSocketDisconnect
from fastapi.encoders import jsonable_encoder
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Literal
from .exceptions import PropertyNotObservableError

if TYPE_CHECKING:
from .thing import Thing


WEBTHING_ERROR_URL = "https://w3c.github.io/web-thing-protocol/errors"


def observation_error_response(
name: str, affordance_type: Literal["action", "property"], exception: Exception
) -> dict[str, str | dict]:
r"""Generate a websocket error response for observing an action or property.

When a websocket client asks to observe a property or action that either
doesn't exist or isn't observable, this function makes a dictionary that
can be returned to the client indicating an error.

:param name: The name of the affordance being observed.
:param affordance_type: The type of the affordance.
:param exception: The error that was raised.
:returns: A dictionary that may be returned to the websocket.

:raises TypeError: if the exception is not a `KeyError`
or `.PropertyNotObservableError`\ .
"""
if isinstance(exception, KeyError):
error = {
"status": "404",
"type": f"{WEBTHING_ERROR_URL}#not-found",
"title": "Not Found",
"detail": f"No {affordance_type} found with the name '{name}'.",
}
elif isinstance(exception, PropertyNotObservableError):
error = {
"status": "403",
"type": f"{WEBTHING_ERROR_URL}#not-observable",
"title": "Not Observable",
"detail": f"Property '{name}' is not observable.",
}
else:
raise TypeError(f"Can't generate an error response for {exception}.")

Check warning on line 69 in src/labthings_fastapi/websockets.py

View workflow job for this annotation

GitHub Actions / coverage

69 line is not covered with tests
return {
"messageType": "response",
"operation": f"observe{affordance_type}",
"name": name,
"error": error,
}


async def relay_notifications_to_websocket(
websocket: WebSocket, receive_stream: ObjectReceiveStream
) -> None:
Expand Down Expand Up @@ -66,17 +111,23 @@
while True:
try:
data = await websocket.receive_json()
if data["messageType"] == "addPropertyObservation":
except WebSocketDisconnect:
await send_stream.aclose()
return
if data["messageType"] == "addPropertyObservation":
try:
for k in data["data"].keys():
thing.observe_property(k, send_stream)
if data["messageType"] == "addActionObservation":
except (KeyError, PropertyNotObservableError) as e:
logging.error(f"Got a bad websocket message: {data}, caused {e!r}.")
await send_stream.send(observation_error_response(k, "property", e))
if data["messageType"] == "addActionObservation":
try:
for k in data["data"].keys():
thing.observe_action(k, send_stream)
except KeyError as e:
logging.error(f"Got a bad websocket message: {data}, caused KeyError({e})")
except WebSocketDisconnect:
await send_stream.aclose()
return
except KeyError as e:
logging.error(f"Got a bad websocket message: {data}, caused {e!r}.")
await send_stream.send(observation_error_response(k, "action", e))


async def websocket_endpoint(thing: Thing, websocket: WebSocket) -> None:
Expand Down
Loading