Merge pull request #2053 from edx/fix/gprice/modulestore-500-errors

Handle ItemNotFoundError from the modulestore to avoid 500 errors

lms/djangoapps/courseware/module_render.py

@@ -414,6 +414,11 @@ def modx_dispatch(request, dispatch, location, course_id):
            through the part before the first '?'.
       - location -- the module location. Used to look up the XModule instance
       - course_id -- defines the course context for this request.
+
+    Raises PermissionDenied if the user is not logged in. Raises Http404 if
+    the location and course_id do not identify a valid module, the module is
+    not accessible by the user, or the module raises NotFoundError. If the
+    module raises any other error, it will escape this function.
     '''
     # ''' (fix emacs broken parsing)
 
@@ -442,8 +447,19 @@ def modx_dispatch(request, dispatch, location, course_id):
                     return HttpResponse(json.dumps({'success': file_too_big_msg}))
             p[fileinput_id] = inputfiles
 
+    try:
+        descriptor = modulestore().get_instance(course_id, location)
+    except ItemNotFoundError:
+        log.warn(
+            "Invalid location for course id {course_id}: {location}".format(
+                course_id=course_id,
+                location=location
+            )
+        )
+        raise Http404
+
     model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
-        request.user, modulestore().get_instance(course_id, location))
+        request.user, descriptor)
 
     instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax')
     if instance is None:

lms/djangoapps/courseware/tests/test_module_render.py

@@ -69,19 +69,38 @@ class ModuleRenderTestCase(LoginEnrollmentTestCase):
                           json.dumps({'success': 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %
                                       (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2))}))
         mock_request_3 = MagicMock()
-        mock_request_3.POST.copy.return_value = {}
+        mock_request_3.POST.copy.return_value = {'position': 1}
         mock_request_3.FILES = False
         mock_request_3.user = UserFactory()
         inputfile_2 = Stub()
         inputfile_2.size = 1
         inputfile_2.name = 'name'
-        self.assertRaises(ItemNotFoundError, render.modx_dispatch,
-                          mock_request_3, 'dummy', self.location, 'toy')
-        self.assertRaises(Http404, render.modx_dispatch, mock_request_3, 'dummy',
-                          self.location, self.course_id)
-        mock_request_3.POST.copy.return_value = {'position': 1}
         self.assertIsInstance(render.modx_dispatch(mock_request_3, 'goto_position',
                                                    self.location, self.course_id), HttpResponse)
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'goto_position',
+            self.location,
+            'bad_course_id'
+        )
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'goto_position',
+            ['i4x', 'edX', 'toy', 'chapter', 'bad_location'],
+            self.course_id
+        )
+        self.assertRaises(
+            Http404,
+            render.modx_dispatch,
+            mock_request_3,
+            'bad_dispatch',
+            self.location,
+            self.course_id
+        )
 
     def test_get_score_bucket(self):
         self.assertEquals(render.get_score_bucket(0, 10), 'incorrect')