Skip to content

Commit 23330ff

Browse files
authored
Add error view for error handling and error view testing (#11263)
* Add error view for error handling and error view testing I was having trouble testing error views and templates locally and found at some point we added a `/500` for testing the 500 error page. I expanded on this to create a view that could emit any error status code and relevant template. Afterwards, it seemed like we had a few redundant views doing similar things, so I expanded the dashboard error handlers to all use the same view as well. Changes here are: - When using the new dashboard, dashboard error templates are in the `/errors/dashboard/` template path. There will be a duplicate set of templates for Proxito errors and documentation errors, in `/errors/proxito/` eventually. - This swaps out the URL `handler500`, and defines the missing handlers in the URL module. Mostly, this loads these error templates from a more structured path instead of top level `/404.html` templates. - Adds a debug view/URL for testing arbitrary error views/templates in debug mode. This does not touch proxito error pages yet. * More linting * Support all request methods in ErrorView * Add support for spam project response/410 This is just the dashboard response view for spam projects. * Symlink 403 to 401 template The 401 template has copy closer to 403 already, and I don't believe we are emitting a templated 403 response anywhere either.
1 parent ee61c35 commit 23330ff

File tree

4 files changed

+72
-18
lines changed

4 files changed

+72
-18
lines changed

readthedocs/core/views/__init__.py

+57-8
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import structlog
99
from django.conf import settings
1010
from django.http import JsonResponse
11-
from django.shortcuts import redirect, render
11+
from django.shortcuts import redirect
1212
from django.urls import reverse
1313
from django.views.generic import TemplateView, View
1414

@@ -84,6 +84,62 @@ def get_context_data(self, **kwargs):
8484
return context
8585

8686

87+
class ErrorView(TemplateView):
88+
89+
"""
90+
Render templated error pages.
91+
92+
This can be used both for testing and as a generic error view. This supports
93+
multiple subpaths for errors, as we need to show application themed errors
94+
for dashboard users and minimal error pages for documentation readers.
95+
96+
View arguments:
97+
98+
status_code
99+
This can also be a kwarg, like in the case of a testing view for all
100+
errors. Set through ``as_view(status_code=504)``, this view will always
101+
render the same template and status code.
102+
103+
base_path
104+
Base path for templates. Dashboard templates can be loaded from a
105+
separate path from Proxito error templates.
106+
"""
107+
108+
base_path = "errors/dashboard/"
109+
status_code = 500
110+
111+
def get_status_code(self):
112+
status_code = self.status_code
113+
try:
114+
status_code = int(self.kwargs["status_code"])
115+
except (ValueError, KeyError):
116+
pass
117+
return status_code
118+
119+
def get_template_names(self):
120+
status_code = self.get_status_code()
121+
if settings.RTD_EXT_THEME_ENABLED:
122+
# First try to load the template for the specific HTTP status code
123+
# and fall back to a generic 400/500 level error template
124+
status_code_class = int(status_code / 100)
125+
generic_code = f"{status_code_class}xx"
126+
return [
127+
f"{self.base_path}/{code}.html" for code in [status_code, generic_code]
128+
]
129+
# TODO the legacy dashboard has top level path errors, as is the
130+
# default. This can be removed later.
131+
return f"{status_code}.html"
132+
133+
def dispatch(self, request, *args, **kwargs):
134+
context = self.get_context_data(**kwargs)
135+
status_code = self.get_status_code()
136+
return self.render_to_response(
137+
context,
138+
status=status_code,
139+
)
140+
141+
142+
# TODO replace this with ErrorView and a template in `errors/` instead
87143
class TeapotView(TemplateView):
88144
template_name = "core/teapot.html"
89145

@@ -92,13 +148,6 @@ def get(self, request, *args, **kwargs):
92148
return self.render_to_response(context, status=418)
93149

94150

95-
def server_error_500(request, template_name="500.html"):
96-
"""A simple 500 handler so we get media."""
97-
r = render(request, template_name)
98-
r.status_code = 500
99-
return r
100-
101-
102151
def do_not_track(request):
103152
dnt_header = request.headers.get("Dnt")
104153

readthedocs/projects/views/base.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ def get(self, request, *args, **kwargs):
103103
)
104104

105105
if is_show_dashboard_denied(self.get_project()):
106-
return render(request, template_name="spam.html", status=410)
106+
template_name = "spam.html"
107+
if settings.RTD_EXT_THEME_ENABLED:
108+
template_name = "errors/dashboard/410.html"
109+
return render(request, template_name=template_name, status=410)
107110

108111
return super().get(request, *args, **kwargs)

readthedocs/templates/403.html

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
401.html

readthedocs/urls.py

+10-9
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88
from django.urls import include, path, re_path
99
from django.views.generic.base import RedirectView, TemplateView
1010

11-
from readthedocs.core.views import (
12-
HomepageView,
13-
SupportView,
14-
do_not_track,
15-
server_error_500,
16-
)
11+
from readthedocs.core.views import ErrorView, HomepageView, SupportView, do_not_track
1712
from readthedocs.search.views import GlobalSearchView
1813

1914
admin.autodiscover()
2015

21-
handler500 = server_error_500
16+
handler400 = ErrorView.as_view(status_code=400)
17+
handler403 = ErrorView.as_view(status_code=403)
18+
handler404 = ErrorView.as_view(status_code=404)
19+
handler500 = ErrorView.as_view(status_code=500)
2220

2321
basic_urls = [
2422
path("", HomepageView.as_view(), name="homepage"),
@@ -51,8 +49,6 @@
5149
path("invitations/", include("readthedocs.invitations.urls")),
5250
# For redirects
5351
path("builds/", include("readthedocs.builds.urls")),
54-
# For testing the 500's with DEBUG on.
55-
path("500/", handler500),
5652
# Put this as a unique path for the webhook, so we don't clobber existing Stripe URL's
5753
path("djstripe/", include("djstripe.urls", namespace="djstripe")),
5854
]
@@ -125,6 +121,11 @@
125121
"style-catalog/",
126122
TemplateView.as_view(template_name="style_catalog.html"),
127123
),
124+
# For testing error responses and templates
125+
path(
126+
"error/<int:status_code>/",
127+
ErrorView.as_view(base_path="errors/dashboard"),
128+
),
128129
# This must come last after the build output files
129130
path(
130131
"media/<path:remainder>",

0 commit comments

Comments
 (0)