Keep task errors when exception details exceed the database column

coriolis/db/api.py

@@ -898,11 +898,62 @@ def _get_task(context, task_id):
     return task
 
 
+def _fit_exception_details(task_id, exception_details):
+    """Fit exception details into the task.exception_details column.
+
+    Command output embedded in a task error, such as a yum progress bar,
+    can exceed the MySQL TEXT limit. The status UPDATE then fails and the
+    task stays in a non-terminal state. The beginning and the end are kept:
+    Coriolis describes the failure first, and the underlying command reports
+    the actual error last.
+    """
+    if not exception_details:
+        return exception_details
+
+    max_length = models.MAX_EXCEPTION_DETAILS_LENGTH
+    original_length = len(exception_details)
+    if original_length <= max_length:
+        return exception_details
+
+    LOG.warn(
+        f"Exception details for task '{task_id}' are too long "
+        f"({original_length} characters) to store in the database. "
+        f"Truncating to {max_length} characters so the task status can "
+        f"be recorded."
+    )
+
+    # The marker states how many characters were removed, and that count
+    # depends on the marker's own length. Grow the omitted count until the
+    # marker and the kept text fit together.
+    omitted = original_length - max_length
+    while True:
+        marker = f"\n... ({omitted} characters truncated) ...\n"
+        keep = max_length - len(marker)
+        if keep < 1:
+            return exception_details[:max_length]
+        required_omitted = original_length - keep
+        if omitted >= required_omitted:
+            break
+        omitted = required_omitted
+
+    head_length = keep // 2
+    tail_length = keep - head_length
+    return (
+        f"{exception_details[:head_length]}{marker}{exception_details[-tail_length:]}"
+    )
+
+
 @enginefacade.writer
 def set_task_status(context, task_id, status, exception_details=None):
+    """Set a task's status and, when given, its exception details.
+
+    Exception details are shortened to fit the database column. A value
+    that exceeds the column makes the status update fail and leaves the
+    task stuck.
+    """
     task = _get_task(context, task_id)
     task.status = status
-    task.exception_details = exception_details
+    task.exception_details = _fit_exception_details(task_id, exception_details)
 
 
 @enginefacade.writer

coriolis/db/sqlalchemy/models.py

@@ -15,6 +15,11 @@
 
 MAX_EVENT_MESSAGE_LENGHT = 1024
 
+# MySQL TEXT holds at most 65,535 bytes. utf8mb4 can use 4 bytes per
+# character, so this is the longest exception_details value guaranteed
+# to fit in the column.
+MAX_EXCEPTION_DETAILS_LENGTH = 65535 // 4
+
 
 class TaskEvent(BASE, models.TimestampMixin, models.SoftDeleteMixin, models.ModelBase):
     __tablename__ = 'task_event'

coriolis/tests/db/test_api.py

@@ -880,6 +880,83 @@ def test_set_execution_status_update_action_status(self):
         )
         self.assertEqual(self.valid_transfer.last_execution_status, new_status)
 
+    def test_set_task_status_stores_short_exception_details(self):
+        task = self._new_task()
+        details = "Failed to install required packages."
+
+        api.set_task_status(self.context, task.id, constants.TASK_STATUS_ERROR, details)
+
+        stored = api.get_task(self.context, task.id)
+        self.assertEqual(constants.TASK_STATUS_ERROR, stored.status)
+        self.assertEqual(details, stored.exception_details)
+
+    def test_set_task_status_records_status_when_exception_details_are_too_long(self):
+        task = self._new_task()
+        max_length = models.MAX_EXCEPTION_DETAILS_LENGTH
+        details = "start-" + ("x" * (max_length * 3)) + "-end"
+
+        api.set_task_status(self.context, task.id, constants.TASK_STATUS_ERROR, details)
+
+        stored = api.get_task(self.context, task.id)
+        self.assertEqual(constants.TASK_STATUS_ERROR, stored.status)
+        self.assertLessEqual(len(stored.exception_details), max_length)
+        self.assertLessEqual(len(stored.exception_details.encode("utf-8")), 65535)
+        self.assertTrue(stored.exception_details.startswith("start-"))
+        self.assertTrue(stored.exception_details.endswith("-end"))
+        self.assertIn("characters truncated", stored.exception_details)
+
+    def _new_task(self):
+        task = models.Task()
+        task.id = str(uuid.uuid4())
+        task.execution = self.valid_tasks_execution
+        task.instance = DEFAULT_INSTANCE
+        task.status = constants.TASK_STATUS_RUNNING
+        task.task_type = constants.TASK_TYPE_VALIDATE_TRANSFER_SOURCE_INPUTS
+        task.index = 1000 + len(self.valid_tasks_execution.tasks)
+        task.on_error = False
+        self.session.add(task)
+        self.session.commit()
+        return task
+
+
+class ExceptionDetailsFitTestCase(test_base.CoriolisBaseTestCase):
+    def test_fit_exception_details_leaves_short_values_unchanged(self):
+        self.assertIsNone(api._fit_exception_details("task-id", None))
+        self.assertEqual("", api._fit_exception_details("task-id", ""))
+        details = "a" * models.MAX_EXCEPTION_DETAILS_LENGTH
+        self.assertEqual(details, api._fit_exception_details("task-id", details))
+
+    @mock.patch.object(api.models, "MAX_EXCEPTION_DETAILS_LENGTH", 80)
+    def test_fit_exception_details_keeps_head_and_tail(self):
+        details = ("HEAD-" * 20) + ("x" * 200) + ("-TAIL" * 20)
+
+        fitted = api._fit_exception_details("task-id", details)
+
+        self.assertLessEqual(len(fitted), 80)
+        self.assertTrue(fitted.startswith("HEAD-"))
+        self.assertTrue(fitted.endswith("-TAIL"))
+        marker_start = fitted.index("\n... (")
+        marker_end = fitted.index(") ...\n") + len(") ...\n")
+        marker = fitted[marker_start:marker_end]
+        head, tail = fitted.split(marker)
+        self.assertEqual(details[: len(head)], head)
+        self.assertEqual(details[-len(tail) :], tail)
+        omitted = len(details) - len(head) - len(tail)
+        self.assertIn(f"({omitted} characters truncated)", marker)
+        self.assertLessEqual(len(fitted.encode("utf-8")), 65535)
+
+    def test_fit_exception_details_stays_within_mysql_text_for_multibyte_text(self):
+        # U+10000 encodes to 4 bytes in UTF-8, the utf8mb4 worst case.
+        four_byte_char = "\U00010000"
+        details = four_byte_char * (models.MAX_EXCEPTION_DETAILS_LENGTH + 500)
+
+        fitted = api._fit_exception_details("task-id", details)
+
+        self.assertLessEqual(len(fitted), models.MAX_EXCEPTION_DETAILS_LENGTH)
+        self.assertLessEqual(len(fitted.encode("utf-8")), 65535)
+        self.assertTrue(fitted.startswith(four_byte_char))
+        self.assertTrue(fitted.endswith(four_byte_char))
+
 
 class TransferSchedulesDBAPITestCase(BaseDBAPITestCase):
     @classmethod