Merge pull request #175 from labthings/better-observeproperty-errors

Raise better errors in observe_property, and test them.

Author: rwb27

src/labthings_fastapi/exceptions.py

@@ -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.
+    """

src/labthings_fastapi/thing.py

@@ -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:
@@ -347,10 +348,13 @@ def observe_property(self, property_name: str, stream: ObjectSendStream) -> None
         :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) -> None:
@@ -361,7 +365,7 @@ def observe_action(self, action_name: str, stream: ObjectSendStream) -> None:
 
         :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)

src/labthings_fastapi/websockets.py

@@ -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}.")
+    return {
+        "messageType": "response",
+        "operation": f"observe{affordance_type}",
+        "name": name,
+        "error": error,
+    }
+
+
 async def relay_notifications_to_websocket(
     websocket: WebSocket, receive_stream: ObjectReceiveStream
 ) -> None:
@@ -66,17 +111,23 @@ async def process_messages_from_websocket(
     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: