Skip to content

Commit

Permalink
Merge branch 'edge' into EXEC-504-get-error-list-route
Browse files Browse the repository at this point in the history
  • Loading branch information
TamarZanzouri committed Aug 1, 2024
2 parents 0e3b72e + a5204fb commit ce55f2f
Show file tree
Hide file tree
Showing 33 changed files with 178 additions and 5 deletions.
1 change: 1 addition & 0 deletions api-client/src/runs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export interface LegacyGoodRunData {
status: RunStatus
actions: RunAction[]
errors: RunError[]
hasEverEnteredErrorRecovery: boolean
pipettes: LoadedPipette[]
labware: LoadedLabware[]
liquids: Liquid[]
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/cli/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ async def _do_analyze(protocol_source: ProtocolSource) -> RunResult:
modules=[],
labwareOffsets=[],
liquids=[],
hasEverEnteredErrorRecovery=False,
),
parameters=[],
)
Expand Down
18 changes: 18 additions & 0 deletions api/src/opentrons/protocol_engine/state/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ class CommandState:
This value can be used to generate future hashes.
"""

failed_command_errors: List[ErrorOccurrence]
"""List of command errors that occurred during run execution."""

has_entered_error_recovery: bool
"""Whether the run has entered error recovery."""

stopped_by_estop: bool
"""If this is set to True, the engine was stopped by an estop event."""

Expand Down Expand Up @@ -238,7 +244,9 @@ def __init__(
run_started_at=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=error_recovery_policy,
has_entered_error_recovery=False,
)

def handle_action(self, action: Action) -> None:
Expand Down Expand Up @@ -330,6 +338,7 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None:
notes=action.notes,
)
self._state.failed_command = self._state.command_history.get(action.command_id)
self._state.failed_command_errors.append(public_error_occurrence)

other_command_ids_to_fail: List[str]
if prev_entry.command.intent == CommandIntent.SETUP:
Expand Down Expand Up @@ -373,6 +382,7 @@ def _handle_fail_command_action(self, action: FailCommandAction) -> None:
):
self._state.queue_status = QueueStatus.AWAITING_RECOVERY
self._state.recovery_target_command_id = action.command_id
self._state.has_entered_error_recovery = True

def _handle_play_action(self, action: PlayAction) -> None:
if not self._state.run_result:
Expand Down Expand Up @@ -635,6 +645,14 @@ def get_error(self) -> Optional[ErrorOccurrence]:
else:
return run_error or finish_error

def get_all_errors(self) -> List[ErrorOccurrence]:
"""Get the run's full error list, if there was none, returns an empty list."""
return self._state.failed_command_errors

def get_has_entered_recovery_mode(self) -> bool:
"""Get whether the run has entered recovery mode."""
return self._state.has_entered_error_recovery

def get_running_command_id(self) -> Optional[str]:
"""Return the ID of the command that's currently running, if there is one."""
running_command = self._state.command_history.get_running_command()
Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/protocol_engine/state/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def get_summary(self) -> StateSummary:
completedAt=self._state.commands.run_completed_at,
startedAt=self._state.commands.run_started_at,
liquids=self._liquid.get_all(),
hasEverEnteredErrorRecovery=self._commands.get_has_entered_recovery_mode(),
)


Expand Down
1 change: 1 addition & 0 deletions api/src/opentrons/protocol_engine/state/state_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class StateSummary(BaseModel):
# errors is a list for historical reasons. (This model needs to stay compatible with
# robot-server's database.) It shouldn't have more than 1 element.
errors: List[ErrorOccurrence]
hasEverEnteredErrorRecovery: bool = Field(default=False)
labware: List[LoadedLabware]
pipettes: List[LoadedPipette]
modules: List[LoadedModule]
Expand Down
54 changes: 50 additions & 4 deletions api/tests/opentrons/protocol_engine/state/test_command_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def test_command_failure(error_recovery_type: ErrorRecoveryType) -> None:
)

assert subject_view.get("command-id") == expected_failed_command
assert subject.state.failed_command_errors == [expected_error_occurrence]


def test_command_failure_clears_queues() -> None:
Expand Down Expand Up @@ -227,12 +228,20 @@ def test_command_failure_clears_queues() -> None:
started_at=datetime(year=2022, month=2, day=2),
)
subject.handle_action(run_1)
expected_error = errors.ProtocolEngineError(message="oh no")
expected_error_occurance = errors.ErrorOccurrence(
id="error-id",
errorType="ProtocolEngineError",
createdAt=datetime(year=2023, month=3, day=3),
detail="oh no",
errorCode=ErrorCodes.GENERAL_ERROR.value.code,
)
fail_1 = actions.FailCommandAction(
command_id="command-id-1",
running_command=subject_view.get("command-id-1"),
error_id="error-id",
failed_at=datetime(year=2023, month=3, day=3),
error=errors.ProtocolEngineError(message="oh no"),
error=expected_error,
notes=[],
type=ErrorRecoveryType.FAIL_RUN,
)
Expand All @@ -245,6 +254,7 @@ def test_command_failure_clears_queues() -> None:
assert subject_view.get_running_command_id() is None
assert subject_view.get_queue_ids() == OrderedSet()
assert subject_view.get_next_to_execute() is None
assert subject.state.failed_command_errors == [expected_error_occurance]


def test_setup_command_failure_only_clears_setup_command_queue() -> None:
Expand Down Expand Up @@ -489,12 +499,20 @@ def test_door_during_error_recovery() -> None:
started_at=datetime(year=2022, month=2, day=2),
)
subject.handle_action(run_1)
expected_error = errors.ProtocolEngineError(message="oh no")
expected_error_occurance = errors.ErrorOccurrence(
id="error-id",
errorType="ProtocolEngineError",
createdAt=datetime(year=2023, month=3, day=3),
detail="oh no",
errorCode=ErrorCodes.GENERAL_ERROR.value.code,
)
fail_1 = actions.FailCommandAction(
command_id="command-id-1",
running_command=subject_view.get("command-id-1"),
error_id="error-id",
failed_at=datetime(year=2023, month=3, day=3),
error=errors.ProtocolEngineError(message="oh no"),
error=expected_error,
notes=[],
type=ErrorRecoveryType.WAIT_FOR_RECOVERY,
)
Expand Down Expand Up @@ -536,6 +554,7 @@ def test_door_during_error_recovery() -> None:
subject.handle_action(play)
assert subject_view.get_status() == EngineStatus.AWAITING_RECOVERY
assert subject_view.get_next_to_execute() == "command-id-2"
assert subject.state.failed_command_errors == [expected_error_occurance]


@pytest.mark.parametrize(
Expand Down Expand Up @@ -605,7 +624,7 @@ def test_error_recovery_type_tracking() -> None:
command_id="c1",
running_command=running_command_1,
error_id="c1-error",
failed_at=datetime.now(),
failed_at=datetime(year=2023, month=3, day=3),
error=PythonException(RuntimeError("new sheriff in town")),
notes=[],
type=ErrorRecoveryType.WAIT_FOR_RECOVERY,
Expand All @@ -620,7 +639,7 @@ def test_error_recovery_type_tracking() -> None:
command_id="c2",
running_command=running_command_2,
error_id="c2-error",
failed_at=datetime.now(),
failed_at=datetime(year=2023, month=3, day=3),
error=PythonException(RuntimeError("new sheriff in town")),
notes=[],
type=ErrorRecoveryType.FAIL_RUN,
Expand All @@ -631,6 +650,19 @@ def test_error_recovery_type_tracking() -> None:
assert view.get_error_recovery_type("c1") == ErrorRecoveryType.WAIT_FOR_RECOVERY
assert view.get_error_recovery_type("c2") == ErrorRecoveryType.FAIL_RUN

exception = PythonException(RuntimeError("new sheriff in town"))
error_occurrence_1 = ErrorOccurrence.from_failed(
id="c1-error", createdAt=datetime(year=2023, month=3, day=3), error=exception
)
error_occurrence_2 = ErrorOccurrence.from_failed(
id="c2-error", createdAt=datetime(year=2023, month=3, day=3), error=exception
)

assert subject.state.failed_command_errors == [
error_occurrence_1,
error_occurrence_2,
]


def test_recovery_target_tracking() -> None:
"""It should keep track of the command currently undergoing error recovery."""
Expand Down Expand Up @@ -729,6 +761,8 @@ def test_recovery_target_tracking() -> None:
assert subject_view.get_recovery_target() is None
assert not subject_view.get_recovery_in_progress_for_command("c3")

assert subject_view.get_has_entered_recovery_mode() is True


def test_final_state_after_estop() -> None:
"""Test the final state of the run after it's E-stopped."""
Expand Down Expand Up @@ -761,6 +795,7 @@ def test_final_state_after_estop() -> None:

assert subject_view.get_status() == EngineStatus.FAILED
assert subject_view.get_error() == expected_error_occurrence
assert subject_view.get_all_errors() == []


def test_final_state_after_stop() -> None:
Expand Down Expand Up @@ -833,6 +868,13 @@ def test_final_state_after_error_recovery_stop() -> None:
notes=[],
type=ErrorRecoveryType.WAIT_FOR_RECOVERY,
)
expected_error_occurrence_1 = ErrorOccurrence(
id="error-id",
createdAt=datetime(year=2023, month=3, day=3),
errorCode=ErrorCodes.GENERAL_ERROR.value.code,
errorType="ProtocolEngineError",
detail="oh no",
)
subject.handle_action(fail_1)
assert subject_view.get_status() == EngineStatus.AWAITING_RECOVERY

Expand All @@ -856,9 +898,13 @@ def test_final_state_after_error_recovery_stop() -> None:
finish_error_details=None,
)
)

assert subject_view.get_status() == EngineStatus.STOPPED
assert subject_view.get_recovery_target() is None
assert subject_view.get_error() is None
assert subject_view.get_all_errors() == [
expected_error_occurrence_1,
]


def test_set_and_get_error_recovery_policy() -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ def test_command_store_handles_pause_action(pause_source: PauseSource) -> None:
recovery_target_command_id=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)


Expand Down Expand Up @@ -365,7 +367,9 @@ def test_command_store_handles_play_action(pause_source: PauseSource) -> None:
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -398,7 +402,9 @@ def test_command_store_handles_finish_action() -> None:
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -451,7 +457,9 @@ def test_command_store_handles_stop_action(
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=from_estop,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -487,7 +495,9 @@ def test_command_store_handles_stop_action_when_awaiting_recovery() -> None:
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -519,7 +529,9 @@ def test_command_store_cannot_restart_after_should_stop() -> None:
run_started_at=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -664,7 +676,9 @@ def test_command_store_wraps_unknown_errors() -> None:
recovery_target_command_id=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -732,7 +746,9 @@ def __init__(self, message: str) -> None:
run_started_at=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -766,7 +782,9 @@ def test_command_store_ignores_stop_after_graceful_finish() -> None:
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -800,7 +818,9 @@ def test_command_store_ignores_finish_after_non_graceful_stop() -> None:
run_started_at=datetime(year=2021, month=1, day=1),
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down Expand Up @@ -834,7 +854,9 @@ def test_handles_hardware_stopped() -> None:
run_started_at=None,
latest_protocol_command_hash=None,
stopped_by_estop=False,
failed_command_errors=[],
error_recovery_policy=matchers.Anything(),
has_entered_error_recovery=False,
)
assert subject.state.command_history.get_running_command() is None
assert subject.state.command_history.get_all_ids() == []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ def get_command_view( # noqa: C901
finish_error: Optional[errors.ErrorOccurrence] = None,
commands: Sequence[cmd.Command] = (),
latest_command_hash: Optional[str] = None,
failed_command_errors: Optional[List[ErrorOccurrence]] = None,
has_entered_error_recovery: bool = False,
) -> CommandView:
"""Get a command view test subject."""
command_history = CommandHistory()
Expand Down Expand Up @@ -108,6 +110,8 @@ def get_command_view( # noqa: C901
run_started_at=run_started_at,
latest_protocol_command_hash=latest_command_hash,
stopped_by_estop=False,
failed_command_errors=failed_command_errors or [],
has_entered_error_recovery=has_entered_error_recovery,
error_recovery_policy=_placeholder_error_recovery_policy,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { simpleAnalysisFileFixture } from '@opentrons/api-client'
import { OT2_ROBOT_TYPE } from '@opentrons/shared-data'
import { renderWithProviders } from '../../../__testing-utils__'
import { i18n } from '../../../i18n'
import { useFeatureFlag } from '../../../redux/config'
import { getStoredProtocols } from '../../../redux/protocol-storage'
import { mockConnectableRobot } from '../../../redux/discovery/__fixtures__'
import {
Expand All @@ -18,6 +19,7 @@ import { useCreateRunFromProtocol } from '../../ChooseRobotToRunProtocolSlideout
import { ChooseProtocolSlideout } from '../'
import { useNotifyDataReady } from '../../../resources/useNotifyDataReady'
import type { ProtocolAnalysisOutput } from '@opentrons/shared-data'
import { when } from 'vitest-when'

vi.mock('../../ChooseRobotToRunProtocolSlideout/useCreateRunFromProtocol')
vi.mock('../../../redux/protocol-storage')
Expand Down Expand Up @@ -67,6 +69,7 @@ describe('ChooseProtocolSlideout', () => {
trackCreateProtocolRunEvent: mockTrackCreateProtocolRunEvent,
})
vi.mocked(useNotifyDataReady).mockReturnValue({} as any)
when(vi.mocked(useFeatureFlag)).calledWith('enableCsvFile').thenReturn(true)
})

it('renders slideout if showSlideout true', () => {
Expand Down Expand Up @@ -127,6 +130,7 @@ describe('ChooseProtocolSlideout', () => {
files: [expect.any(File)],
protocolKey: storedProtocolDataFixture.protocolKey,
runTimeParameterValues: expect.any(Object),
runTimeParameterFiles: expect.any(Object),
})
)
expect(mockTrackCreateProtocolRunEvent).toHaveBeenCalled()
Expand Down
Loading

0 comments on commit ce55f2f

Please sign in to comment.