Skip to content

Serve simple error pages if HTML isn't required #398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions tbx/core/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,25 @@ def test_setting_theme_on_one_site_sets_it_on_multiple_sites(self):
# check theme is set on new site
resp = self.client.get(f"http://{new_site.hostname}/")
self.assertEqual(resp.context["MODE"], mode)


class PageNotFoundTestCase(TestCase):
url = "/does-not-exist/"

def test_accept_html(self) -> None:
response = self.client.get(self.url, headers={"Accept": "text/html"})
self.assertEqual(response.status_code, 404)
self.assertIn("text/html", response.headers["content-type"])
self.assertEqual(response.headers["Vary"], "Accept, Cookie")

def test_accept_text(self) -> None:
response = self.client.get(self.url, headers={"Accept": "text/plain"})
self.assertEqual(response.status_code, 404)
self.assertIn("text/plain", response.headers["content-type"])
self.assertEqual(response.headers["Vary"], "Accept")

def test_simple_when_doesnt_accept_html(self) -> None:
response = self.client.get(self.url, headers={"Accept": "text/css"})
self.assertEqual(response.status_code, 404)
self.assertIn("text/plain", response.headers["content-type"])
self.assertEqual(response.headers["Vary"], "Accept")
29 changes: 27 additions & 2 deletions tbx/core/utils/views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,34 @@
from django.http import HttpResponseNotFound, HttpResponseServerError
from django.views import defaults
from django.views.decorators.vary import vary_on_headers


def show_html_error_page(request):
"""
- If HTML is the most preferred type, show HTML.
- If plain text is most preferred, don't show HTML
- If neither are accepted, don't show HTML
"""
return request.get_preferred_type(["text/html", "text/plain"]) == "text/html"


@vary_on_headers("Accept")
def page_not_found(request, exception, template_name="patterns/pages/errors/404.html"):
return defaults.page_not_found(request, exception, template_name)
if show_html_error_page(request):
return defaults.page_not_found(request, exception, template_name)

# Serve a simpler, cheaper 404 page if possible
return HttpResponseNotFound(
"Page not found", content_type="text/plain; charset=utf-8"
)


@vary_on_headers("Accept")
def server_error(request, template_name="patterns/pages/errors/500.html"):
return defaults.server_error(request, template_name)
if show_html_error_page(request):
return defaults.server_error(request, template_name)

# Serve a simpler, cheaper 500 page if possible
return HttpResponseServerError(
"Server error", content_type="text/plain; charset=utf-8"
)
16 changes: 5 additions & 11 deletions tbx/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
get_default_cache_control_decorator,
get_default_cache_control_method_decorator,
)
from tbx.core.utils.views import page_not_found, server_error
from tbx.core.views import robots, switch_mode


Expand All @@ -35,7 +36,6 @@
if settings.DEBUG:
from django.conf.urls.static import static
from django.contrib.staticfiles.urls import staticfiles_urlpatterns
from django.views.generic import TemplateView

# Serve static and media files from development server
urlpatterns += staticfiles_urlpatterns()
Expand All @@ -44,14 +44,8 @@
# Add views for testing 404 and 500 templates
urlpatterns += [
# Add views for testing 404 and 500 templates
path(
"test404/",
TemplateView.as_view(template_name="patterns/pages/errors/404.html"),
),
path(
"test500/",
TemplateView.as_view(template_name="patterns/pages/errors/500.html"),
),
path("test404/", page_not_found),
path("test500/", server_error),
]

# Django Debug Toolbar
Expand Down Expand Up @@ -104,5 +98,5 @@
)

# Error handlers
handler404 = "tbx.core.utils.views.page_not_found"
handler500 = "tbx.core.utils.views.server_error"
handler404 = page_not_found
handler500 = server_error