Fix/12 human readable errors

README.md

@@ -2,6 +2,10 @@
 
 Implementation of the FarmBot at UAS Technikum Wien.
 
+[![Pylint](https://github.com/TW-Robotics/TWFarmBot/actions/workflows/pylint.yml/badge.svg)](https://github.com/TW-Robotics/TWFarmBot/actions/workflows/pylint.yml)
+[![Python 3.10](https://img.shields.io/badge/python-3.10-blue.svg)](https://www.python.org/downloads/release/python-3100/)
+[![Issues](https://img.shields.io/github/issues/TW-Robotics/TWFarmBot)](https://github.com/TW-Robotics/TWFarmBot/issues)
+
 This repository is a **monorepo** for the FarmBot system: the physical robot
 integration, sensor pipelines, irrigation/vision/planning services, the API and
 worker apps, and the student projects and experiments that build on top of

apps/api_server/src/twfarmbot_api_server/app.py

@@ -158,6 +158,19 @@ def post_action(payload: ActionPayload, wait: bool = True) -> dict[str, Any]:
             raise HTTPException(status_code=404, detail=str(err)) from err
         except UnsafeActionError as err:
             raise HTTPException(status_code=400, detail=str(err)) from err
+        except FarmBotConnectionError as err:
+            raise HTTPException(
+                status_code=502,
+                detail=f"FarmBot not connected: {err}",
+            ) from err
+        except Exception as err:  # noqa: BLE001 — surface real cause to the UI
+            log.exception(
+                "action failed kind=%s params=%s", action.kind, action.params
+            )
+            raise HTTPException(
+                status_code=500,
+                detail=f"{type(err).__name__}: {err}",
+            ) from err
         return {
             "status": "ok",
             "action": {"kind": executed.kind, "params": executed.params},

apps/ui/src/twfarmbot_ui/app.py

@@ -25,7 +25,7 @@
 
 from twfarmbot_core.actions import summarize_action
 
-from twfarmbot_ui.client import ApiClient
+from twfarmbot_ui.client import ApiClient, ApiResult
 
 # ── config ────────────────────────────────────────────────────────────────────
 
@@ -336,7 +336,7 @@ def _do_move(client: ApiClient, x: float, y: float, z: float, label: str = "") -
         st.toast(msg, icon="➡️")
         _refresh_position(client)
     else:
-        st.error(f"HTTP {r.code}: {r.body}")
+        st.error(r.error_message())
 
 
 def _do_pin_write(
@@ -353,7 +353,7 @@ def _do_pin_write(
     if r.ok:
         st.toast(f"pin {pin} = {value}", icon="✏️")
     else:
-        st.error(f"HTTP {r.code}: {r.body}")
+        st.error(r.error_message())
 
 
 def _do_pin_pulse(
@@ -370,7 +370,7 @@ def _do_pin_pulse(
     if r.ok:
         st.toast(f"pin {pin} HIGH for {seconds}s", icon="✏️")
     else:
-        st.error(f"HTTP {r.code}: {r.body}")
+        st.error(r.error_message())
 
 
 # ── page shell ────────────────────────────────────────────────────────────────
@@ -579,7 +579,7 @@ def _do_pin_pulse(
         if r.ok:
             st.toast("ESTOP sent", icon="🛑")
         else:
-            st.error(str(r.body))
+            st.error(r.error_message())
 
 # ── tab content ───────────────────────────────────────────────────────────────
 
@@ -763,7 +763,7 @@ def _render_garden() -> None:
 
     result = client.request("GET", "/garden")
     if not result.ok or not isinstance(result.body, dict):
-        st.error(f"Garden model unavailable: {result.body}")
+        st.error(f"Garden model unavailable: {result.error_message()}")
         return
 
     world = result.body
@@ -1065,7 +1065,7 @@ def _render_motion() -> None:
         if r.ok:
             st.toast("Homing queued")
         else:
-            st.error(str(r.body))
+            st.error(r.error_message())
 
     # Presets
     if "presets" not in st.session_state:
@@ -1135,7 +1135,7 @@ def _render_io() -> None:
                 if r.ok:
                     st.success("Queued")
                 else:
-                    st.error(str(r.body))
+                    st.error(r.error_message())
 
     with b:
         st.markdown("**Peripheral control**")
@@ -1190,7 +1190,7 @@ def _render_camera() -> None:
         if r.ok:
             st.toast("Capture queued", icon="📷")
         else:
-            st.error(str(r.body))
+            st.error(r.error_message())
     if refresh.button("↻ Refresh gallery", use_container_width=True):
         st.session_state["images"] = client.request(
             "GET", "/images", params={"refresh": "true"}, timeout=10.0
@@ -1694,7 +1694,7 @@ def _close_text_segment() -> None:
                             seg[1] = accumulated
                             seg[0].markdown(accumulated)
                     else:
-                        stream_error = f"Fallback failed: HTTP {r.code}: {r.body}"
+                        stream_error = f"Fallback failed: {r.error_message()}"
                 except Exception as exc:  # noqa: BLE001
                     stream_error = f"Fallback failed: {type(exc).__name__}: {exc}"
 
@@ -1906,7 +1906,11 @@ def _render_plan() -> None:
         st.json(response)
 
     if status and status >= 400:
-        st.error(f"Planner error (HTTP {status}): {response.get('error', response)}")
+        err_body = response.get("error", response)
+        st.error(
+            f"Planner error (HTTP {status}): "
+            f"{ApiResult(ok=False, code=status, body=err_body).error_message()}"
+        )
         return
 
     actions = response.get("actions", []) or []
@@ -1946,7 +1950,7 @@ def _render_plan() -> None:
                 st.toast(f"Queued {action['kind']}", icon="➡️")
             else:
                 failed += 1
-                st.error(f"Failed to queue {action['kind']}: {r.body}")
+                st.error(f"Failed to queue {action['kind']}: {r.error_message()}")
         if failed == 0:
             st.success(f"Plan queued · {queued} action(s)")
         else:
@@ -1968,7 +1972,7 @@ def _render_diagnostics() -> None:
         if d.ok and isinstance(d.body, dict):
             st.session_state["diag"] = d.body.get("state", {})
         else:
-            st.error(f"Read failed: {d.body}")
+            st.error(f"Read failed: {d.error_message()}")
 
     payload = st.session_state.get("diag", {}) or {}
     info = payload.get("informational_settings", {}) or {}

apps/ui/src/twfarmbot_ui/client.py

@@ -19,6 +19,22 @@ class ApiResult:
     code: int
     body: Any
 
+    def error_message(self) -> str:
+        """Return a human-readable backend error from the response body.
+
+        FastAPI errors are shaped like ``{"detail": "..."}``; legacy or
+        connection-failure bodies may use ``{"error": "..."}``. Fall back to
+        the raw body text when neither key is present.
+        """
+        if isinstance(self.body, dict):
+            if "detail" in self.body:
+                return str(self.body["detail"])
+            if "error" in self.body:
+                return str(self.body["error"])
+        if isinstance(self.body, str):
+            return self.body
+        return str(self.body)
+
 
 class ApiClient:
     def __init__(self, base_url: str, timeout: float = 2.0) -> None:

services/planning_service/planning_service/providers.py

@@ -9,6 +9,7 @@
 
 import os
 from abc import ABC, abstractmethod
+from typing import Any
 
 import requests
 from langchain_core.language_models import BaseChatModel
@@ -27,7 +28,7 @@ def build_chat_model(
         self, model: str, config: PlannerConfig
     ) -> BaseChatModel:
         """Return a configured LangChain chat model for ``model``."""
-        raise NotImplementedError
+        ...
 
     def list_models(self, config: PlannerConfig) -> list[str]:
         """Return a list of model ids available from this provider."""

tests/test_actions_endpoint.py

@@ -0,0 +1,81 @@
+"""Tests for the POST /actions endpoint."""
+
+from __future__ import annotations
+
+import pytest
+from fastapi.testclient import TestClient
+
+from farmbot_client import FarmBotConnectionError
+from safety_service import UnsafeActionError
+from twfarmbot_api_server.app import create_app
+from twfarmbot_core.actions import ActionRegistry
+from twfarmbot_core.domain import Action
+
+
+@pytest.fixture
+def client() -> TestClient:
+    registry = ActionRegistry()
+    registry.register("noop", lambda a: a)
+    app = create_app(registry=registry)
+    return TestClient(app)
+
+
+def test_post_action_returns_200_for_valid_action(client: TestClient) -> None:
+    r = client.post("/actions", json={"kind": "noop", "params": {"foo": "bar"}})
+    assert r.status_code == 200
+    body = r.json()
+    assert body["status"] == "ok"
+    assert body["action"]["kind"] == "noop"
+    assert body["action"]["params"] == {"foo": "bar"}
+
+
+def test_post_action_returns_404_for_unknown_kind(client: TestClient) -> None:
+    r = client.post("/actions", json={"kind": "unknown", "params": {}})
+    assert r.status_code == 404
+    assert "no handler registered" in r.json()["detail"].lower()
+
+
+def test_post_action_returns_400_for_unsafe_action(
+    client: TestClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    def fake_validate(action: Action) -> None:
+        raise UnsafeActionError("too dangerous")
+
+    monkeypatch.setattr("twfarmbot_core.actions.safety_validate", fake_validate)
+    r = client.post("/actions", json={"kind": "noop", "params": {}})
+    assert r.status_code == 400
+    assert "too dangerous" in r.json()["detail"]
+
+
+def test_post_action_returns_502_on_farmbot_connection_error(
+    client: TestClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    def explode(action: Action) -> Action:
+        raise FarmBotConnectionError("broker down")
+
+    monkeypatch.setattr(client.app.state.registry, "dispatch", explode)
+    r = client.post("/actions", json={"kind": "noop", "params": {}})
+    assert r.status_code == 502
+    detail = r.json()["detail"]
+    assert "FarmBot not connected" in detail
+    assert "broker down" in detail
+
+
+def test_post_action_returns_500_with_real_error_message(
+    client: TestClient, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    def explode(action: Action) -> Action:
+        raise RuntimeError("handler exploded")
+
+    monkeypatch.setattr(client.app.state.registry, "dispatch", explode)
+    r = client.post("/actions", json={"kind": "noop", "params": {}})
+    assert r.status_code == 500
+    detail = r.json()["detail"]
+    assert "RuntimeError" in detail
+    assert "handler exploded" in detail
+
+
+def test_post_action_non_wait_returns_404_for_unknown_kind(client: TestClient) -> None:
+    r = client.post("/actions?wait=false", json={"kind": "unknown", "params": {}})
+    assert r.status_code == 404
+    assert "unknown action kind" in r.json()["detail"].lower()

tests/test_harness.py

@@ -11,6 +11,7 @@
 
 import json
 
+import pytest
 from langchain_core.language_models.fake_chat_models import FakeListChatModel
 from langchain_core.messages import AIMessage, AIMessageChunk
 from langchain_core.messages.tool import ToolCallChunk
@@ -22,6 +23,7 @@
     ContextBuilder,
     ReasoningController,
     ToolCategory,
+    ToolDescriptor,
     ToolPolicy,
     ToolRegistry,
 )

tests/test_ui.py

@@ -73,6 +73,34 @@ def test_api_client_strips_trailing_slash() -> None:
     assert c.base_url == "http://api"
 
 
+def test_api_result_error_message_prefers_detail() -> None:
+    from twfarmbot_ui.client import ApiResult
+
+    r = ApiResult(ok=False, code=500, body={"detail": "FarmBot not connected"})
+    assert r.error_message() == "FarmBot not connected"
+
+
+def test_api_result_error_message_falls_back_to_error_key() -> None:
+    from twfarmbot_ui.client import ApiResult
+
+    r = ApiResult(ok=False, code=0, body={"error": "ConnectError: refused"})
+    assert r.error_message() == "ConnectError: refused"
+
+
+def test_api_result_error_message_falls_back_to_string_body() -> None:
+    from twfarmbot_ui.client import ApiResult
+
+    r = ApiResult(ok=False, code=500, body="raw error text")
+    assert r.error_message() == "raw error text"
+
+
+def test_api_result_error_message_stringifies_unknown_body() -> None:
+    from twfarmbot_ui.client import ApiResult
+
+    r = ApiResult(ok=False, code=500, body={"nested": ["info"]})
+    assert "nested" in r.error_message()
+
+
 def test_huggingface_image_processor_calls_gradio_endpoint(
     monkeypatch: "pytest.MonkeyPatch",
 ) -> None: