From bdd91daeb6038deb26852d8e71392c3db121ec7f Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Wed, 3 Sep 2014 11:23:38 -0400 Subject: [PATCH] Include stack trace in Course Rerun errors; Log errors. --- cms/djangoapps/contentstore/tasks.py | 7 +++++-- .../contentstore/tests/test_contentstore.py | 12 ++++++++++++ common/djangoapps/course_action_state/managers.py | 7 ++++--- .../tests/test_rerun_manager.py | 15 ++++++++++----- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 8b05565eb8..df7ada6585 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -5,6 +5,7 @@ This file contains celery tasks for contentstore views from celery.task import task from django.contrib.auth.models import User import json +import logging from xmodule.modulestore.django import modulestore from xmodule.course_module import CourseFields @@ -40,13 +41,15 @@ def rerun_course(source_course_key_string, destination_course_key_string, user_i except DuplicateCourseError as exc: # do NOT delete the original course, only update the status - CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc) + CourseRerunState.objects.failed(course_key=destination_course_key) + logging.exception(u'Course Rerun Error') return "duplicate course" # catch all exceptions so we can update the state and properly cleanup the course. except Exception as exc: # pylint: disable=broad-except # update state: Failed - CourseRerunState.objects.failed(course_key=destination_course_key, exception=exc) + CourseRerunState.objects.failed(course_key=destination_course_key) + logging.exception(u'Course Rerun Error') try: # cleanup any remnants of the course diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index aeb0903632..ab8c172814 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1699,6 +1699,18 @@ class RerunCourseTest(ContentStoreTestCase): self.user.save() self.post_rerun_request(source_course.id, response_code=403, expect_error=True) + def test_rerun_error(self): + error_message = "Mock Error Message" + with mock.patch( + 'xmodule.modulestore.mixed.MixedModuleStore.clone_course', + mock.Mock(side_effect=Exception(error_message)) + ): + source_course = CourseFactory.create() + 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.assertIn(error_message, rerun_state.message) + class EntryPageTestCase(TestCase): """ diff --git a/common/djangoapps/course_action_state/managers.py b/common/djangoapps/course_action_state/managers.py index 661f417a69..1756c594d7 100644 --- a/common/djangoapps/course_action_state/managers.py +++ b/common/djangoapps/course_action_state/managers.py @@ -1,6 +1,7 @@ """ Model Managers for Course Actions """ +import traceback from django.db import models, transaction @@ -135,14 +136,14 @@ class CourseRerunUIStateManager(CourseActionUIStateManager): new_state=self.State.SUCCEEDED, ) - def failed(self, course_key, exception): + def failed(self, course_key): """ - To be called when an existing rerun for the given course has failed with the given exception. + To be called within an exception handler when an existing rerun for the given course has failed. """ self.update_state( course_key=course_key, new_state=self.State.FAILED, - message=exception.message, + message=traceback.format_exc(), ) diff --git a/common/djangoapps/course_action_state/tests/test_rerun_manager.py b/common/djangoapps/course_action_state/tests/test_rerun_manager.py index e94a6a1cb2..8c67da3a9b 100644 --- a/common/djangoapps/course_action_state/tests/test_rerun_manager.py +++ b/common/djangoapps/course_action_state/tests/test_rerun_manager.py @@ -91,12 +91,17 @@ class TestCourseRerunStateManager(TestCase): # set state to fail exception = Exception("failure in rerunning") - CourseRerunState.objects.failed(course_key=self.course_key, exception=exception) - self.expected_rerun_state.update({ - 'state': CourseRerunUIStateManager.State.FAILED, - 'message': exception.message, - }) + try: + raise exception + except: + CourseRerunState.objects.failed(course_key=self.course_key) + + self.expected_rerun_state.update( + {'state': CourseRerunUIStateManager.State.FAILED} + ) + self.expected_rerun_state.pop('message') rerun = self.verify_rerun_state() + self.assertIn(exception.message, rerun.message) # dismiss ui and verify self.dismiss_ui_and_verify(rerun)