feat(api, robot-server): get historical and current command errors (#…

…16697)

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

@@ -24,6 +24,9 @@ class CommandHistory:
     _all_command_ids: List[str]
     """All command IDs, in insertion order."""
 
+    _all_failed_command_ids: List[str]
+    """All failed command IDs, in insertion order."""
+
     _all_command_ids_but_fixit_command_ids: List[str]
     """All command IDs besides fixit command intents, in insertion order."""
 
@@ -47,6 +50,7 @@ class CommandHistory:
 
     def __init__(self) -> None:
         self._all_command_ids = []
+        self._all_failed_command_ids = []
         self._all_command_ids_but_fixit_command_ids = []
         self._queued_command_ids = OrderedSet()
         self._queued_setup_command_ids = OrderedSet()
@@ -101,6 +105,13 @@ def get_all_commands(self) -> List[Command]:
             for command_id in self._all_command_ids
         ]
 
+    def get_all_failed_commands(self) -> List[Command]:
+        """Get all failed commands."""
+        return [
+            self._commands_by_id[command_id].command
+            for command_id in self._all_failed_command_ids
+        ]
+
     def get_filtered_command_ids(self, include_fixit_commands: bool) -> List[str]:
         """Get all fixit command IDs."""
         if include_fixit_commands:
@@ -242,6 +253,7 @@ def set_command_failed(self, command: Command) -> None:
         self._remove_queue_id(command.id)
         self._remove_setup_queue_id(command.id)
         self._set_most_recently_completed_command_id(command.id)
+        self._all_failed_command_ids.append(command.id)
 
     def _add(self, command_id: str, command_entry: CommandEntry) -> None:
         """Create or update a command entry."""

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

@@ -228,9 +228,6 @@ 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."""
 
@@ -269,7 +266,6 @@ 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,
         )
@@ -366,7 +362,6 @@ 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)
 
         if (
             prev_entry.command.intent in (CommandIntent.PROTOCOL, None)
@@ -706,7 +701,12 @@ def get_error(self) -> Optional[ErrorOccurrence]:
 
     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
+        failed_commands = self._state.command_history.get_all_failed_commands()
+        return [
+            command_error.error
+            for command_error in failed_commands
+            if command_error.error is not None
+        ]
 
     def get_has_entered_recovery_mode(self) -> bool:
         """Get whether the run has entered recovery mode."""

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

@@ -30,6 +30,7 @@
 from opentrons.protocol_engine.state.commands import (
     CommandStore,
     CommandView,
+    CommandErrorSlice,
 )
 from opentrons.protocol_engine.state.config import Config
 from opentrons.protocol_engine.state.update_types import StateUpdate
@@ -193,7 +194,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]
+    assert subject_view.get_all_errors() == [expected_error_occurrence]
 
 
 def test_command_failure_clears_queues() -> None:
@@ -255,7 +256,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]
+    assert subject_view.get_all_errors() == [expected_error_occurance]
 
 
 def test_setup_command_failure_only_clears_setup_command_queue() -> None:
@@ -555,7 +556,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]
+    assert subject_view.get_all_errors() == [expected_error_occurance]
 
 
 @pytest.mark.parametrize("close_door_before_queueing", [False, True])
@@ -732,7 +733,7 @@ def test_error_recovery_type_tracking() -> None:
         id="c2-error", createdAt=datetime(year=2023, month=3, day=3), error=exception
     )
 
-    assert subject.state.failed_command_errors == [
+    assert view.get_all_errors() == [
         error_occurrence_1,
         error_occurrence_2,
     ]
@@ -1100,3 +1101,94 @@ def test_get_state_update_for_false_positive() -> None:
     subject.handle_action(resume_from_recovery)
 
     assert subject_view.get_state_update_for_false_positive() == empty_state_update
+
+
+def test_get_errors_slice_empty() -> None:
+    """It should return an empty error list."""
+    subject = CommandStore(
+        config=_make_config(),
+        error_recovery_policy=_placeholder_error_recovery_policy,
+        is_door_open=False,
+    )
+    subject_view = CommandView(subject.state)
+    result = subject_view.get_errors_slice(cursor=0, length=2)
+
+    assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0)
+
+
+def test_get_errors_slice() -> None:
+    """It should return a slice of all command errors."""
+    subject = CommandStore(
+        config=_make_config(),
+        error_recovery_policy=_placeholder_error_recovery_policy,
+        is_door_open=False,
+    )
+
+    subject_view = CommandView(subject.state)
+
+    queue_1 = actions.QueueCommandAction(
+        request=commands.WaitForResumeCreate(
+            params=commands.WaitForResumeParams(), key="command-key-1"
+        ),
+        request_hash=None,
+        created_at=datetime(year=2021, month=1, day=1),
+        command_id="command-id-1",
+    )
+    subject.handle_action(queue_1)
+    queue_2_setup = actions.QueueCommandAction(
+        request=commands.WaitForResumeCreate(
+            params=commands.WaitForResumeParams(),
+            intent=commands.CommandIntent.SETUP,
+            key="command-key-2",
+        ),
+        request_hash=None,
+        created_at=datetime(year=2021, month=1, day=1),
+        command_id="command-id-2",
+    )
+    subject.handle_action(queue_2_setup)
+    queue_3_setup = actions.QueueCommandAction(
+        request=commands.WaitForResumeCreate(
+            params=commands.WaitForResumeParams(),
+            intent=commands.CommandIntent.SETUP,
+            key="command-key-3",
+        ),
+        request_hash=None,
+        created_at=datetime(year=2021, month=1, day=1),
+        command_id="command-id-3",
+    )
+    subject.handle_action(queue_3_setup)
+
+    run_2_setup = actions.RunCommandAction(
+        command_id="command-id-2",
+        started_at=datetime(year=2022, month=2, day=2),
+    )
+    subject.handle_action(run_2_setup)
+    fail_2_setup = actions.FailCommandAction(
+        command_id="command-id-2",
+        running_command=subject_view.get("command-id-2"),
+        error_id="error-id",
+        failed_at=datetime(year=2023, month=3, day=3),
+        error=errors.ProtocolEngineError(message="oh no"),
+        notes=[],
+        type=ErrorRecoveryType.CONTINUE_WITH_ERROR,
+    )
+    subject.handle_action(fail_2_setup)
+
+    result = subject_view.get_errors_slice(cursor=1, length=3)
+
+    assert result == CommandErrorSlice(
+        [
+            ErrorOccurrence(
+                id="error-id",
+                createdAt=datetime(2023, 3, 3, 0, 0),
+                isDefined=False,
+                errorType="ProtocolEngineError",
+                errorCode="4000",
+                detail="oh no",
+                errorInfo={},
+                wrappedErrors=[],
+            )
+        ],
+        cursor=0,
+        total_length=1,
+    )

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

@@ -333,7 +333,6 @@ def test_command_store_handles_pause_action(pause_source: PauseSource) -> None:
         recovery_target=None,
         latest_protocol_command_hash=None,
         stopped_by_estop=False,
-        failed_command_errors=[],
         error_recovery_policy=matchers.Anything(),
         has_entered_error_recovery=False,
     )
@@ -363,7 +362,6 @@ 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,
     )
@@ -398,7 +396,6 @@ 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,
     )
@@ -453,7 +450,6 @@ 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,
     )
@@ -491,7 +487,6 @@ 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,
     )
@@ -525,7 +520,6 @@ 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,
     )
@@ -672,7 +666,6 @@ def test_command_store_wraps_unknown_errors() -> None:
         recovery_target=None,
         latest_protocol_command_hash=None,
         stopped_by_estop=False,
-        failed_command_errors=[],
         error_recovery_policy=matchers.Anything(),
         has_entered_error_recovery=False,
     )
@@ -742,7 +735,6 @@ 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,
     )
@@ -778,7 +770,6 @@ 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,
     )
@@ -814,7 +805,6 @@ 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,
     )
@@ -850,7 +840,6 @@ 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,
     )

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

@@ -28,19 +28,17 @@
     CommandState,
     CommandView,
     CommandSlice,
-    CommandErrorSlice,
     CommandPointer,
     RunResult,
     QueueStatus,
 )
 
-from opentrons.protocol_engine.state.command_history import CommandEntry
+from opentrons.protocol_engine.state.command_history import CommandEntry, CommandHistory
 
 from opentrons.protocol_engine.errors import ProtocolCommandFailedError, ErrorOccurrence
 
 from opentrons_shared_data.errors.codes import ErrorCodes
 
-from opentrons.protocol_engine.state.command_history import CommandHistory
 from opentrons.protocol_engine.state.update_types import StateUpdate
 
 from .command_fixtures import (
@@ -77,7 +75,6 @@ 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."""
@@ -121,7 +118,6 @@ 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,
     )
@@ -1031,42 +1027,6 @@ def test_get_slice_default_cursor_running() -> None:
     )
 
 
-def test_get_errors_slice_empty() -> None:
-    """It should return a slice from the tail if no current command."""
-    subject = get_command_view(failed_command_errors=[])
-    result = subject.get_errors_slice(cursor=0, length=2)
-
-    assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0)
-
-
-def test_get_errors_slice() -> None:
-    """It should return a slice of all command errors."""
-    error_1 = ErrorOccurrence.construct(id="error-id-1")  # type: ignore[call-arg]
-    error_2 = ErrorOccurrence.construct(id="error-id-2")  # type: ignore[call-arg]
-    error_3 = ErrorOccurrence.construct(id="error-id-3")  # type: ignore[call-arg]
-    error_4 = ErrorOccurrence.construct(id="error-id-4")  # type: ignore[call-arg]
-
-    subject = get_command_view(
-        failed_command_errors=[error_1, error_2, error_3, error_4]
-    )
-
-    result = subject.get_errors_slice(cursor=1, length=3)
-
-    assert result == CommandErrorSlice(
-        commands_errors=[error_2, error_3, error_4],
-        cursor=1,
-        total_length=4,
-    )
-
-    result = subject.get_errors_slice(cursor=-3, length=10)
-
-    assert result == CommandErrorSlice(
-        commands_errors=[error_1, error_2, error_3, error_4],
-        cursor=0,
-        total_length=4,
-    )
-
-
 def test_get_slice_without_fixit() -> None:
     """It should select a cursor based on the running command, if present."""
     command_1 = create_succeeded_command(command_id="command-id-1")

robot-server/robot_server/data_files/data_files_store.py

@@ -15,7 +15,7 @@
     analysis_csv_rtp_table,
     run_csv_rtp_table,
 )
-from robot_server.persistence.tables.schema_7 import DataFileSourceSQLEnum
+from robot_server.persistence.tables import DataFileSourceSQLEnum
 
 from .models import DataFileSource, FileIdNotFoundError, FileInUseError