Merge pull request #2816 from edx/usman/lms1192-module-render-exceptions

Catch module render exceptions

AUTHORS

@@ -108,7 +108,7 @@ Cristian Salamea <cristian.salamea@iaen.edu.ec>
 Graham Lowe <graham.lowe@gmail.com>
 Matt Bachmann <bachmann.matt@gmail.com>
 Dave St.Germain <dstgermain@edx.org>
-Usman Khalid <symbolist@users.noreply.github.com>
+Usman Khalid <2200617@gmail.com>
 John Kern <kern3020@gmail.com>
 John Orr <jorr@google.com>
 Mark Hoeber <hoeber@edx.org>

common/test/data/toy/about/short_description.html

+A course about toys.
\ No newline at end of file

lms/djangoapps/courseware/courses.py

@@ -6,15 +6,18 @@ import inspect
 from path import path
 from django.http import Http404
 from django.conf import settings
-from .module_render import get_module
+
+from edxmako.shortcuts import render_to_string
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore import Location, XML_MODULESTORE_TYPE
 from xmodule.modulestore.django import modulestore, loc_mapper
 from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
-from courseware.model_data import FieldDataCache
 from static_replace import replace_static_urls
+
 from courseware.access import has_access
+from courseware.model_data import FieldDataCache
+from courseware.module_render import get_module
 import branding
 
 log = logging.getLogger(__name__)
@@ -184,8 +187,14 @@ def get_course_about_section(course, section_key):
             html = ''
 
             if about_module is not None:
-                html = about_module.render('student_view').content
-
+                try:
+                    html = about_module.render('student_view').content
+                except Exception:  # pylint: disable=broad-except
+                    html = render_to_string('courseware/error-message.html', None)
+                    log.exception("Error rendering course={course}, section_key={section_key}".format(
+                        course=course,
+                        section_key=section_key
+                    ))
             return html
 
         except ItemNotFoundError:
@@ -230,7 +239,14 @@ def get_course_info_section(request, course, section_key):
     html = ''
 
     if info_module is not None:
-        html = info_module.render('student_view').content
+        try:
+            html = info_module.render('student_view').content
+        except Exception:  # pylint: disable=broad-except
+            html = render_to_string('courseware/error-message.html', None)
+            log.exception("Error rendering course={course}, section_key={section_key}".format(
+                course=course,
+                section_key=section_key
+            ))
 
     return html
 

lms/djangoapps/courseware/tabs.py

@@ -15,14 +15,13 @@ import logging
 from django.conf import settings
 from django.core.urlresolvers import reverse
 
-from courseware.access import has_access
-
-from .module_render import get_module
-from courseware.access import has_access
+from edxmako.shortcuts import render_to_string
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
-from courseware.model_data import FieldDataCache
 
+from courseware.access import has_access
+from courseware.model_data import FieldDataCache
+from courseware.module_render import get_module
 from open_ended_grading import open_ended_notifications
 
 import waffle
@@ -443,6 +442,13 @@ def get_static_tab_contents(request, course, tab):
     html = ''
 
     if tab_module is not None:
-        html = tab_module.render('student_view').content
+        try:
+            html = tab_module.render('student_view').content
+        except Exception:  # pylint: disable=broad-except
+            html = render_to_string('courseware/error-message.html', None)
+            log.exception("Error rendering course={course}, tab={tab_url}".format(
+                course=course,
+                tab_url=tab['url_slug']
+            ))
 
     return html

lms/djangoapps/courseware/tests/helpers.py

@@ -2,6 +2,7 @@ import json
 
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
+from django.test.client import RequestFactory
 
 from student.models import Registration
 
@@ -9,38 +10,49 @@ from django.test import TestCase
 
 
 def check_for_get_code(self, code, url):
-        """
-        Check that we got the expected code when accessing url via GET.
-        Returns the HTTP response.
+    """
+    Check that we got the expected code when accessing url via GET.
+    Returns the HTTP response.
 
-        `self` is a class that subclasses TestCase.
+    `self` is a class that subclasses TestCase.
 
-        `code` is a status code for HTTP responses.
+    `code` is a status code for HTTP responses.
 
-        `url` is a url pattern for which we have to test the response.
-        """
-        resp = self.client.get(url)
-        self.assertEqual(resp.status_code, code,
-                         "got code %d for url '%s'. Expected code %d"
-                         % (resp.status_code, url, code))
-        return resp
+    `url` is a url pattern for which we have to test the response.
+    """
+    resp = self.client.get(url)
+    self.assertEqual(resp.status_code, code,
+                     "got code %d for url '%s'. Expected code %d"
+                     % (resp.status_code, url, code))
+    return resp
 
 
 def check_for_post_code(self, code, url, data={}):
-        """
-        Check that we got the expected code when accessing url via POST.
-        Returns the HTTP response.
-        `self` is a class that subclasses TestCase.
+    """
+    Check that we got the expected code when accessing url via POST.
+    Returns the HTTP response.
+    `self` is a class that subclasses TestCase.
 
-        `code` is a status code for HTTP responses.
+    `code` is a status code for HTTP responses.
 
-        `url` is a url pattern for which we want to test the response.
-        """
-        resp = self.client.post(url, data)
-        self.assertEqual(resp.status_code, code,
-                         "got code %d for url '%s'. Expected code %d"
-                         % (resp.status_code, url, code))
-        return resp
+    `url` is a url pattern for which we want to test the response.
+    """
+    resp = self.client.post(url, data)
+    self.assertEqual(resp.status_code, code,
+                     "got code %d for url '%s'. Expected code %d"
+                     % (resp.status_code, url, code))
+    return resp
+
+
+def get_request_for_user(user):
+    """Create a request object for user."""
+
+    request = RequestFactory()
+    request.user = user
+    request.META = {}
+    request.is_secure = lambda: True
+    request.get_host = lambda: "edx.org"
+    return request
 
 
 class LoginEnrollmentTestCase(TestCase):

lms/djangoapps/courseware/tests/test_courses.py

@@ -6,14 +6,19 @@ import mock
 
 from django.http import Http404
 from django.test.utils import override_settings
+from student.tests.factories import UserFactory
 from xmodule.modulestore.django import get_default_store_name_for_current_request
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.tests.xml import factories as xml
 from xmodule.tests.xml import XModuleXmlImportTest
 
-from courseware.courses import get_course_by_id, get_course, get_cms_course_link, course_image_url
-from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
+from courseware.courses import (
+    get_course_by_id, get_course, get_cms_course_link, course_image_url,
+    get_course_info_section, get_course_about_section
+)
+from courseware.tests.helpers import get_request_for_user
+from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE, TEST_DATA_MIXED_MODULESTORE
 
 
 CMS_BASE_TEST = 'testcms'
@@ -135,3 +140,43 @@ class XmlCourseImageTestCase(XModuleXmlImportTest):
         # XML Course images are always stored at /images/course_image.jpg
         course = self.process_xml(xml.CourseFactory.build(course_image=u'before after.jpg'))
         self.assertEquals(course_image_url(course), '/static/xml_test_course/images/course_image.jpg')
+
+
+class CoursesRenderTest(ModuleStoreTestCase):
+    """Test methods related to rendering courses content."""
+
+    @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+    def test_get_course_info_section_render(self):
+        course = get_course_by_id('edX/toy/2012_Fall')
+        request = get_request_for_user(UserFactory.create())
+
+        # Test render works okay
+        course_info = get_course_info_section(request, course, 'handouts')
+        self.assertEqual(course_info, "<a href='/static/toy/handouts/sample_handout.txt'>Sample</a>")
+
+        # Test when render raises an exception
+        with mock.patch('courseware.courses.get_module') as mock_module_render:
+            mock_module_render.return_value = mock.MagicMock(
+                render=mock.Mock(side_effect=Exception('Render failed!'))
+            )
+            course_info = get_course_info_section(request, course, 'handouts')
+            self.assertIn("this module is temporarily unavailable", course_info)
+
+    @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+    @mock.patch('courseware.courses.get_request_for_thread')
+    def test_get_course_about_section_render(self, mock_get_request):
+        course = get_course_by_id('edX/toy/2012_Fall')
+        request = get_request_for_user(UserFactory.create())
+        mock_get_request.return_value = request
+
+        # Test render works okay
+        course_about = get_course_about_section(course, 'short_description')
+        self.assertEqual(course_about, "A course about toys.")
+
+        # Test when render raises an exception
+        with mock.patch('courseware.courses.get_module') as mock_module_render:
+            mock_module_render.return_value = mock.MagicMock(
+                render=mock.Mock(side_effect=Exception('Render failed!'))
+            )
+            course_about = get_course_about_section(course, 'short_description')
+            self.assertIn("this module is temporarily unavailable", course_about)

lms/djangoapps/courseware/tests/test_tabs.py

 from django.test import TestCase
-from mock import MagicMock
-from mock import patch
+from mock import MagicMock, Mock, patch
 
-import courseware.tabs as tabs
+from courseware import tabs
+from courseware.courses import get_course_by_id
 
 from django.test.utils import override_settings
 from django.core.urlresolvers import reverse
 
+from student.tests.factories import UserFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
-from .helpers import LoginEnrollmentTestCase
 
 FAKE_REQUEST = None
 
@@ -113,7 +114,8 @@ class ExternalLinkTestCase(TestCase):
         self.assertEqual(tab_list[0].is_active, False)
 
 
-class StaticTabTestCase(TestCase):
+class StaticTabTestCase(ModuleStoreTestCase):
+    """Tests for static tabs."""
 
     def setUp(self):
 
@@ -147,6 +149,26 @@ class StaticTabTestCase(TestCase):
         )
         self.assertEqual(tab_list[0].is_active, False)
 
+    @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+    def test_get_static_tab_contents(self):
+        course = get_course_by_id('edX/toy/2012_Fall')
+        request = get_request_for_user(UserFactory.create())
+        tab = tabs.get_static_tab_by_slug(course, 'resources')
+
+        # Test render works okay
+        tab_content = tabs.get_static_tab_contents(request, course, tab)
+        self.assertIn('edX/toy/2012_Fall', tab_content)
+        self.assertIn('static_tab', tab_content)
+
+        # Test when render raises an exception
+        with patch('courseware.tabs.get_module') as mock_module_render:
+            mock_module_render.return_value = MagicMock(
+                render=Mock(side_effect=Exception('Render failed!'))
+            )
+            static_tab = tabs.get_static_tab_contents(request, course, tab)
+            self.assertIn("this module is temporarily unavailable", static_tab)
+
+
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
     def setUp(self):

lms/templates/courseware/error-message.html

+<%! from django.utils.translation import ugettext as _ %>
+<%
+    link_to_support_email='<a href=\"mailto:{tech_support_email}\">{tech_support_email}</a>'.format(tech_support_email=settings.TECH_SUPPORT_EMAIL)
+%>
+<p>${_("We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at {link_to_support_email} to report any problems or downtime.").format(link_to_support_email=link_to_support_email)}</p>