diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index b942d6726b25b0a539fe6a1173a270ecad45efd2..4d412000eccf54327e483e065939f1c8fd7e2e97 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -89,8 +89,8 @@ def add_histogram(get_html, module): else: edit_link = False - staff_context = {'definition': dict(module.definition), - 'metadata': dict(module.metadata), + staff_context = {'definition': json.dumps(module.definition, indent=4), + 'metadata': json.dumps(module.metadata, indent=4), 'element_id': module.location.html_id(), 'edit_link': edit_link, 'histogram': json.dumps(histogram), diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py index 6a407fe189f52524bebf2d00137ccf727af9a2f5..0d3e2260fb0dfebfa0d2a6e68bae6bbc375ef378 100644 --- a/common/lib/xmodule/tests/test_import.py +++ b/common/lib/xmodule/tests/test_import.py @@ -73,6 +73,7 @@ class ImportTestCase(unittest.TestCase): def test_reimport(self): '''Make sure an already-exported error xml tag loads properly''' + self.maxDiff = None bad_xml = '''<sequential display_name="oops"><video url="hi"></sequential>''' system = self.get_system() descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course', diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 62b3b82a39bcace13cf64cc3c288dd4b26ecb1bd..ecc90873b92c5726f97342556b887e11878afbed 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -1,11 +1,14 @@ +import sys +import logging + 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 -import logging log = logging.getLogger(__name__) @@ -14,14 +17,11 @@ class ErrorModule(XModule): '''Show an error. TODO (vshnayder): proper style, divs, etc. ''' - if not self.system.is_staff: - return self.system.render_template('module-error.html', {}) - # staff get to see all the details - return self.system.render_template('module-error-staff.html', { - 'data' : self.definition['data'], - # TODO (vshnayder): need to get non-syntax errors in here somehow - 'error' : self.definition.get('error', 'Error not available') + return self.system.render_template('module-error.html', { + 'data' : self.definition['data']['contents'], + 'error' : self.definition['data']['error_msg'], + 'is_staff' : self.system.is_staff, }) class ErrorDescriptor(EditingDescriptor): @@ -31,29 +31,36 @@ class ErrorDescriptor(EditingDescriptor): module_class = ErrorModule @classmethod - def from_xml(cls, xml_data, system, org=None, course=None, err=None): + def from_xml(cls, xml_data, system, org=None, course=None, + error_msg='Error not available'): '''Create an instance of this descriptor from the supplied data. Does not try to parse the data--just stores it. Takes an extra, optional, parameter--the error that caused an - issue. + issue. (should be a string, or convert usefully into one). ''' - - definition = {} - if err is not None: - definition['error'] = err + # Use a nested inner dictionary because 'data' is hardcoded + inner = {} + definition = {'data': inner} + inner['error_msg'] = str(error_msg) try: # If this is already an error tag, don't want to re-wrap it. xml_obj = etree.fromstring(xml_data) if xml_obj.tag == 'error': xml_data = xml_obj.text - except etree.XMLSyntaxError as err: + error_node = xml_obj.find('error_msg') + if error_node is not None: + inner['error_msg'] = error_node.text + else: + inner['error_msg'] = 'Error not available' + + except etree.XMLSyntaxError: # Save the error to display later--overrides other problems - definition['error'] = err + inner['error_msg'] = exc_info_to_str(sys.exc_info()) - definition['data'] = xml_data + 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', 'slug'] @@ -71,10 +78,12 @@ class ErrorDescriptor(EditingDescriptor): files, etc. That would just get re-wrapped on import. ''' try: - xml = etree.fromstring(self.definition['data']) + xml = etree.fromstring(self.definition['data']['contents']) return etree.tostring(xml) except etree.XMLSyntaxError: # still not valid. root = etree.Element('error') - root.text = self.definition['data'] + root.text = self.definition['data']['contents'] + err_node = etree.SubElement(root, 'error_msg') + err_node.text = self.definition['data']['error_msg'] return etree.tostring(root) diff --git a/common/lib/xmodule/xmodule/errortracker.py b/common/lib/xmodule/xmodule/errortracker.py index b8d42a6983cb12e98c2dad485c67aa77b9d5ec3f..8ac2903149d52af86532a290392f4412553f478f 100644 --- a/common/lib/xmodule/xmodule/errortracker.py +++ b/common/lib/xmodule/xmodule/errortracker.py @@ -8,6 +8,12 @@ log = logging.getLogger(__name__) ErrorLog = namedtuple('ErrorLog', 'tracker errors') +def exc_info_to_str(exc_info): + """Given some exception info, convert it into a string using + the traceback.format_exception() function. + """ + return ''.join(traceback.format_exception(*exc_info)) + def in_exception_handler(): '''Is there an active exception?''' return sys.exc_info() != (None, None, None) @@ -27,7 +33,7 @@ def make_error_tracker(): '''Log errors''' exc_str = '' if in_exception_handler(): - exc_str = ''.join(traceback.format_exception(*sys.exc_info())) + exc_str = exc_info_to_str(sys.exc_info()) errors.append((msg, exc_str)) diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 360f1b07d02c9d72943fbfee7a49b246ba062d34..ac6b5db5a4cb78887d77419263d2a3714e1cc588 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,12 +1,14 @@ -from lxml import etree -from lxml.etree import XMLSyntaxError -import pkg_resources import logging +import pkg_resources +import sys + from fs.errors import ResourceNotFoundError from functools import partial +from lxml import etree +from lxml.etree import XMLSyntaxError from xmodule.modulestore import Location - +from xmodule.errortracker import exc_info_to_str log = logging.getLogger('mitx.' + __name__) @@ -471,7 +473,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet): msg = "Error loading from xml." log.exception(msg) system.error_tracker(msg) - descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err) + err_msg = msg + "\n" + exc_info_to_str(sys.exc_info()) + descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, + err_msg) return descriptor diff --git a/lms/templates/module-error-staff.html b/lms/templates/module-error-staff.html deleted file mode 100644 index 955f413db37ff03657d6e32730d504c1aad486bd..0000000000000000000000000000000000000000 --- a/lms/templates/module-error-staff.html +++ /dev/null @@ -1,11 +0,0 @@ -<section class="outside-app"> - <h1>There has been an error on the <em>MITx</em> servers</h1> - <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p> - -<h1>Staff-only details below:</h1> - -<p>Error: ${error}</p> - -<p>Raw data: ${data}</p> - -</section> diff --git a/lms/templates/module-error.html b/lms/templates/module-error.html index 28597fa13c9668831ffbea8285f2ae0c7b5b7c66..7c731db17a1cca0afbf89bcb2dd7b61c850dedff 100644 --- a/lms/templates/module-error.html +++ b/lms/templates/module-error.html @@ -1,4 +1,13 @@ <section class="outside-app"> <h1>There has been an error on the <em>MITx</em> servers</h1> <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p> + +% if is_staff: +<h1>Staff-only details below:</h1> + +<p>Error: ${error | h}</p> + +<p>Raw data: ${data | h}</p> +% endif + </section>