Skip to content

Commit d83c4e6

Browse files
fix(api): restore empty error blocks on cancelled runs (#15215)
I think this would be a subtle side effect of a previous pr trying to improve PE end behavior. The reason this fix is required is that when a client cancels a protocol by stopping the engine, the `StopAction` sets the run result (good) and doesn't set a run error (good, correct, there wasn't one, this is a cancel). In 7.2, with the behavior this PR now restores, a `FinishAction` that might contain an error wouldn't set that error in the run if it already had a _result_, whether or not it had an _error_. In 7.3, it would set the error if the engine had no error, which it wouldn't, because there is no error when you stop. ## Testing - [x] With this PR, cancelling a run on a real robot causes the ODD to show an inactive error details button - [x] With this PR, stopping a run by hitting an estop still causes the ODD and desktop to display that an estop error cancelled the run Closes RQA-2737 --------- Co-authored-by: Max Marrone <[email protected]>
1 parent bb1f1ce commit d83c4e6

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

api/src/opentrons/protocol_engine/state/commands.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,21 @@ def handle_action(self, action: Action) -> None: # noqa: C901
384384
else:
385385
self._state.run_result = RunResult.STOPPED
386386

387-
if not self._state.run_error and action.error_details:
388-
self._state.run_error = self._map_run_exception_to_error_occurrence(
389-
action.error_details.error_id,
390-
action.error_details.created_at,
391-
action.error_details.error,
392-
)
387+
if not self._state.run_error and action.error_details:
388+
self._state.run_error = self._map_run_exception_to_error_occurrence(
389+
action.error_details.error_id,
390+
action.error_details.created_at,
391+
action.error_details.error,
392+
)
393+
else:
394+
# HACK(sf): There needs to be a better way to set
395+
# an estop error than this else clause
396+
if self._state.stopped_by_estop and action.error_details:
397+
self._state.run_error = self._map_run_exception_to_error_occurrence(
398+
action.error_details.error_id,
399+
action.error_details.created_at,
400+
action.error_details.error,
401+
)
393402

394403
elif isinstance(action, HardwareStoppedAction):
395404
self._state.queue_status = QueueStatus.PAUSED

api/tests/opentrons/protocol_engine/state/test_command_state.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,3 +469,33 @@ def test_final_state_after_estop() -> None:
469469

470470
assert subject_view.get_status() == EngineStatus.FAILED
471471
assert subject_view.get_error() == expected_error_occurrence
472+
473+
474+
def test_final_state_after_stop() -> None:
475+
"""Test the final state of the run after it's stopped."""
476+
subject = CommandStore(config=_make_config(), is_door_open=False)
477+
subject_view = CommandView(subject.state)
478+
479+
subject.handle_action(actions.StopAction())
480+
subject.handle_action(
481+
actions.FinishAction(
482+
error_details=actions.FinishErrorDetails(
483+
error=RuntimeError(
484+
"uh oh I was a command and then I got cancelled because someone"
485+
" stopped the run, and now I'm raising this exception because"
486+
" of that. Woe is me"
487+
),
488+
error_id="error-id",
489+
created_at=datetime.now(),
490+
)
491+
)
492+
)
493+
subject.handle_action(
494+
actions.HardwareStoppedAction(
495+
completed_at=sentinel.hardware_stopped_action_completed_at,
496+
finish_error_details=None,
497+
)
498+
)
499+
500+
assert subject_view.get_status() == EngineStatus.STOPPED
501+
assert subject_view.get_error() is None

0 commit comments

Comments
 (0)