diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index a3ac10cd07..7819a45fec 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -1,6 +1,7 @@ from util.json_request import expect_json import json import logging +import sys from collections import defaultdict from django.http import HttpResponse, Http404 @@ -12,6 +13,8 @@ from django.conf import settings from xmodule.modulestore import Location from xmodule.x_module import ModuleSystem +from xmodule.error_module import ErrorDescriptor +from xmodule.errortracker import exc_info_to_str from github_sync import export_to_github from static_replace import replace_urls @@ -288,7 +291,14 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ shared_state: A shared state string """ system = preview_module_system(request, preview_id, descriptor) - module = descriptor.xmodule_constructor(system)(instance_state, shared_state) + try: + module = descriptor.xmodule_constructor(system)(instance_state, shared_state) + except: + module = ErrorDescriptor.from_descriptor( + descriptor, + error_msg=exc_info_to_str(sys.exc_info()) + ).xmodule_constructor(system)(None, None) + module.get_html = replace_static_urls( wrap_xmodule(module.get_html, module, "xmodule_display.html"), module.metadata.get('data_dir', module.location.course) diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index f8e2467910..199fb0d580 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -1,14 +1,10 @@ import hashlib import logging -import random -import string +import json import sys -from pkg_resources import resource_string from lxml import etree from xmodule.x_module import XModule -from xmodule.mako_module import MakoModuleDescriptor -from xmodule.xml_module import XmlDescriptor from xmodule.editing_module import EditingDescriptor from xmodule.errortracker import exc_info_to_str @@ -22,6 +18,7 @@ log = logging.getLogger(__name__) # what to show, and the logic for that belongs in the LMS (e.g. in get_module), so the error handler # decides whether to create a staff or not-staff module. + class ErrorModule(XModule): def get_html(self): '''Show an error to staff. @@ -29,9 +26,9 @@ class ErrorModule(XModule): ''' # staff get to see all the details return self.system.render_template('module-error.html', { - 'staff_access' : True, - 'data' : self.definition['data']['contents'], - 'error' : self.definition['data']['error_msg'], + 'staff_access': True, + 'data': self.definition['data']['contents'], + 'error': self.definition['data']['error_msg'], }) @@ -42,9 +39,9 @@ class NonStaffErrorModule(XModule): ''' # staff get to see all the details return self.system.render_template('module-error.html', { - 'staff_access' : False, - 'data' : "", - 'error' : "", + 'staff_access': False, + 'data': "", + 'error': "", }) @@ -54,6 +51,46 @@ class ErrorDescriptor(EditingDescriptor): """ module_class = ErrorModule + def __init__(self, system, contents, error_msg, org=None, course=None): + + # Pick a unique url_name -- the sha1 hash of the contents. + # NOTE: We could try to pull out the url_name of the errored descriptor, + # but url_names aren't guaranteed to be unique between descriptor types, + # and ErrorDescriptor can wrap any type. When the wrapped module is fixed, + # it will be written out with the original url_name. + url_name = hashlib.sha1(contents).hexdigest() + + definition = { + 'data': { + 'error_msg': str(error_msg), + 'contents': contents, + } + } + + # TODO (vshnayder): Do we need a unique slug here? Just pick a random + # 64-bit num? + location = ['i4x', org, course, 'error', url_name] + + # real metadata stays in the content, but add a display name + metadata = {'display_name': 'Error ' + url_name} + super(ErrorDescriptor, self).__init__( + system, + definition, + location=location, + metadata=metadata + ) + + @classmethod + def from_descriptor(cls, descriptor, error_msg='Error not available'): + return cls( + descriptor.system, + json.dumps({ + 'definition': descriptor.definition, + 'metadata': descriptor.metadata, + }, indent=4), + error_msg + ) + @classmethod def from_xml(cls, xml_data, system, org=None, course=None, error_msg='Error not available'): @@ -65,17 +102,6 @@ class ErrorDescriptor(EditingDescriptor): Takes an extra, optional, parameter--the error that caused an issue. (should be a string, or convert usefully into one). ''' - # Use a nested inner dictionary because 'data' is hardcoded - inner = {} - definition = {'data': inner} - inner['error_msg'] = str(error_msg) - - # Pick a unique url_name -- the sha1 hash of the xml_data. - # NOTE: We could try to pull out the url_name of the errored descriptor, - # but url_names aren't guaranteed to be unique between descriptor types, - # and ErrorDescriptor can wrap any type. When the wrapped module is fixed, - # it will be written out with the original url_name. - url_name = hashlib.sha1(xml_data).hexdigest() try: # If this is already an error tag, don't want to re-wrap it. @@ -84,22 +110,15 @@ class ErrorDescriptor(EditingDescriptor): xml_data = xml_obj.text error_node = xml_obj.find('error_msg') if error_node is not None: - inner['error_msg'] = error_node.text + error_msg = error_node.text else: - inner['error_msg'] = 'Error not available' + error_msg = 'Error not available' except etree.XMLSyntaxError: # Save the error to display later--overrides other problems - inner['error_msg'] = exc_info_to_str(sys.exc_info()) + error_msg = exc_info_to_str(sys.exc_info()) - inner['contents'] = xml_data - # TODO (vshnayder): Do we need a unique slug here? Just pick a random - # 64-bit num? - location = ['i4x', org, course, 'error', url_name] - # real metadata stays in the xml_data, but add a display name - metadata = {'display_name': 'Error ' + url_name} - - return cls(system, definition, location=location, metadata=metadata) + return cls(system, xml_data, error_msg) def export_to_xml(self, resource_fs): ''' @@ -111,8 +130,8 @@ class ErrorDescriptor(EditingDescriptor): files, etc. That would just get re-wrapped on import. ''' try: - xml = etree.fromstring(self.definition['data']['contents']) - return etree.tostring(xml) + xml = etree.fromstring(self.definition['data']['contents']) + return etree.tostring(xml) except etree.XMLSyntaxError: # still not valid. root = etree.Element('error') @@ -121,6 +140,7 @@ class ErrorDescriptor(EditingDescriptor): err_node.text = self.definition['data']['error_msg'] return etree.tostring(root) + class NonStaffErrorDescriptor(ErrorDescriptor): """ Module that provides non-staff error messages.