AN-8573 Respond with 404, not 500, if invalid course_id requested (#647)

* Raise 404, not 500, if invalid course_id requested

* Remove unneeded try/except in CourseContextMixin

The Http404 will be raised by the middleware or by the CourseValidMixin before
it even starts executing that function.

* Revert adding try/except to format_course_key tag

analytics_dashboard/courses/middleware.py

@@ -3,7 +3,9 @@ This file contains Django middleware. For more information visit
 https://docs.djangoproject.com/en/dev/topics/http/middleware/.
 """
 import logging
+from django.http import Http404
 from django.template.response import TemplateResponse
+from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 from courses.exceptions import PermissionsRetrievalFailedError
 
@@ -22,7 +24,12 @@ class CourseMiddleware(object):
         course_id = view_kwargs.get('course_id', None)
 
         if course_id:
-            request.course_key = CourseKey.from_string(course_id)
+            try:
+                request.course_key = CourseKey.from_string(course_id)
+            except InvalidKeyError:
+                # Raising an InvalidKeyError here causes a 500-level error which alerts devops. This should really be a
+                # 404 error because though the course requested cannot be found, the server is operating correctly.
+                raise Http404
             request.course_id = unicode(request.course_key)
 
         return None

analytics_dashboard/courses/tests/test_middleware.py

 import logging
 
 import ddt
+from django.http import Http404
 from django.template.response import TemplateResponse
 from opaque_keys.edx.keys import CourseKey
 from testfixtures import LogCapture
@@ -43,6 +44,12 @@ class CourseMiddlewareTests(MiddlewareTestCase):
         self.assertEqual(request.course_id, course_id)
         self.assertEqual(request.course_key, course_key)
 
+    @ddt.data('edX/DemoX/Demo_Course/Foo', 'course-v1:edX+DemoX')
+    def test_invalid_course_id(self, course_id):
+        request = self.factory.get('/')
+        with self.assertRaises(Http404):
+            self.middleware.process_view(request, '', None, {'course_id': course_id})
+
 
 class CoursePermissionsExceptionMiddlewareTests(CoursePermissionsExceptionMixin, MiddlewareTestCase):
     middleware_class = CoursePermissionsExceptionMiddleware