Merge pull request #236 from labthings/ThingClient-exceptions

Thing client exceptions

Author: rwb27

src/labthings_fastapi/client/__init__.py

@@ -16,6 +16,11 @@
 from pydantic import BaseModel
 
 from .outputs import ClientBlobOutput
+from ..exceptions import (
+    FailedToInvokeActionError,
+    ServerActionError,
+    ClientPropertyError,
+)
 
 __all__ = ["ThingClient", "poll_invocation"]
 ACTION_RUNNING_KEYWORDS = ["idle", "pending", "running"]
@@ -143,10 +148,17 @@ def get_property(self, path: str) -> Any:
             to the ``base_url``.
 
         :return: the property's value, as deserialised from JSON.
+        :raise ClientPropertyError: is raised the property cannot be read.
         """
-        r = self.client.get(urljoin(self.path, path))
-        r.raise_for_status()
-        return r.json()
+        response = self.client.get(urljoin(self.path, path))
+        if response.is_error:
+            detail = response.json().get("detail")
+            err_msg = "Unknown error"
+            if isinstance(detail, str):
+                err_msg = detail
+            raise ClientPropertyError(f"Failed to get property {path}: {err_msg}")
+
+        return response.json()
 
     def set_property(self, path: str, value: Any) -> None:
         """Make a PUT request to set the value of a property.
@@ -155,9 +167,20 @@ def set_property(self, path: str, value: Any) -> None:
             to the ``base_url``.
         :param value: the property's value. Currently this must be
             serialisable to JSON.
+        :raise ClientPropertyError: is raised the property cannot be set.
         """
-        r = self.client.put(urljoin(self.path, path), json=value)
-        r.raise_for_status()
+        response = self.client.put(urljoin(self.path, path), json=value)
+        if response.is_error:
+            detail = response.json().get("detail")
+            err_msg = "Unknown error"
+            if isinstance(detail, str):
+                err_msg = detail
+            elif (
+                isinstance(detail, list) and len(detail) and isinstance(detail[0], dict)
+            ):
+                err_msg = detail[0].get("msg", "Unknown error")
+
+            raise ClientPropertyError(f"Failed to get property {path}: {err_msg}")
 
     def invoke_action(self, path: str, **kwargs: Any) -> Any:
         r"""Invoke an action on the Thing.
@@ -177,7 +200,9 @@ def invoke_action(self, path: str, **kwargs: Any) -> Any:
 
         :return: the output value of the action.
 
-        :raise RuntimeError: is raised if the action does not complete successfully.
+        :raise FailedToInvokeActionError: if the action fails to start.
+        :raise ServerActionError: is raised if the action does not complete
+            successfully.
         """
         for k in kwargs.keys():
             value = kwargs[k]
@@ -191,9 +216,12 @@ def invoke_action(self, path: str, **kwargs: Any) -> Any:
                 # Note that the blob will not be uploaded: we rely on the blob
                 # still existing on the server.
                 kwargs[k] = {"href": value.href, "media_type": value.media_type}
-        r = self.client.post(urljoin(self.path, path), json=kwargs)
-        r.raise_for_status()
-        invocation = poll_invocation(self.client, r.json())
+        response = self.client.post(urljoin(self.path, path), json=kwargs)
+        if response.is_error:
+            message = _construct_failed_to_invoke_message(path, response)
+            raise FailedToInvokeActionError(message)
+
+        invocation = poll_invocation(self.client, response.json())
         if invocation["status"] == "completed":
             if (
                 isinstance(invocation["output"], Mapping)
@@ -206,8 +234,8 @@ def invoke_action(self, path: str, **kwargs: Any) -> Any:
                     client=self.client,
                 )
             return invocation["output"]
-        else:
-            raise RuntimeError(f"Action did not complete successfully: {invocation}")
+        message = _construct_invocation_error_message(invocation)
+        raise ServerActionError(message)
 
     def follow_link(self, response: dict, rel: str) -> httpx.Response:
         """Follow a link in a response object, by its `rel` attribute.
@@ -398,3 +426,52 @@ def add_property(cls: type[ThingClient], property_name: str, property: dict) ->
             readable=not property.get("writeOnly", False),
         ),
     )
+
+
+def _construct_failed_to_invoke_message(path: str, response: httpx.Response) -> str:
+    """Format an error for ThingClient to raise if an invocation fails to start.
+
+    :param path: The path of the action
+    :param response: The response object from the POST request to start the action.
+    :return: The message for the raised error
+    """
+    # Default message if we can't process return
+    message = f"Unknown error when invoking action {path}"
+    details = response.json().get("detail", [])
+
+    if isinstance(details, str):
+        message = f"Error when invoking action {path}: {details}"
+    if isinstance(details, list) and len(details) and isinstance(details[0], dict):
+        loc = details[0].get("loc", [])
+        loc_str = "" if len(loc) < 2 else f"'{loc[1]}' - "
+        err_msg = details[0].get("msg", "Unknown Error")
+        message = f"Error when invoking action {path}: {loc_str}{err_msg}"
+    return message
+
+
+def _construct_invocation_error_message(invocation: Mapping[str, Any]) -> str:
+    """Format an error for ThingClient to raise if an invocation ends in and error.
+
+    :param invocation: The invocation dictionary returned.
+    :return: The message for the raised error
+    """
+    inv_id = invocation["id"]
+    action_name = invocation["action"].split("/")[-1]
+
+    err_message = "Unknown error"
+
+    if len(invocation.get("log", [])) > 0:
+        last_log = invocation["log"][-1]
+        err_message = last_log.get("message", err_message)
+
+        exception_type = last_log.get("exception_type")
+        if exception_type is not None:
+            err_message = f"[{exception_type}]: {err_message}"
+
+        traceback = last_log.get("traceback")
+        if traceback is not None:
+            err_message += "\n\nSERVER TRACEBACK START:\n\n"
+            err_message += traceback
+            err_message += "\n\nSERVER TRACEBACK END\n\n"
+
+    return f"Action {action_name} (ID: {inv_id}) failed with error:\n{err_message}"

src/labthings_fastapi/exceptions.py

@@ -156,3 +156,25 @@ class UnsupportedConstraintError(ValueError):
     supported arguments. Their meaning is described in the `pydantic.Field`
     documentation.
     """
+
+
+class FailedToInvokeActionError(RuntimeError):
+    """The action could not be started.
+
+    This error is raised by a `.ThingClient` instance if an action could not be started.
+    It most commonly occurs because the input to the action could not be converted
+    to the required type: the error message should give more detail on what's wrong.
+    """
+
+
+class ServerActionError(RuntimeError):
+    """The action ended with an error on the server.
+
+    This error is raised by a `ThingClient` when an action is successfully invoked on
+    the server, but does not complete. The error message should include more information
+    on why this happened.
+    """
+
+
+class ClientPropertyError(RuntimeError):
+    """Setting or getting a property via a ThingClient failed."""

src/labthings_fastapi/invocations.py

@@ -7,6 +7,7 @@
 from datetime import datetime
 from enum import Enum
 import logging
+import traceback
 from typing import Optional, Any, Sequence, TypeVar, Generic
 import uuid
 
@@ -42,6 +43,10 @@ class LogRecordModel(BaseModel):
     filename: str
     created: datetime
 
+    # Optional exception info
+    exception_type: Optional[str] = None
+    traceback: Optional[str] = None
+
     @model_validator(mode="before")
     @classmethod
     def generate_message(cls, data: Any) -> Any:
@@ -62,6 +67,11 @@ def generate_message(cls, data: Any) -> Any:
                     # the invocation.
                     # This way, you can find and fix the source.
                     data.message = f"Error constructing message ({e}) from {data!r}."
+
+        if data.exc_info:
+            data.exception_type = data.exc_info[0].__name__
+            data.traceback = "\n".join(traceback.format_exception(*data.exc_info))
+
         return data
 
 

tests/test_blob_output.py

@@ -7,7 +7,6 @@
 from uuid import uuid4
 
 from fastapi.testclient import TestClient
-from httpx import HTTPStatusError
 from pydantic_core import PydanticSerializationError
 import pytest
 import labthings_fastapi as lt
@@ -229,7 +228,9 @@ def test_blob_input(client):
     bad_blob = ClientBlobOutput(
         media_type="text/plain", href="http://nonexistent.local/totally_bogus"
     )
-    with pytest.raises(HTTPStatusError, match="404 Not Found"):
+
+    msg = "Error when invoking action passthrough_blob: Could not find blob ID in href"
+    with pytest.raises(lt.exceptions.FailedToInvokeActionError, match=msg):
         tc.passthrough_blob(blob=bad_blob)
 
     # Check that the same thing works on the server side

tests/test_thing_client.py

@@ -0,0 +1,176 @@
+"""Test that Thing Client's can call actions and read properties."""
+
+import re
+
+import pytest
+import labthings_fastapi as lt
+from fastapi.testclient import TestClient
+
+
+class ThingToTest(lt.Thing):
+    """A thing to be tested by using a ThingClient."""
+
+    int_prop: int = lt.property(default=1)
+    float_prop: float = lt.property(default=0.1)
+    str_prop: str = lt.property(default="foo")
+
+    int_prop_read_only: int = lt.property(default=1, readonly=True)
+    float_prop_read_only: float = lt.property(default=0.1, readonly=True)
+    str_prop_read_only: str = lt.property(default="foo", readonly=True)
+
+    @lt.action
+    def increment(self) -> None:
+        """Increment the counter.
+
+        An action with no arguments or return.
+        """
+        self.int_prop += 1
+
+    @lt.action
+    def increment_and_return(self) -> int:
+        """Increment the counter and return value.
+
+        An action with no arguments, but with a return value
+        """
+        self.int_prop += 1
+        return self.int_prop
+
+    @lt.action
+    def increment_by_input(self, value: int) -> None:
+        """Increment the counter by input value.
+
+        An action with an argument but no return.
+        """
+        self.int_prop += value
+
+    @lt.action
+    def increment_by_input_and_return(self, value: int) -> int:
+        """Increment the counter by input value and return the new value.
+
+        An action with and argument and a return value.
+        """
+        self.int_prop += value
+        return self.int_prop
+
+    @lt.action
+    def throw_value_error(self) -> None:
+        """Throw a value error."""
+        raise ValueError("This never works!")
+
+
+@pytest.fixture
+def thing_client():
+    """Yield a test client connected to a ThingServer."""
+    server = lt.ThingServer({"test_thing": ThingToTest})
+    with TestClient(server.app) as client:
+        yield lt.ThingClient.from_url("/test_thing/", client=client)
+
+
+def test_reading_and_setting_properties(thing_client):
+    """Test reading and setting properties."""
+    assert thing_client.int_prop == 1
+    assert thing_client.float_prop == 0.1
+    assert thing_client.str_prop == "foo"
+
+    thing_client.int_prop = 2
+    thing_client.float_prop = 0.2
+    thing_client.str_prop = "foo2"
+
+    assert thing_client.int_prop == 2
+    assert thing_client.float_prop == 0.2
+    assert thing_client.str_prop == "foo2"
+
+    # Set a property that doesn't exist.
+    err = "Failed to get property foobar: Not Found"
+    with pytest.raises(lt.exceptions.ClientPropertyError, match=err):
+        thing_client.get_property("foobar")
+
+    # Set a property with bad data type.
+    err = (
+        "Failed to get property int_prop: Input should be a valid integer, unable to "
+        "parse string as an integer"
+    )
+    with pytest.raises(lt.exceptions.ClientPropertyError, match=err):
+        thing_client.int_prop = "Bad value!"
+
+
+def test_reading_and_not_setting_read_only_properties(thing_client):
+    """Test reading read_only properties, but failing to set."""
+    assert thing_client.int_prop_read_only == 1
+    assert thing_client.float_prop_read_only == 0.1
+    assert thing_client.str_prop_read_only == "foo"
+
+    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+        thing_client.int_prop_read_only = 2
+    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+        thing_client.float_prop_read_only = 0.2
+    with pytest.raises(lt.exceptions.ClientPropertyError, match="Method Not Allowed"):
+        thing_client.str_prop_read_only = "foo2"
+
+    assert thing_client.int_prop_read_only == 1
+    assert thing_client.float_prop_read_only == 0.1
+    assert thing_client.str_prop_read_only == "foo"
+
+
+def test_call_action(thing_client):
+    """Test calling an action."""
+    assert thing_client.int_prop == 1
+    thing_client.increment()
+    assert thing_client.int_prop == 2
+
+
+def test_call_action_with_return(thing_client):
+    """Test calling an action with a return."""
+    assert thing_client.int_prop == 1
+    new_value = thing_client.increment_and_return()
+    assert new_value == 2
+    assert thing_client.int_prop == 2
+
+
+def test_call_action_with_args(thing_client):
+    """Test calling an action."""
+    assert thing_client.int_prop == 1
+    thing_client.increment_by_input(value=5)
+    assert thing_client.int_prop == 6
+
+
+def test_call_action_with_args_and_return(thing_client):
+    """Test calling an action with a return."""
+    assert thing_client.int_prop == 1
+    new_value = thing_client.increment_by_input_and_return(value=5)
+    assert new_value == 6
+    assert thing_client.int_prop == 6
+
+
+def test_call_action_wrong_arg(thing_client):
+    """Test calling an action with wrong argument."""
+    err = "Error when invoking action increment_by_input: 'value' - Field required"
+
+    with pytest.raises(lt.exceptions.FailedToInvokeActionError, match=err):
+        thing_client.increment_by_input(input=5)
+
+
+def test_call_action_wrong_type(thing_client):
+    """Test calling an action with wrong argument."""
+    err = (
+        "Error when invoking action increment_by_input: 'value' - Input should be a "
+        "valid integer, unable to parse string as an integer"
+    )
+    with pytest.raises(lt.exceptions.FailedToInvokeActionError, match=err):
+        thing_client.increment_by_input(value="foo")
+
+
+def test_call_that_errors(thing_client):
+    """Test calling an action with wrong argument."""
+    regex = r"Action throw_value_error \(ID: [0-9a-f\-]*\) failed with error:"
+    with pytest.raises(lt.exceptions.ServerActionError, match=regex) as exc_info:
+        thing_client.throw_value_error()
+
+    full_message = str(exc_info.value)
+    assert "[ValueError]: This never works!" in full_message
+    assert "SERVER TRACEBACK START:" in full_message
+    assert "SERVER TRACEBACK END" in full_message
+    assert re.search(
+        r'File ".*test_thing_client\.py", line \d+, in throw_value_error',
+        full_message,
+    )