Skip to content

Commit

Permalink
fix: TNL-8233: Change exception raised at problem creation failure fr…
Browse files Browse the repository at this point in the history
…om generic exception to LoncapaProblemError. (#27361)

Raising this specific exception will cause the failure to be handled more gracefully by problem rescoring code.
  • Loading branch information
Julia Eskew authored Apr 21, 2021
1 parent a1fe3d5 commit efa13ff
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/capa_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def lcp(self): # lint-amnesty, pylint: disable=method-hidden, missing-function-
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))
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def get_module(descriptor):
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={
Expand Down
26 changes: 26 additions & 0 deletions common/lib/xmodule/xmodule/tests/test_capa_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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
Expand Down Expand Up @@ -1231,6 +1232,31 @@ def test_rescore_problem_not_supported(self):
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
Expand Down
13 changes: 12 additions & 1 deletion lms/djangoapps/instructor_task/tasks_helper/module_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit efa13ff

Please sign in to comment.