Merge pull request #12443 from edx/shr/log/PLAT-1023-PLAT-1031-Detail-Logging

Detail Logging xmodule exceptions

common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py

@@ -230,6 +230,23 @@ class RemapNamespaceTest(ModuleStoreNoSettings):
             'graded', new_version.get_explicitly_set_fields_by_scope(scope=Scope.settings)
         )
 
+    def test_xblock_invalid_field_value_type(self):
+        # Setting the wrong field-value in Xblock-field will raise TypeError.
+        # Example if xblock-field is of 'Dictionary' type by setting the 'List' value in that dict-type will raise
+        # TypeError.
+
+        # Set the XBlock's location
+        self.xblock.location = Location("org", "import", "run", "category", "stubxblock")
+        # Explicitly set the content field
+        self.xblock.test_content_field = ['Explicitly set']
+        self.xblock.save()
+
+        # clearing the dirty fields and removing value from cache will fetch the value from field-data.
+        self.xblock._dirty_fields = {}  # pylint: disable=protected-access
+        self.xblock.fields['test_content_field']._del_cached_value(self.xblock)  # pylint: disable=protected-access
+        with self.assertRaises(TypeError):
+            self.xblock.get_explicitly_set_fields_by_scope(scope=Scope.content)
+
 
 class StubXBlockWithMutableFields(StubXBlock):
     """

common/lib/xmodule/xmodule/tests/test_video.py

@@ -750,6 +750,14 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         expected = '<video url_name="SampleProblem" download_video="false"/>\n'
         self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
 
+    def test_export_to_xml_invalid_characters_in_attributes(self):
+        """
+        Test XML export will raise TypeError by lxml library if contains illegal characters.
+        """
+        self.descriptor.display_name = '\x1e'
+        with self.assertRaises(ValueError):
+            self.descriptor.definition_to_xml(None)
+
 
 class VideoDescriptorIndexingTestCase(unittest.TestCase):
     """

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -561,7 +561,16 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             # is set to its default value, we don't write it out.
             if value:
                 if key in self.fields and self.fields[key].is_set_on(self):
-                    xml.set(key, unicode(value))
+                    try:
+                        xml.set(key, unicode(value))
+                    except ValueError as exception:
+                        exception_message = "{message}, Block-location:{location}, Key:{key}, Value:{value}".format(
+                            message=exception.message,
+                            location=unicode(self.location),
+                            key=key,
+                            value=unicode(value)
+                        )
+                        raise ValueError(exception_message)
 
         for source in self.html5_sources:
             ele = etree.Element('source')

common/lib/xmodule/xmodule/x_module.py

@@ -411,7 +411,15 @@ class XModuleMixin(XModuleFields, XBlock):
         result = {}
         for field in self.fields.values():
             if field.scope == scope and field.is_set_on(self):
-                result[field.name] = field.read_json(self)
+                try:
+                    result[field.name] = field.read_json(self)
+                except TypeError as exception:
+                    exception_message = "{message}, Block-location:{location}, Field-name:{field_name}".format(
+                        message=exception.message,
+                        location=unicode(self.location),
+                        field_name=field.name
+                    )
+                    raise TypeError(exception_message)
         return result
 
     def has_children_at_depth(self, depth):