Skip to content

Commit e0a2c9d

Browse files
authored
fix: Check plan outcome when calling Plan objects (#1470)
When using the client.plans.plan_name() approach to running plans, the task status returned by the server was not being checked so that plans failing did not raise exceptions on the client side. Checking the task status also allows values returned by plans to be accessed easily if they were serializable.
1 parent a7cb17f commit e0a2c9d

2 files changed

Lines changed: 48 additions & 4 deletions

File tree

src/blueapi/client/client.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from functools import cached_property
77
from itertools import chain
88
from pathlib import Path
9-
from typing import Self
9+
from typing import Any, Self
1010

1111
from bluesky_stomp.messaging import MessageContext, StompClient
1212
from bluesky_stomp.models import Broker
@@ -38,7 +38,7 @@
3838
)
3939
from blueapi.utils import deprecated
4040
from blueapi.worker import WorkerEvent, WorkerState
41-
from blueapi.worker.event import ProgressEvent, TaskStatus
41+
from blueapi.worker.event import ProgressEvent, TaskError, TaskResult, TaskStatus
4242
from blueapi.worker.task_worker import TrackableTask
4343

4444
from .event_bus import AnyEvent, EventBusClient, OnAnyEvent
@@ -141,13 +141,17 @@ def __init__(self, name, model: PlanModel, client: "BlueapiClient"):
141141
self._client = client
142142
self.__doc__ = model.description
143143

144-
def __call__(self, *args, **kwargs):
144+
def __call__(self, *args, **kwargs) -> Any:
145145
req = TaskRequest(
146146
name=self.name,
147147
params=self._build_args(*args, **kwargs),
148148
instrument_session=self._client.instrument_session,
149149
)
150-
self._client.run_task(req)
150+
match self._client.run_task(req):
151+
case TaskStatus(result=TaskResult(result=res)):
152+
return res
153+
case TaskStatus(result=TaskError(type=typ, message=msg)):
154+
raise PlanFailedError(typ, msg)
151155

152156
@property
153157
def help_text(self) -> str:
@@ -744,3 +748,9 @@ def login(self, token_path: Path | None = None):
744748
auth.start_device_flow()
745749
else:
746750
print("Server is not configured to use authentication!")
751+
752+
753+
class PlanFailedError(Exception):
754+
def __init__(self, typ: str, message: str):
755+
super().__init__(message)
756+
self._type = typ

tests/unit_tests/client/test_client.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
MissingInstrumentSessionError,
1818
Plan,
1919
PlanCache,
20+
PlanFailedError,
2021
)
2122
from blueapi.client.event_bus import AnyEvent, EventBusClient
2223
from blueapi.client.rest import BlueapiRestClient, BlueskyRemoteControlError
@@ -512,6 +513,39 @@ def callback(on_event: Callable[[AnyEvent, MessageContext], None]):
512513
mock_on_event.assert_called_once_with(COMPLETE_EVENT)
513514

514515

516+
def test_scripting_interface_returns_result():
517+
client = Mock(spec=BlueapiClient, instrument_session="cm12345-1")
518+
client.run_task.return_value = TaskStatus(
519+
task_id="foobar",
520+
task_complete=True,
521+
task_failed=False,
522+
result=TaskResult(result=42, type="int"),
523+
)
524+
demo_plan = Plan(
525+
"demo",
526+
client=client,
527+
model=PlanModel(name="demo", description="Demo plan", schema={}),
528+
)
529+
assert demo_plan() == 42
530+
531+
532+
def test_scripting_interface_raises_exceptions():
533+
client = Mock(spec=BlueapiClient, instrument_session="cm12345-1")
534+
client.run_task.return_value = TaskStatus(
535+
task_id="foobar",
536+
task_complete=True,
537+
task_failed=True,
538+
result=TaskError(type="ValueError", message="Plan failed"),
539+
)
540+
demo_plan = Plan(
541+
"demo",
542+
client=client,
543+
model=PlanModel(name="demo", description="Demo plan", schema={}),
544+
)
545+
with pytest.raises(PlanFailedError, match="Plan failed"):
546+
demo_plan()
547+
548+
515549
def test_oidc_config_property(client, mock_rest):
516550
assert client.oidc_config == mock_rest.get_oidc_config()
517551

0 commit comments

Comments
 (0)