diff --git a/src/labthings_fastapi/client/__init__.py b/src/labthings_fastapi/client/__init__.py index 07fdb74..1b45d4c 100644 --- a/src/labthings_fastapi/client/__init__.py +++ b/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}" diff --git a/src/labthings_fastapi/exceptions.py b/src/labthings_fastapi/exceptions.py index a21246c..42a1709 100644 --- a/src/labthings_fastapi/exceptions.py +++ b/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.""" diff --git a/src/labthings_fastapi/invocations.py b/src/labthings_fastapi/invocations.py index 33cda73..7ed10e9 100644 --- a/src/labthings_fastapi/invocations.py +++ b/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 diff --git a/tests/test_blob_output.py b/tests/test_blob_output.py index 1bb4ddf..8a15562 100644 --- a/tests/test_blob_output.py +++ b/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 diff --git a/tests/test_thing_client.py b/tests/test_thing_client.py new file mode 100644 index 0000000..518907b --- /dev/null +++ b/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, + )