Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
Merge pull request #280 from edx/dsjen/502
Browse files Browse the repository at this point in the history
Error page will be displayed for 502s from course API.
  • Loading branch information
dsjen committed Mar 6, 2015
2 parents 1534ce3 + decc702 commit 2c1ef6a
Show file tree
Hide file tree
Showing 13 changed files with 107 additions and 25 deletions.
5 changes: 5 additions & 0 deletions acceptance_tests/pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,8 @@ class NotFoundErrorPage(ErrorPage):
class AccessDeniedErrorPage(ErrorPage):
error_code = 403
error_title = u'Access Denied'


class BadGatewayErrorPage(ErrorPage):
error_code = 502
error_title = u"We're having trouble loading this page. Please try again in a minute."
4 changes: 2 additions & 2 deletions acceptance_tests/test_error_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
from bok_choy.web_app_test import WebAppTest

from acceptance_tests import PLATFORM_NAME, APPLICATION_NAME, SUPPORT_URL, ENABLE_ERROR_PAGE_TESTS
from acceptance_tests.pages import ServerErrorPage, NotFoundErrorPage, AccessDeniedErrorPage
from acceptance_tests.pages import ServerErrorPage, NotFoundErrorPage, AccessDeniedErrorPage, BadGatewayErrorPage


@skipUnless(ENABLE_ERROR_PAGE_TESTS, 'Error page tests are not enabled.')
class ErrorPagesTests(WebAppTest):
error_page_classes = [ServerErrorPage, NotFoundErrorPage, AccessDeniedErrorPage]
error_page_classes = [ServerErrorPage, NotFoundErrorPage, AccessDeniedErrorPage, BadGatewayErrorPage]

def test_valid_pages(self):
for page_class in self.error_page_classes:
Expand Down
5 changes: 5 additions & 0 deletions analytics_dashboard/core/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class BadGatewayError(Exception):
"""
Raise if bad gateway (502).
"""
pass
18 changes: 18 additions & 0 deletions analytics_dashboard/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@
Middleware for Language Preferences
"""

import logging
from lang_pref_middleware import middleware

from django.template.response import TemplateResponse

from core.exceptions import BadGatewayError

logger = logging.getLogger(__name__)


class LanguagePreferenceMiddleware(middleware.LanguagePreferenceMiddleware):
def get_user_language_preference(self, user):
Expand All @@ -13,3 +20,14 @@ def get_user_language_preference(self, user):
Returns None if no preference set.
"""
return user.language


class BadGatewayExceptionMiddleware(object):
"""
Display an error template for 502 errors.
"""

def process_exception(self, request, exception):
if type(exception) is BadGatewayError:
logger.exception(exception)
return TemplateResponse(request, '502.html', status=502)
45 changes: 43 additions & 2 deletions analytics_dashboard/core/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,32 @@
from django.test import TestCase
import logging

from django.template.response import TemplateResponse
from django.test import RequestFactory, TestCase
from django_dynamic_fixture import G
from lang_pref_middleware.tests import LangPrefMiddlewareTestCaseMixin
from testfixtures import LogCapture

from core.middleware import LanguagePreferenceMiddleware
from core.exceptions import BadGatewayError
from core.middleware import BadGatewayExceptionMiddleware, LanguagePreferenceMiddleware
from core.models import User


class MiddlewareTestCase(TestCase):
middleware_class = None

def setUp(self):
super(MiddlewareTestCase, self).setUp()
self.factory = RequestFactory()
self.middleware = self.middleware_class() # pylint: disable=not-callable


class MiddlewareAssertionMixin(object):
def assertStandardExceptions(self, request):
self.assertIsNone(self.middleware.process_exception(request, None))
self.assertIsNone(self.middleware.process_exception(request, Exception))
self.assertIsNone(self.middleware.process_exception(request, ArithmeticError))


class TestUserLanguagePreferenceMiddleware(LangPrefMiddlewareTestCaseMixin, TestCase):
middleware_class = LanguagePreferenceMiddleware

Expand All @@ -15,3 +36,23 @@ def get_user(self):
def set_user_language_preference(self, user, language):
user.language = language
user.save()


class BadGatewayMiddlewareTests(MiddlewareAssertionMixin, MiddlewareTestCase):
middleware_class = BadGatewayExceptionMiddleware

def assertIsBadGatewayErrorResponse(self, response):
self.assertEqual(response.status_code, 502)
self.assertIs(type(response), TemplateResponse)
self.assertEqual(response.template_name, '502.html')

def test_process_exception(self):
request = self.factory.get('/')
self.assertStandardExceptions(request)
with LogCapture(level=logging.WARN) as l:
exception = BadGatewayError()
response = self.middleware.process_exception(request, exception)
self.assertIsBadGatewayErrorResponse(response)

# Verify the exception was logged
l.check(('core.middleware', 'ERROR', str(exception)),)
7 changes: 7 additions & 0 deletions analytics_dashboard/core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ def logout_then_login(request, login_url=reverse_lazy('login'), current_app=None
return django.contrib.auth.views.logout_then_login(request, login_url, current_app, extra_context)


class BadGatewayView(TemplateView):
"""
Bad gateway error page requesting users to wait and reload the page.
"""
template_name = "502.html"


class LandingView(TemplateView):
"""
Displays a public landing page when users first come to the site.
Expand Down
19 changes: 4 additions & 15 deletions analytics_dashboard/courses/tests/test_middleware.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,15 @@
import logging

from django.template.response import TemplateResponse
from django.test import RequestFactory, TestCase
from opaque_keys.edx.keys import CourseKey
from testfixtures import LogCapture

from core.tests.test_middleware import MiddlewareTestCase, MiddlewareAssertionMixin
from courses.exceptions import PermissionsRetrievalFailedError
from courses.middleware import CourseMiddleware, CoursePermissionsExceptionMiddleware


class MiddlewareTestCase(TestCase):
middleware_class = None

def setUp(self):
super(MiddlewareTestCase, self).setUp()
self.factory = RequestFactory()
self.middleware = self.middleware_class() # pylint: disable=not-callable


class MiddlewareAssertionMixin(object):
class CoursePermissionsExceptionMixin(MiddlewareAssertionMixin):
def assertIsPermissionsRetrievalFailedResponse(self, response):
self.assertEqual(response.status_code, 500)
self.assertIs(type(response), TemplateResponse)
Expand Down Expand Up @@ -49,16 +40,14 @@ def test_course_id(self):
self.assertEqual(request.course_key, course_key)


class CoursePermissionsExceptionMiddlewareTests(MiddlewareAssertionMixin, MiddlewareTestCase):
class CoursePermissionsExceptionMiddlewareTests(CoursePermissionsExceptionMixin, MiddlewareTestCase):
middleware_class = CoursePermissionsExceptionMiddleware

def test_process_exception(self):
request = self.factory.get('/')

# Method should return None if exception argument is NOT PermissionsRetrievalFailedError.
self.assertIsNone(self.middleware.process_exception(request, None))
self.assertIsNone(self.middleware.process_exception(request, Exception))
self.assertIsNone(self.middleware.process_exception(request, ArithmeticError))
self.assertStandardExceptions(request)

with LogCapture(level=logging.WARN) as l:
# Method should only return a response for PermissionsRetrievalFailedError.
Expand Down
4 changes: 2 additions & 2 deletions analytics_dashboard/courses/tests/test_views/test_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
CourseAPIMixin

from courses.exceptions import PermissionsRetrievalFailedError
from courses.tests.test_middleware import MiddlewareAssertionMixin
from courses.tests.test_middleware import CoursePermissionsExceptionMixin


@ddt
Expand All @@ -28,7 +28,7 @@ def test_missing_data(self, course_id):
self.skipTest('The course homepage does not check for the existence of a course.')


class CourseIndexViewTests(CourseAPIMixin, ViewTestMixin, MiddlewareAssertionMixin, TestCase):
class CourseIndexViewTests(CourseAPIMixin, ViewTestMixin, CoursePermissionsExceptionMixin, TestCase):
viewname = 'courses:index'

def setUp(self):
Expand Down
10 changes: 7 additions & 3 deletions analytics_dashboard/courses/views/performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from slugify import slugify
from slumber.exceptions import SlumberBaseException

from core.exceptions import BadGatewayError
from courses.presenters.performance import CoursePerformancePresenter
from courses.views import CourseTemplateWithNavView, CourseAPIMixin

Expand Down Expand Up @@ -40,9 +41,12 @@ def dispatch(self, request, *args, **kwargs):
except SlumberBaseException as e:
# Return the appropriate response if a 404 occurred.
response = getattr(e, 'response')
if response is not None and response.status_code == 404:
logger.info('Course API data not found for %s: %s', self.course_id, e)
raise Http404
if response is not None:
if response.status_code == 404:
logger.info('Course API data not found for %s: %s', self.course_id, e)
raise Http404
elif response.status_code == 502:
raise BadGatewayError

# Not a 404. Continue raising the error.
logger.error('An error occurred while using Slumber to communicate with an API: %s', e)
Expand Down
2 changes: 1 addition & 1 deletion analytics_dashboard/help/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class HelpURLMiddleware(object):

def process_template_response(self, _request, response):
# Error responses do not have context.
if response.status_code == 500:
if response.status_code in [500, 502]:
return response

page_token = response.context_data.get(HELP_CONTEXT_TOKEN_NAME)
Expand Down
1 change: 1 addition & 0 deletions analytics_dashboard/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'waffle.middleware.WaffleMiddleware',
'core.middleware.LanguagePreferenceMiddleware',
'core.middleware.BadGatewayExceptionMiddleware',
'courses.middleware.CourseMiddleware',
'courses.middleware.CoursePermissionsExceptionMiddleware',
'social.apps.django_app.middleware.SocialAuthExceptionMiddleware',
Expand Down
11 changes: 11 additions & 0 deletions analytics_dashboard/templates/502.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% extends "base-error.html" %}

{% load i18n %}

{% block title %}{% trans "We're having trouble loading this page. Please try again in a minute." %} {{ block.super }}{% endblock title %}

{% block error-title %}{% trans "We're having trouble loading this page. Please try again in a minute." %}{% endblock %}
{% block error-copy %}
{{ block.super }}
{% trans "(Technical details: 502 Bad Gateway.)" %}
{% endblock %}
1 change: 1 addition & 0 deletions analytics_dashboard/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
url(r'^403/$', 'django.views.defaults.permission_denied'),
url(r'^404/$', 'django.views.defaults.page_not_found'),
url(r'^500/$', 'django.views.defaults.server_error'),
url(r'^502/$', views.BadGatewayView.as_view(), name='bad_gateway'),
)

if os.environ.get('ENABLE_DJANGO_TOOLBAR', False):
Expand Down

0 comments on commit 2c1ef6a

Please sign in to comment.