Skip to content

fix: Check plan outcome when calling Plan objects#1470

Open
tpoliaw wants to merge 2 commits intomainfrom
client-task-result
Open

fix: Check plan outcome when calling Plan objects#1470
tpoliaw wants to merge 2 commits intomainfrom
client-task-result

Conversation

@tpoliaw
Copy link
Copy Markdown
Contributor

@tpoliaw tpoliaw commented Apr 2, 2026

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.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.22%. Comparing base (65b7e75) to head (8e74dea).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1470      +/-   ##
==========================================
+ Coverage   95.21%   95.22%   +0.01%     
==========================================
  Files          43       43              
  Lines        3132     3140       +8     
==========================================
+ Hits         2982     2990       +8     
  Misses        150      150              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tpoliaw tpoliaw force-pushed the client-task-result branch from 4a8807a to e2e0071 Compare April 7, 2026 13:34
@tpoliaw tpoliaw marked this pull request as ready for review April 7, 2026 15:05
@tpoliaw tpoliaw requested a review from a team as a code owner April 7, 2026 15:05
tpoliaw added 2 commits April 8, 2026 14:36
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.
@tpoliaw tpoliaw force-pushed the client-task-result branch from e2e0071 to 8e74dea Compare April 8, 2026 13:36
@tpoliaw tpoliaw requested a review from oliwenmandiamond April 8, 2026 13:36
@oliwenmandiamond
Copy link
Copy Markdown
Contributor

Have tested this, it is an improvement as an error is propagated to the user now

>>> plans.set_absolute(devs.fsj1, "INVALID")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/scratch/bluesky_development/blueapi/src/blueapi/client/client.py", line 154, in __call__
    raise PlanFailedError(typ, msg)
blueapi.client.client.PlanFailedError: Error(s) publishing event: state=<WorkerState.IDLE: 'IDLE'> task_status=TaskStatus(task_id='11a88252-9794-493c-a3a9-4a9c8517fef7', result=None, task_complete=False, task_failed=False) errors=[] warnings=[] (1 sub-exception)

However, it doesn't tell me anything useful. Ideally, it would tell me why it failed like below

bluesky.utils.FailedStatus: <AsyncStatus, device: fsj1, task: <coroutine object AsyncStatusBase.__init__.<locals>.wait_with_error_message at 0x7fc9baf97840>, errored: ValueError("INVALID is not a valid choice for BL09J-EA-FSHTR-01:CTRL, valid choices: ['Out', 'In']")>

@tpoliaw
Copy link
Copy Markdown
Contributor Author

tpoliaw commented Apr 10, 2026

However, it doesn't tell me anything useful

I think it does. In this case the plan didn't fail (note task_complete=False, task_failed=False in the message), but blueapi had failed because it couldn't broadcast the event.

It looks like the stomp connection was lost causing the event to not send, then the connection was restored in time for the "we failed to send the previous event" event to be sent successfully. We could potentially look at how retries are handled. It might be that updating to a bluesky-stomp release that includes bluesky-stomp/#45 would help.

For the cases where the plan did fail, it would be useful to see what events were sent and how the client handled them.

Copy link
Copy Markdown
Contributor

@oliwenmandiamond oliwenmandiamond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, what i ran into while testing seems to be a different issue. Happy for this to be merged then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlueapiClient reports successful response on device move when actually fails

2 participants