improve error messages when running plan without parameters

src/blueapi/cli/cli.py

@@ -7,6 +7,7 @@
 from typing import Optional, Tuple, Union
 
 import click
+from pydantic import ValidationError, parse_obj_as
 from requests.exceptions import ConnectionError
 
 from blueapi import __version__
@@ -16,7 +17,7 @@
 from blueapi.messaging import MessageContext
 from blueapi.messaging.stomptemplate import StompMessagingTemplate
 from blueapi.service.main import start
-from blueapi.service.model import WorkerTask
+from blueapi.service.model import PlanModel, WorkerTask
 from blueapi.service.openapi import (
     DOCS_SCHEMA_LOCATION,
     generate_schema,
@@ -182,6 +183,29 @@ def store_finished_event(event: WorkerEvent) -> None:
             finished_event.append(event)
 
     parameters = parameters or "{}"
+    schema: PlanModel = client.get_plan(name)
+    progress_tracking = f"Trying to run plan: {name}."
+    print(progress_tracking)
+    try:
+        text = "Checking supplied parameters against expected parameters..."
+        print(text)
+        validated_data = parse_obj_as(type(schema.parameter_schema), parameters)
+        print("Plan params validation successful:", validated_data)
+    except ValidationError as e:
+        errors = e.errors()
+        formatted_errors = "; ".join(
+            [f"{err['loc'][0]}: {err['msg']}" for err in errors]
+        )
+
+        print(f"Input validation failed: {formatted_errors}")
+        # Handle the case where the parameters are invalid according to the PlanModel
+        expected_params = schema.parameter_schema.get("properties")
+        print(
+            f"""failed to run the {name} plan, supplied params {parameters}
+            do not match the expected params: {expected_params}"""
+        )
+        return
+
     task = Task(name=name, params=json.loads(parameters))
 
     resp = client.create_task(task)

src/blueapi/cli/rest.py

@@ -1,3 +1,4 @@
+from http import HTTPStatus
 from typing import Any, Callable, Literal, Mapping, Optional, Type, TypeVar
 
 import requests
@@ -23,6 +24,15 @@ def _is_exception(response: requests.Response) -> bool:
     return response.status_code >= 400
 
 
+def get_status_message(code: int) -> str:
+    """Returns the standard description for a given HTTP status code."""
+    try:
+        message = HTTPStatus(code).phrase
+        return message
+    except ValueError:
+        return "Unknown Status Code"
+
+
 class BlueapiRestClient:
     _config: RestConfig
 
@@ -104,9 +114,17 @@ def _request_and_deserialize(
         raise_if: Callable[[requests.Response], bool] = _is_exception,
     ) -> T:
         url = self._url(suffix)
+        # todo consider changing this to a switch statement for easier mocking
+        # alternatively do single responsiblity principle
+        # and decoule request from deserializing.
         response = requests.request(method, url, json=data)
         if raise_if(response):
-            raise BlueskyRemoteError(str(response))
+            message = get_status_message(response.status_code)
+            t = response.text
+            error_message = f"""Response failed with text: {t},
+            with error code: {response.status_code}
+            which corresponds to {message}"""
+            raise BlueskyRemoteError(error_message)
         deserialized = parse_obj_as(target_type, response.json())
         return deserialized
 

tests/test_cli.py

@@ -150,22 +150,76 @@ def test_config_passed_down_to_command_children(
 
     with patch("uvicorn.run", side_effect=None):
         result = runner.invoke(main, ["-c", config_path, "serve"])
+        assert result.exit_code == 0
+
+    with patch("requests.get") as mock_get, patch("requests.post") as mock_post:
+        mock_requests.return_value = Mock()
+        mock_get.return_value.json.return_value = {"time": 5}
+
+        runner.invoke(
+            main, ["-c", config_path, "controller", "run", "sleep", '{"time": 5}']
+        )
+        assert result.exit_code == 0
+
+        # expect the first call to be to the helper UI
+        assert mock_requests.call_args[0] == (
+            "GET",
+            "http://a.fake.host:12345/plans/sleep",
+        )
+
+        assert mock_requests.call_args[1] == (
+            "POST",
+            "http://a.fake.host:12345/tasks",
+        )
+        mock_post.return_value = Mock(status_code=200)  # Mock a successful POST request
+        assert mock_requests.call_args[2] == {
+            "json": {
+                "name": "sleep",
+                "params": {"time": 5},
+            }
+        }
 
-    assert result.exit_code == 0
-
-    mock_requests.return_value = Mock()
 
-    runner.invoke(
-        main, ["-c", config_path, "controller", "run", "sleep", '{"time": 5}']
-    )
+@pytest.mark.handler
+@patch("blueapi.service.handler.Handler")
+def test_config_passed_down_to_command_children_2(
+    mock_handler: Mock,
+    handler,  # This seems to be provided; ensure it's correctly instantiated
+    runner: CliRunner,  # Ensure runner is correctly instantiated before the test
+):
+    mock_handler.side_effect = Mock(return_value=handler)
+    config_path = "tests/example_yaml/rest_config.yaml"
 
-    assert mock_requests.call_args[0] == (
-        "POST",
-        "http://a.fake.host:12345/tasks",
-    )
-    assert mock_requests.call_args[1] == {
-        "json": {
-            "name": "sleep",
-            "params": {"time": 5},
-        }
-    }
+    with patch("uvicorn.run", side_effect=None):
+        result = runner.invoke(main, ["-c", config_path, "serve"])
+        assert result.exit_code == 0
+
+    # Mocking `requests.get` and `requests.post` separately
+    with patch("requests.request") as mock_get, patch("requests.post") as mock_post:
+        # Mock the GET response
+        mock_get.return_value = Mock()
+        mock_get.return_value.json.return_value = {"time": 5}
+
+        # Mock the POST response
+        mock_post.return_value = Mock(status_code=200)  # Mock a successful POST request
+
+        # Invoke the command that triggers the HTTP requests
+        result = runner.invoke(
+            main, ["-c", config_path, "controller", "run", "sleep", '{"time": 5}']
+        )
+        assert result.exit_code == 0
+
+        # Check that the correct GET request was made
+        mock_get.assert_called_once_with("http://a.fake.host:12345/plans/sleep")
+
+        # If you're sending a POST request in the process that should be captured here
+        # This part depends on how your `main` function
+        # and its subcommands handle the POST request
+        # You might need to adjust the assertion to match your application's behavior
+        mock_post.assert_called_once_with(
+            "http://a.fake.host:12345/tasks",
+            json={
+                "name": "sleep",
+                "params": {"time": 5},
+            },
+        )