diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index e79399c5fc..bc8e7ff541 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -53,12 +53,17 @@ class LoncapaProblemError(Exception): class ResponseError(Exception): ''' - Error for failure in processing a response + Error for failure in processing a response, including + exceptions that occur when executing a custom script. ''' pass class StudentInputError(Exception): + ''' + Error for an invalid student input. + For example, submitting a string when the problem expects a number + ''' pass #----------------------------------------------------------------------------- @@ -1151,7 +1156,7 @@ def sympy_check2(): # Raise an exception else: log.error(traceback.format_exc()) - raise LoncapaProblemError( + raise ResponseError( "CustomResponse: check function returned an invalid dict") # The check function can return a boolean value, @@ -1226,7 +1231,7 @@ def sympy_check2(): Handle an exception raised during the execution of custom Python code. - Raises a StudentInputError + Raises a ResponseError ''' # Log the error if we are debugging @@ -1236,7 +1241,7 @@ def sympy_check2(): # Notify student with a student input error _, _, traceback_obj = sys.exc_info() - raise StudentInputError, StudentInputError(err.message), traceback_obj + raise ResponseError, ResponseError(err.message), traceback_obj #----------------------------------------------------------------------------- @@ -1912,7 +1917,14 @@ class SchematicResponse(LoncapaResponse): submission = [json.loads(student_answers[ k]) for k in sorted(self.answer_ids)] self.context.update({'submission': submission}) - exec self.code in global_context, self.context + + try: + exec self.code in global_context, self.context + + except Exception as err: + _, _, traceback_obj = sys.exc_info() + raise ResponseError, ResponseError(err.message), traceback_obj + cmap = CorrectMap() cmap.set_dict(dict(zip(sorted( self.answer_ids), self.context['correct']))) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index d7346faa67..fd25016ca5 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -12,12 +12,13 @@ from lxml import etree from pkg_resources import resource_string from capa.capa_problem import LoncapaProblem -from capa.responsetypes import StudentInputError +from capa.responsetypes import StudentInputError, \ + ResponseError, LoncapaProblemError from capa.util import convert_files_to_filenames from .progress import Progress from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor -from xmodule.exceptions import NotFoundError +from xmodule.exceptions import NotFoundError, ProcessingError from xblock.core import Integer, Scope, BlockScope, ModelType, String, Boolean, Object, Float from .fields import Timedelta @@ -454,7 +455,14 @@ class CapaModule(CapaFields, XModule): return 'Error' before = self.get_progress() - d = handlers[dispatch](get) + + try: + d = handlers[dispatch](get) + + except Exception as err: + _, _, traceback_obj = sys.exc_info() + raise ProcessingError, ProcessingError(err.message), traceback_obj + after = self.get_progress() d.update({ 'progress_changed': after != before, @@ -726,7 +734,7 @@ class CapaModule(CapaFields, XModule): correct_map = self.lcp.grade_answers(answers) self.set_state_from_lcp() - except StudentInputError as inst: + except (StudentInputError, ResponseError, LoncapaProblemError) as inst: log.exception("StudentInputError in capa_module:problem_check") # If the user is a staff member, include diff --git a/common/lib/xmodule/xmodule/exceptions.py b/common/lib/xmodule/xmodule/exceptions.py index 3db5ceccde..d38fbb12bb 100644 --- a/common/lib/xmodule/xmodule/exceptions.py +++ b/common/lib/xmodule/xmodule/exceptions.py @@ -1,6 +1,12 @@ class InvalidDefinitionError(Exception): pass - class NotFoundError(Exception): pass + +class ProcessingError(Exception): + ''' + An error occurred while processing a request to the XModule. + For example: if an exception occurs while checking a capa problem. + ''' + pass diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 18d20a2756..cb7d599413 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -7,6 +7,8 @@ import random import xmodule import capa +from capa.responsetypes import StudentInputError, \ + LoncapaProblemError, ResponseError from xmodule.capa_module import CapaModule from xmodule.modulestore import Location from lxml import etree @@ -502,38 +504,52 @@ class CapaModuleTest(unittest.TestCase): self.assertEqual(module.attempts, 1) - def test_check_problem_student_input_error(self): - module = CapaFactory.create(attempts=1) + def test_check_problem_error(self): - # Ensure that the user is NOT staff - module.system.user_is_staff = False + # Try each exception that capa_module should handle + for exception_class in [StudentInputError, + LoncapaProblemError, + ResponseError]: - # Simulate a student input exception - with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: - mock_grade.side_effect = capa.responsetypes.StudentInputError('test error') + # Create the module + module = CapaFactory.create(attempts=1) - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.check_problem(get_request_dict) + # Ensure that the user is NOT staff + module.system.user_is_staff = False + + # Simulate answering a problem that raises the exception + with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + mock_grade.side_effect = exception_class('test error') + + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.check_problem(get_request_dict) # Expect an AJAX alert message in 'success' expected_msg = 'Error: test error' self.assertEqual(expected_msg, result['success']) - # Expect that the number of attempts is NOT incremented - self.assertEqual(module.attempts, 1) + # Expect that the number of attempts is NOT incremented + self.assertEqual(module.attempts, 1) - def test_check_problem_student_input_error_with_staff_user(self): - module = CapaFactory.create(attempts=1) + def test_check_problem_error_with_staff_user(self): + + # Try each exception that capa module should handle + for exception_class in [StudentInputError, + LoncapaProblemError, + ResponseError]: - # Ensure that the user IS staff - module.system.user_is_staff = True + # Create the module + module = CapaFactory.create(attempts=1) - # Simulate a student input exception - with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: - mock_grade.side_effect = capa.responsetypes.StudentInputError('test error') + # Ensure that the user IS staff + module.system.user_is_staff = True - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.check_problem(get_request_dict) + # Simulate answering a problem that raises an exception + with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + mock_grade.side_effect = exception_class('test error') + + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.check_problem(get_request_dict) # Expect an AJAX alert message in 'success' self.assertTrue('test error' in result['success']) @@ -541,6 +557,10 @@ class CapaModuleTest(unittest.TestCase): # We DO include traceback information for staff users self.assertTrue('Traceback' in result['success']) + # Expect that the number of attempts is NOT incremented + self.assertEqual(module.attempts, 1) + + def test_reset_problem(self): module = CapaFactory.create(done=True) module.new_lcp = Mock(wraps=module.new_lcp) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 973940d784..182c45775d 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -22,7 +22,7 @@ from .models import StudentModule from psychometrics.psychoanalyze import make_psychometrics_data_update_handler from student.models import unique_id_for_user from xmodule.errortracker import exc_info_to_str -from xmodule.exceptions import NotFoundError +from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.x_module import ModuleSystem @@ -443,9 +443,18 @@ def modx_dispatch(request, dispatch, location, course_id): # Let the module handle the AJAX try: ajax_return = instance.handle_ajax(dispatch, p) + + # If we can't find the module, respond with a 404 except NotFoundError: log.exception("Module indicating to user that request doesn't exist") raise Http404 + + # For XModule-specific errors, we respond with 404 + except ProcessingError: + log.exception("Module encountered an error while prcessing AJAX call") + raise Http404 + + # If any other error occurred, re-raise it to trigger a 500 response except: log.exception("error processing ajax call") raise