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

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:
@@ -251,6 +262,10 @@ def _add(self, command_id: str, command_entry: CommandEntry) -> None:
                 self._all_command_ids_but_fixit_command_ids.append(command_id)
         self._commands_by_id[command_id] = command_entry
 
+    def append_failed_command_id(self, command_id: str) -> None:
+        """Create or update a failed command id."""
+        self._all_failed_command_ids.append(command_id)
+
     def _add_to_queue(self, command_id: str) -> None:
         """Add new ID to the queued."""
         self._queued_command_ids.add(command_id)

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,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)
+        self._state.command_history.append_failed_command_id(action.command_id)
 
         if (
             prev_entry.command.intent in (CommandIntent.PROTOCOL, None)
@@ -706,7 +702,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

@@ -193,7 +193,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 +255,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 +555,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 +732,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,
     ]

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

@@ -34,13 +34,12 @@
     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 +76,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."""
@@ -101,6 +99,8 @@ def get_command_view(  # noqa: C901
                 command_id=command.id,
                 command_entry=CommandEntry(index=index, command=command),
             )
+            if command.error:
+                command_history.append_failed_command_id(command.id)
 
     state = CommandState(
         command_history=command_history,
@@ -121,7 +121,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,
     )
@@ -1033,7 +1032,7 @@ 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=[])
+    subject = get_command_view()
     result = subject.get_errors_slice(cursor=0, length=2)
 
     assert result == CommandErrorSlice(commands_errors=[], cursor=0, total_length=0)
@@ -1046,8 +1045,15 @@ def test_get_errors_slice() -> None:
     error_3 = ErrorOccurrence.construct(id="error-id-3")  # type: ignore[call-arg]
     error_4 = ErrorOccurrence.construct(id="error-id-4")  # type: ignore[call-arg]
 
+    command_1 = create_failed_command(command_id="command-id-1", error=error_1)
+    command_2 = create_failed_command(command_id="command-id-2", error=error_2)
+    command_3 = create_failed_command(command_id="command-id-3", error=error_3)
+    command_4 = create_failed_command(command_id="command-id-4", error=error_4)
+    command_5 = create_running_command(command_id="command-id-5")
+    command_6 = create_queued_command(command_id="command-id-6")
+
     subject = get_command_view(
-        failed_command_errors=[error_1, error_2, error_3, error_4]
+        commands=[command_1, command_2, command_3, command_4, command_5, command_6]
     )
 
     result = subject.get_errors_slice(cursor=1, length=3)

robot-server/robot_server/persistence/_migrations/v7_to_v8.py

@@ -0,0 +1,86 @@
+"""Migrate the persistence directory from schema 7 to 8.
+
+Summary of changes from schema 7:
+
+- Adds a new command_intent to store the commands intent in the commands table
+- Adds a new source to store the data files origin in the data_files table
+- Adds the `boolean_setting` table.
+"""
+
+import json
+from pathlib import Path
+from contextlib import ExitStack
+import shutil
+from typing import Any
+
+import sqlalchemy
+
+from ..database import sql_engine_ctx, sqlite_rowid
+from ..tables import schema_8
+from .._folder_migrator import Migration
+
+from ..file_and_directory_names import (
+    DB_FILE,
+)
+
+
+class Migration7to8(Migration):  # noqa: D101
+    def migrate(self, source_dir: Path, dest_dir: Path) -> None:
+        """Migrate the persistence directory from schema 6 to 7."""
+        # Copy over all existing directories and files to new version
+        for item in source_dir.iterdir():
+            if item.is_dir():
+                shutil.copytree(src=item, dst=dest_dir / item.name)
+            else:
+                shutil.copy(src=item, dst=dest_dir / item.name)
+
+        dest_db_file = dest_dir / DB_FILE
+
+        # Append the new column to existing protocols and data_files in v6 database
+        with ExitStack() as exit_stack:
+            dest_engine = exit_stack.enter_context(sql_engine_ctx(dest_db_file))
+
+            schema_8.metadata.create_all(dest_engine)
+
+            dest_transaction = exit_stack.enter_context(dest_engine.begin())
+
+            def add_column(
+                engine: sqlalchemy.engine.Engine,
+                table_name: str,
+                column: Any,
+            ) -> None:
+                column_type = column.type.compile(engine.dialect)
+                engine.execute(
+                    f"ALTER TABLE {table_name} ADD COLUMN {column.key} {column_type}"
+                )
+
+            add_column(
+                dest_engine,
+                schema_8.run_command_table.name,
+                schema_8.run_command_table.c.command_error,
+            )
+
+            _migrate_command_table_with_new_command_error_col(
+                dest_transaction=dest_transaction
+            )
+
+
+def _migrate_command_table_with_new_command_error_col(
+    dest_transaction: sqlalchemy.engine.Connection,
+) -> None:
+    """Add a new 'command_intent' column to run_command_table table."""
+    select_commands = sqlalchemy.select(schema_8.run_command_table).order_by(
+        sqlite_rowid
+    )
+    for row in dest_transaction.execute(select_commands).all():
+        data = json.loads(row.command)
+        new_command_error = (
+            # Account for old_row.command["error"] being NULL.
+            None
+            if "error" not in row.command or data["error"] == None  # noqa: E711
+            else json.dumps(data["error"])
+        )
+
+        dest_transaction.execute(
+            f"UPDATE run_command SET command_error='{new_command_error}' WHERE row_id={row.row_id}"
+        )

robot-server/robot_server/persistence/file_and_directory_names.py

@@ -8,7 +8,7 @@
 
 from typing import Final
 
-LATEST_VERSION_DIRECTORY: Final = "7.1"
+LATEST_VERSION_DIRECTORY: Final = "8.1"
 
 DECK_CONFIGURATION_FILE: Final = "deck_configuration.json"
 PROTOCOLS_DIRECTORY: Final = "protocols"

robot-server/robot_server/persistence/persistence_directory.py

@@ -11,7 +11,7 @@
 from anyio import Path as AsyncPath, to_thread
 
 from ._folder_migrator import MigrationOrchestrator
-from ._migrations import up_to_3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7
+from ._migrations import up_to_3, v3_to_v4, v4_to_v5, v5_to_v6, v6_to_v7, v7_to_v8
 from .file_and_directory_names import LATEST_VERSION_DIRECTORY
 
 _TEMP_PERSISTENCE_DIR_PREFIX: Final = "opentrons-robot-server-"
@@ -55,7 +55,8 @@ async def prepare_active_subdirectory(prepared_root: Path) -> Path:
             # Subdirectory "7" was previously used on our edge branch for an in-dev
             # schema that was never released to the public. It may be present on
             # internal robots.
-            v6_to_v7.Migration6to7(subdirectory=LATEST_VERSION_DIRECTORY),
+            v6_to_v7.Migration6to7(subdirectory="7"),
+            v7_to_v8.Migration7to8(subdirectory=LATEST_VERSION_DIRECTORY),
         ],
         temp_file_prefix="temp-",
     )

robot-server/robot_server/persistence/tables/__init__.py

@@ -1,7 +1,7 @@
 """SQL database schemas."""
 
 # Re-export the latest schema.
-from .schema_7 import (
+from .schema_8 import (
     metadata,
     protocol_table,
     analysis_table,