From efa13ff1d2fb3d8b2ddee8be0868ae60f9bc35a6 Mon Sep 17 00:00:00 2001
From: Julia Eskew <julia.eskew@edx.org>
Date: Wed, 21 Apr 2021 16:13:23 -0400
Subject: [PATCH] fix: TNL-8233: Change exception raised at problem creation
 failure from generic exception to LoncapaProblemError. (#27361)

Raising this specific exception will cause the failure to be handled more gracefully by problem rescoring code.
---
 common/lib/xmodule/xmodule/capa_base.py       |  2 +-
 common/lib/xmodule/xmodule/tests/__init__.py  |  2 +-
 .../xmodule/xmodule/tests/test_capa_module.py | 26 +++++++++++++++++++
 .../tasks_helper/module_state.py              | 13 +++++++++-
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py
index 614bf1f51a..a85e96367f 100644
--- a/common/lib/xmodule/xmodule/capa_base.py
+++ b/common/lib/xmodule/xmodule/capa_base.py
@@ -298,7 +298,7 @@ class CapaMixin(ScorableXBlockMixin, CapaFields):
         except Exception as err:  # pylint: disable=broad-except
             msg = 'cannot create LoncapaProblem {loc}: {err}'.format(
                 loc=str(self.location), err=err)
-            raise Exception(msg).with_traceback(sys.exc_info()[2])
+            raise LoncapaProblemError(msg).with_traceback(sys.exc_info()[2])
 
         if self.score is None:
             self.set_score(self.score_from_lcp(lcp))
diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py
index 5f3639a7be..284d2b8075 100644
--- a/common/lib/xmodule/xmodule/tests/__init__.py
+++ b/common/lib/xmodule/xmodule/tests/__init__.py
@@ -132,7 +132,7 @@ def get_test_system(
         replace_urls=str,
         user=user,
         get_real_user=lambda __: user,
-        filestore=Mock(name='get_test_system.filestore'),
+        filestore=Mock(name='get_test_system.filestore', root_path='.'),
         debug=True,
         hostname="edx.org",
         xqueue={
diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py
index a3351277b3..5fdbae514d 100644
--- a/common/lib/xmodule/xmodule/tests/test_capa_module.py
+++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py
@@ -16,6 +16,7 @@ import pytest
 import ddt
 import requests
 import webob
+from codejail.safe_exec import SafeExecException
 from django.utils.encoding import smart_text
 from edx_user_state_client.interface import XBlockUserState
 from lxml import etree
@@ -1231,6 +1232,31 @@ class ProblemBlockTest(unittest.TestCase):  # lint-amnesty, pylint: disable=miss
             with pytest.raises(NotImplementedError):
                 module.rescore(only_if_higher=False)
 
+    def capa_factory_for_problem_xml(self, xml):  # lint-amnesty, pylint: disable=missing-function-docstring
+        class CustomCapaFactory(CapaFactory):
+            """
+            A factory for creating a Capa problem with arbitrary xml.
+            """
+            sample_problem_xml = textwrap.dedent(xml)
+
+        return CustomCapaFactory
+
+    def test_codejail_error_upon_problem_creation(self):
+        # Simulate a codejail safe_exec failure upon problem creation.
+        # Create a problem with some script attached.
+        xml_str = textwrap.dedent("""
+            <problem>
+                <script>test=True</script>
+            </problem>
+        """)
+        factory = self.capa_factory_for_problem_xml(xml_str)
+
+        # When codejail safe_exec fails upon problem creation, a LoncapaProblemError should be raised.
+        with pytest.raises(LoncapaProblemError):
+            with patch('capa.capa_problem.safe_exec') as mock_safe_exec:
+                mock_safe_exec.side_effect = SafeExecException()
+                factory.create()
+
     def _rescore_problem_error_helper(self, exception_class):
         """Helper to allow testing all errors that rescoring might return."""
         # Create the module
diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py
index 35d4213947..7e7af2f00e 100644
--- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py
+++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py
@@ -170,7 +170,18 @@ def rescore_problem_module_state(xmodule_instance_args, module_descriptor, stude
         # specific events from CAPA are not propagated up the stack. Do we want this?
         try:
             instance.rescore(only_if_higher=task_input['only_if_higher'])
-        except (LoncapaProblemError, StudentInputError, ResponseError):
+        except (LoncapaProblemError, ResponseError):
+            # Capture a backtrace for these errors, but only a warning below for student input errors.
+            TASK_LOG.exception(
+                "error processing rescore call for course %(course)s, problem %(loc)s "
+                "and student %(student)s",
+                dict(
+                    course=course_id,
+                    loc=usage_key,
+                    student=student
+                )
+            )
+        except StudentInputError:
             TASK_LOG.warning(
                 "error processing rescore call for course %(course)s, problem %(loc)s "
                 "and student %(student)s",