Display an error descriptor for a preview when a descriptor can be loaded but a module can't

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)

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.