Fix reloading the environment if there is an error

This adds a new error_message field to the environment. Its presence is
now checked against when attempting to delete the environment as if
there is an error the handler is never set to initialized, so previously
could not be reloaded.

Fixes #512.

docs/reference/openapi.yaml

@@ -37,6 +37,10 @@ components:
       additionalProperties: false
       description: State of internal environment.
       properties:
+        error_message:
+          description: If present - error loading context
+          title: Error Message
+          type: string
         initialized:
           description: blueapi context initialized
           title: Initialized

src/blueapi/service/handler.py

@@ -8,7 +8,12 @@
 from blueapi.messaging import StompMessagingTemplate
 from blueapi.messaging.base import MessagingTemplate
 from blueapi.service.handler_base import BlueskyHandler
-from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
+from blueapi.service.model import (
+    DeviceModel,
+    EnvironmentResponse,
+    PlanModel,
+    WorkerTask,
+)
 from blueapi.worker.event import TaskStatusEnum, WorkerState
 from blueapi.worker.reworker import TaskWorker
 from blueapi.worker.task import Task
@@ -143,8 +148,8 @@ def get_task_by_id(self, task_id: str) -> TrackableTask | None:
         return self._worker.get_task_by_id(task_id)
 
     @property
-    def initialized(self) -> bool:
-        return self._initialized
+    def state(self) -> EnvironmentResponse:
+        return EnvironmentResponse(initialized=self._initialized)
 
 
 HANDLER: Handler | None = None

src/blueapi/service/handler_base.py

@@ -1,6 +1,11 @@
 from abc import ABC, abstractmethod
 
-from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
+from blueapi.service.model import (
+    DeviceModel,
+    EnvironmentResponse,
+    PlanModel,
+    WorkerTask,
+)
 from blueapi.worker.event import TaskStatusEnum, WorkerState
 from blueapi.worker.task import Task
 from blueapi.worker.worker import TrackableTask
@@ -103,7 +108,7 @@ def stop(self):
 
     @property
     @abstractmethod
-    def initialized(self) -> bool:
+    def state(self) -> EnvironmentResponse:
         """Handler initialization state"""
 
 

src/blueapi/service/main.py

@@ -88,7 +88,7 @@ async def on_key_error_404(_: Request, __: KeyError):
 def get_environment(
     handler: BlueskyHandler = Depends(get_handler),
 ) -> EnvironmentResponse:
-    return EnvironmentResponse(initialized=handler.initialized)
+    return handler.state
 
 
 @app.delete("/environment", response_model=EnvironmentResponse)
@@ -100,7 +100,7 @@ def restart_handler(handler: BlueskyHandler):
         handler.stop()
         handler.start()
 
-    if handler.initialized:
+    if handler.state.initialized or handler.state.error_message is not None:
         background_tasks.add_task(restart_handler, handler)
     return EnvironmentResponse(initialized=False)
 

src/blueapi/service/model.py

@@ -144,3 +144,6 @@ class EnvironmentResponse(BlueapiBaseModel):
     """
 
     initialized: bool = Field(description="blueapi context initialized")
+    error_message: str | None = Field(
+        default=None, description="If present - error loading context"
+    )

src/blueapi/service/subprocess_handler.py

@@ -7,7 +7,12 @@
 from blueapi.config import ApplicationConfig
 from blueapi.service.handler import get_handler, setup_handler, teardown_handler
 from blueapi.service.handler_base import BlueskyHandler, HandlerNotStartedError
-from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
+from blueapi.service.model import (
+    DeviceModel,
+    EnvironmentResponse,
+    PlanModel,
+    WorkerTask,
+)
 from blueapi.worker.event import TaskStatusEnum, WorkerState
 from blueapi.worker.task import Task
 from blueapi.worker.worker import TrackableTask
@@ -25,6 +30,7 @@ class SubprocessHandler(BlueskyHandler):
     _config: ApplicationConfig
     _subprocess: PoolClass | None
     _initialized: bool = False
+    _error_message: str | None = None
 
     def __init__(
         self,
@@ -39,7 +45,12 @@ def start(self):
             self._subprocess.apply(
                 logging.basicConfig, kwds={"level": self._config.logging.level}
             )
-            self._subprocess.apply(setup_handler, [self._config])
+            try:
+                self._subprocess.apply(setup_handler, [self._config])
+            except Exception as e:
+                self._error_message = f"Error configuring blueapi: {e}"
+                LOGGER.exception(self._error_message)
+                return
             self._initialized = True
 
     def stop(self):
@@ -48,6 +59,7 @@ def stop(self):
             self._subprocess.apply(teardown_handler)
             self._subprocess.close()
             self._subprocess.join()
+            self._error_message = None
             self._subprocess = None
 
     def reload_context(self):
@@ -113,8 +125,11 @@ def get_task_by_id(self, task_id: str) -> TrackableTask | None:
         return self._run_in_subprocess(get_task_by_id, [task_id])
 
     @property
-    def initialized(self) -> bool:
-        return self._initialized
+    def state(self) -> EnvironmentResponse:
+        return EnvironmentResponse(
+            initialized=self._initialized,
+            error_message=self._error_message,
+        )
 
 
 # Free functions (passed to subprocess) for each of the methods required by Handler

tests/service/test_rest_api.py

@@ -513,7 +513,10 @@ def test_current_complete_returns_400(
 
 
 def test_get_environment(handler: Handler, client: TestClient) -> None:
-    assert client.get("/environment").json() == {"initialized": False}
+    assert client.get("/environment").json() == {
+        "initialized": False,
+        "error_message": None,
+    }
 
 
 def test_delete_environment(handler: Handler, client: TestClient) -> None:

tests/service/test_subprocess_handler.py

@@ -1,9 +1,15 @@
+from unittest import mock
 from unittest.mock import MagicMock, patch
 
 import pytest
 
 from blueapi.service.handler_base import BlueskyHandler, HandlerNotStartedError
-from blueapi.service.model import DeviceModel, PlanModel, WorkerTask
+from blueapi.service.model import (
+    DeviceModel,
+    EnvironmentResponse,
+    PlanModel,
+    WorkerTask,
+)
 from blueapi.service.subprocess_handler import SubprocessHandler
 from blueapi.worker.event import TaskStatusEnum, WorkerState
 from blueapi.worker.task import Task
@@ -26,31 +32,23 @@
 ]
 
 
-@pytest.fixture(scope="module")
-def sp_handler():
-    sp_handler = SubprocessHandler()
-    sp_handler.start()
-    yield sp_handler
-    sp_handler.stop()
-
-
 def test_initialize():
     sp_handler = SubprocessHandler()
-    assert not sp_handler.initialized
+    assert not sp_handler.state.initialized
     sp_handler.start()
-    assert sp_handler.initialized
+    assert sp_handler.state.initialized
     # Run a single call to the handler for coverage of dispatch to subprocess
     assert sp_handler.tasks == []
     sp_handler.stop()
-    assert not sp_handler.initialized
+    assert not sp_handler.state.initialized
 
 
 def test_reload():
     sp_handler = SubprocessHandler()
     sp_handler.start()
-    assert sp_handler.initialized
+    assert sp_handler.state.initialized
     sp_handler.reload_context()
-    assert sp_handler.initialized
+    assert sp_handler.state.initialized
     sp_handler.stop()
 
 
@@ -60,6 +58,27 @@ def test_raises_if_not_started():
         assert sp_handler.worker_state is None
 
 
+def test_error_on_handler_setup():
+    sp_handler = SubprocessHandler()
+    expected_state = EnvironmentResponse(
+        initialized=False,
+        error_message="Error configuring blueapi: Can't pickle "
+        "<class 'unittest.mock.MagicMock'>: it's not the same object as "
+        "unittest.mock.MagicMock",
+    )
+
+    # Using a mock for setup_handler causes a failure as the mock is not pickleable
+    # An exception is set on the mock too but this is never reached
+    with mock.patch(
+        "blueapi.service.subprocess_handler.setup_handler",
+        side_effect=Exception("Mock setup_handler exception"),
+    ):
+        sp_handler.reload_context()
+        state = sp_handler.state
+        assert state == expected_state
+    sp_handler.stop()
+
+
 class DummyHandler(BlueskyHandler):
     @property
     def plans(self) -> list[PlanModel]:
@@ -125,10 +144,10 @@ def start(self): ...
 
     def stop(self): ...
 
-    # Initialized is a special case as it is not delegated
+    # state is a special case as it is not delegated
     # Tested by test_initialize
     @property
-    def initialized(self) -> bool:
+    def state(self) -> EnvironmentResponse:
         raise Exception("Not implemented")
 
 

tests/test_cli.py

@@ -264,7 +264,7 @@ def test_reset_env_client_behavior(
     # assert "Environment is initialized." in result.output
     assert (
         reload_result.output
-        == "Reloading the environment...\nEnvironment reload initiated.\nWaiting for environment to initialize...\nWaiting for environment to initialize...\nEnvironment is initialized.\ninitialized=True\n"  # noqa: E501
+        == "Reloading the environment...\nEnvironment reload initiated.\nWaiting for environment to initialize...\nWaiting for environment to initialize...\nEnvironment is initialized.\ninitialized=True error_message=None\n"  # noqa: E501
     )