From d83c4e6ccb2a96806783774200acd0f9d67b2805 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Fri, 17 May 2024 19:28:55 -0400 Subject: [PATCH] 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 --- .../protocol_engine/state/commands.py | 21 +++++++++---- .../state/test_command_state.py | 30 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py index 7500b16d631..0c055fdee39 100644 --- a/api/src/opentrons/protocol_engine/state/commands.py +++ b/api/src/opentrons/protocol_engine/state/commands.py @@ -384,12 +384,21 @@ def handle_action(self, action: Action) -> None: # noqa: C901 else: self._state.run_result = RunResult.STOPPED - if not self._state.run_error and action.error_details: - self._state.run_error = self._map_run_exception_to_error_occurrence( - action.error_details.error_id, - action.error_details.created_at, - action.error_details.error, - ) + if not self._state.run_error and action.error_details: + self._state.run_error = self._map_run_exception_to_error_occurrence( + action.error_details.error_id, + action.error_details.created_at, + action.error_details.error, + ) + else: + # HACK(sf): There needs to be a better way to set + # an estop error than this else clause + if self._state.stopped_by_estop and action.error_details: + self._state.run_error = self._map_run_exception_to_error_occurrence( + action.error_details.error_id, + action.error_details.created_at, + action.error_details.error, + ) elif isinstance(action, HardwareStoppedAction): self._state.queue_status = QueueStatus.PAUSED diff --git a/api/tests/opentrons/protocol_engine/state/test_command_state.py b/api/tests/opentrons/protocol_engine/state/test_command_state.py index 742abf3e6e9..01b9186ac9b 100644 --- a/api/tests/opentrons/protocol_engine/state/test_command_state.py +++ b/api/tests/opentrons/protocol_engine/state/test_command_state.py @@ -469,3 +469,33 @@ def test_final_state_after_estop() -> None: assert subject_view.get_status() == EngineStatus.FAILED assert subject_view.get_error() == expected_error_occurrence + + +def test_final_state_after_stop() -> None: + """Test the final state of the run after it's stopped.""" + subject = CommandStore(config=_make_config(), is_door_open=False) + subject_view = CommandView(subject.state) + + subject.handle_action(actions.StopAction()) + subject.handle_action( + actions.FinishAction( + error_details=actions.FinishErrorDetails( + error=RuntimeError( + "uh oh I was a command and then I got cancelled because someone" + " stopped the run, and now I'm raising this exception because" + " of that. Woe is me" + ), + error_id="error-id", + created_at=datetime.now(), + ) + ) + ) + subject.handle_action( + actions.HardwareStoppedAction( + completed_at=sentinel.hardware_stopped_action_completed_at, + finish_error_details=None, + ) + ) + + assert subject_view.get_status() == EngineStatus.STOPPED + assert subject_view.get_error() is None