Skip to content

Commit

Permalink
fix(api): restore empty error blocks on cancelled runs (#15215)
Browse files Browse the repository at this point in the history
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 <max@opentrons.com>
  • Loading branch information
sfoster1 and SyntaxColoring committed May 17, 2024
1 parent bb1f1ce commit d83c4e6
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
21 changes: 15 additions & 6 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions api/tests/opentrons/protocol_engine/state/test_command_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d83c4e6

Please sign in to comment.