From ace2bffae63bb53f95a306c99c6c78c3f7b1bf9b Mon Sep 17 00:00:00 2001 From: jsa Date: Wed, 21 Jan 2015 09:21:54 -0500 Subject: [PATCH] avoid database error when recording task exception --- .../contentstore/tests/test_contentstore.py | 20 +++++++++++++++++++ .../course_action_state/managers.py | 2 +- .../djangoapps/course_action_state/models.py | 6 +++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index f831edff01..b7f0894774 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1721,6 +1721,26 @@ class RerunCourseTest(ContentStoreTestCase): self.assertEquals(rerun_state.state, CourseRerunUIStateManager.State.FAILED) self.assertIn(error_message, rerun_state.message) + def test_rerun_error_trunc_message(self): + """ + CourseActionUIState.message is sometimes populated with the contents + of Python tracebacks. This test ensures we don't crash when attempting + to insert a value exceeding its max_length (note that sqlite does not + complain if this happens, but MySQL throws an error). + """ + with mock.patch( + 'xmodule.modulestore.mixed.MixedModuleStore.clone_course', + mock.Mock(side_effect=Exception()), + ): + source_course = CourseFactory.create() + message_too_long = "traceback".rjust(CourseRerunState.MAX_MESSAGE_LENGTH * 2, '-') + with mock.patch('traceback.format_exc', return_value=message_too_long): + destination_course_key = self.post_rerun_request(source_course.id) + rerun_state = CourseRerunState.objects.find_first(course_key=destination_course_key) + self.assertEquals(rerun_state.state, CourseRerunUIStateManager.State.FAILED) + self.assertTrue(rerun_state.message.endswith("traceback")) + self.assertEqual(len(rerun_state.message), CourseRerunState.MAX_MESSAGE_LENGTH) + class EntryPageTestCase(TestCase): """ diff --git a/common/djangoapps/course_action_state/managers.py b/common/djangoapps/course_action_state/managers.py index 1756c594d7..96f82300e7 100644 --- a/common/djangoapps/course_action_state/managers.py +++ b/common/djangoapps/course_action_state/managers.py @@ -143,7 +143,7 @@ class CourseRerunUIStateManager(CourseActionUIStateManager): self.update_state( course_key=course_key, new_state=self.State.FAILED, - message=traceback.format_exc(), + message=traceback.format_exc()[-self.model.MAX_MESSAGE_LENGTH:], # truncate to fit ) diff --git a/common/djangoapps/course_action_state/models.py b/common/djangoapps/course_action_state/models.py index 48ca83f47f..61aaf9e263 100644 --- a/common/djangoapps/course_action_state/models.py +++ b/common/djangoapps/course_action_state/models.py @@ -83,13 +83,17 @@ class CourseActionUIState(CourseActionState): """ abstract = True + # WARNING - when you edit this value, you're also modifying the max_length + # of the `message` column (see below) + MAX_MESSAGE_LENGTH = 1000 + # FIELDS # Whether or not the status should be displayed to users should_display = models.BooleanField() # Message related to the status - message = models.CharField(max_length=1000) + message = models.CharField(max_length=MAX_MESSAGE_LENGTH) # Rerun courses also need these fields. All rerun course actions will have a row here as well.