From d83e6c4d3dbb4ac9d58c60551af78d3e07f7ab4e Mon Sep 17 00:00:00 2001
From: Omar Al-Ithawi <i@omardo.com>
Date: Tue, 25 Jan 2022 13:28:52 +0300
Subject: [PATCH] RED-2483: graceful error handling for `migrate_transcripts`
 command

 - a course will either be successful or will fail in one atomic SQL
   transaction.
 - a course failure will be logged, then continue for next course
 - ItemNotFoundError will be ignored seperately from other errors since
   it's usually means that we have some other problem that
   the `migrate_transcripts` command shouldn't worry about
---
 .../tests/test_migrate_transcripts.py         | 35 +++++++++++++++++++
 cms/djangoapps/contentstore/tasks.py          | 19 +++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py b/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py
index d9db0c77bc5b..341a69422b5e 100644
--- a/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py
+++ b/cms/djangoapps/contentstore/management/commands/tests/test_migrate_transcripts.py
@@ -23,6 +23,8 @@
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.video_module import VideoBlock
 from xmodule.video_module.transcripts_utils import save_to_store
+from xmodule.modulestore.exceptions import ItemNotFoundError
+
 
 LOGGER_NAME = "cms.djangoapps.contentstore.tasks"
 
@@ -311,6 +313,39 @@ def test_migrate_transcripts_exception_logging(self):
                 *expected_log
             )
 
+    @ddt.unpack
+    @ddt.data({
+        'exception_class': ItemNotFoundError,
+        'log_message': 'course-transcript-migration-failed-with-not-found-error-exc',
+        'log_type': 'INFO',
+    }, {
+        'exception_class': ValueError,  # Some random exception
+        'log_message': 'course-transcript-migration-failed-with-unknown-exc',
+        'log_type': 'ERROR',
+    })
+    @patch('cms.djangoapps.contentstore.tasks.async_migrate_transcript')
+    def test_tahoe_migrate_additional_error_handling(self, mock_migrate, exception_class, log_message, log_type):
+        """
+        Tahoe: Test migrate transcripts custom exception handling for ItemNotFoundError and other errors.
+        """
+        exception = exception_class('Test exception')
+        mock_migrate.side_effect = exception
+        course_id = six.text_type(self.course_2.id)
+        expected_log = (
+            (
+                'cms.djangoapps.contentstore.tasks', log_type,
+                (u'[Transcript Migration] [run=1] [{}] [course={}] [exc={}]'.format(
+                    log_message, course_id, str(exception)
+                ))
+            ),
+        )
+
+        with LogCapture(LOGGER_NAME, level=logging.INFO) as logger:
+            call_command('migrate_transcripts', '--course-id', six.text_type(self.course_2.id), '--commit')
+            logger.check(
+                *expected_log
+            )
+
     @ddt.data(*itertools.product([1, 2], [True, False], [True, False]))
     @ddt.unpack
     @patch('contentstore.management.commands.migrate_transcripts.log')
diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py
index 8460e7622bbd..636c3f22e2b9 100644
--- a/cms/djangoapps/contentstore/tasks.py
+++ b/cms/djangoapps/contentstore/tasks.py
@@ -108,8 +108,25 @@ def enqueue_async_migrate_transcripts_tasks(course_keys,
         'command_run': command_run
     }
 
+    # TODO: Undo all the migrate_transcripts edits: https://github.com/appsembler/edx-platform/issues/1068
+    # temp import, kept here to reduce migration conflicts
+    from django.db import transaction
+    from xmodule.modulestore.exceptions import ItemNotFoundError
+
     for course_key in course_keys:
-        async_migrate_transcript(text_type(course_key), **kwargs)
+        try:
+            with transaction.atomic():
+                async_migrate_transcript(text_type(course_key), **kwargs)
+        except ItemNotFoundError as not_found_exc:
+            LOGGER.info(
+                '[%s] [run=%s] [course-transcript-migration-failed-with-not-found-error-exc] [course=%s] [exc=%s]',
+                MIGRATION_LOGS_PREFIX, command_run, course_key, str(not_found_exc)
+            )
+        except Exception as exc:
+            LOGGER.exception(
+                '[%s] [run=%s] [course-transcript-migration-failed-with-unknown-exc] [course=%s] [exc=%s]',
+                MIGRATION_LOGS_PREFIX, command_run, course_key, str(exc)
+            )
 
 
 def get_course_videos(course_key):