From b9e768410f7a996fb87ecf9c50c7337dbe3f84bc Mon Sep 17 00:00:00 2001 From: Peter Pinch Date: Wed, 1 Feb 2017 17:35:03 -0500 Subject: [PATCH] return only error value to students when codejail raises an exception remove comment for translators inst.args[0] is more reliable than inst.message consistent escaping --- common/lib/xmodule/xmodule/capa_base.py | 8 +++-- .../xmodule/xmodule/tests/test_capa_module.py | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 94d71e68c6..b22665b0fb 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -1232,8 +1232,12 @@ class CapaMixin(ScorableXBlockMixin, CapaFields): # Otherwise, display just an error message, # without a stack trace else: - # Translators: {msg} will be replaced with a problem's error message. - msg = inst.message + escaped_message = cgi.escape(inst.args[0]) + try: + # only return the error value of the exception + msg = escaped_message.split("\\n")[-2].split(": ", 1)[1] + except IndexError: + msg = escaped_message return {'success': msg} diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index c32ac3de69..e05c64ea31 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -828,6 +828,42 @@ class CapaModuleTest(unittest.TestCase): # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) + def test_submit_problem_error_with_codejail_exception(self): + + # Try each exception that capa_module should handle + exception_classes = [StudentInputError, + LoncapaProblemError, + ResponseError] + for exception_class in exception_classes: + + # Create the module + module = CapaFactory.create(attempts=1) + + # Ensure that the user is NOT staff + module.system.user_is_staff = False + + # Simulate a codejail exception 'Exception: test error' + with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade: + try: + raise ResponseError( + 'Couldn\'t execute jailed code: stdout: \'\', ' + 'stderr: \'Traceback (most recent call last):\\n' + ' File "jailed_code", line 15, in \\n' + ' exec code in g_dict\\n File "", line 67, in \\n' + ' File "", line 65, in check_func\\n' + 'Exception: test error\\n\' with status code: 1',) + except ResponseError as err: + mock_grade.side_effect = exception_class(err.message) + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.submit_problem(get_request_dict) + + # Expect an AJAX alert message in 'success' without the text of the stack trace + expected_msg = 'test error' + self.assertEqual(expected_msg, result['success']) + + # Expect that the number of attempts is NOT incremented + self.assertEqual(module.attempts, 1) + def test_submit_problem_other_errors(self): """ Test that errors other than the expected kinds give an appropriate message.