Skip to content

Commit

Permalink
Add error view for error handling and error view testing (#11263)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
agjohnson committed Apr 10, 2024
1 parent ee61c35 commit 23330ff
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 18 deletions.
65 changes: 57 additions & 8 deletions readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import structlog
from django.conf import settings
from django.http import JsonResponse
from django.shortcuts import redirect, render
from django.shortcuts import redirect
from django.urls import reverse
from django.views.generic import TemplateView, View

Expand Down Expand Up @@ -84,6 +84,62 @@ def get_context_data(self, **kwargs):
return context


class ErrorView(TemplateView):

"""
Render templated error pages.
This can be used both for testing and as a generic error view. This supports
multiple subpaths for errors, as we need to show application themed errors
for dashboard users and minimal error pages for documentation readers.
View arguments:
status_code
This can also be a kwarg, like in the case of a testing view for all
errors. Set through ``as_view(status_code=504)``, this view will always
render the same template and status code.
base_path
Base path for templates. Dashboard templates can be loaded from a
separate path from Proxito error templates.
"""

base_path = "errors/dashboard/"
status_code = 500

def get_status_code(self):
status_code = self.status_code
try:
status_code = int(self.kwargs["status_code"])
except (ValueError, KeyError):
pass
return status_code

def get_template_names(self):
status_code = self.get_status_code()
if settings.RTD_EXT_THEME_ENABLED:
# First try to load the template for the specific HTTP status code
# and fall back to a generic 400/500 level error template
status_code_class = int(status_code / 100)
generic_code = f"{status_code_class}xx"
return [
f"{self.base_path}/{code}.html" for code in [status_code, generic_code]
]
# TODO the legacy dashboard has top level path errors, as is the
# default. This can be removed later.
return f"{status_code}.html"

def dispatch(self, request, *args, **kwargs):
context = self.get_context_data(**kwargs)
status_code = self.get_status_code()
return self.render_to_response(
context,
status=status_code,
)


# TODO replace this with ErrorView and a template in `errors/` instead
class TeapotView(TemplateView):
template_name = "core/teapot.html"

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


def server_error_500(request, template_name="500.html"):
"""A simple 500 handler so we get media."""
r = render(request, template_name)
r.status_code = 500
return r


def do_not_track(request):
dnt_header = request.headers.get("Dnt")

Expand Down
5 changes: 4 additions & 1 deletion readthedocs/projects/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ def get(self, request, *args, **kwargs):
)

if is_show_dashboard_denied(self.get_project()):
return render(request, template_name="spam.html", status=410)
template_name = "spam.html"
if settings.RTD_EXT_THEME_ENABLED:
template_name = "errors/dashboard/410.html"
return render(request, template_name=template_name, status=410)

return super().get(request, *args, **kwargs)
1 change: 1 addition & 0 deletions readthedocs/templates/403.html
19 changes: 10 additions & 9 deletions readthedocs/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@
from django.urls import include, path, re_path
from django.views.generic.base import RedirectView, TemplateView

from readthedocs.core.views import (
HomepageView,
SupportView,
do_not_track,
server_error_500,
)
from readthedocs.core.views import ErrorView, HomepageView, SupportView, do_not_track
from readthedocs.search.views import GlobalSearchView

admin.autodiscover()

handler500 = server_error_500
handler400 = ErrorView.as_view(status_code=400)
handler403 = ErrorView.as_view(status_code=403)
handler404 = ErrorView.as_view(status_code=404)
handler500 = ErrorView.as_view(status_code=500)

basic_urls = [
path("", HomepageView.as_view(), name="homepage"),
Expand Down Expand Up @@ -51,8 +49,6 @@
path("invitations/", include("readthedocs.invitations.urls")),
# For redirects
path("builds/", include("readthedocs.builds.urls")),
# For testing the 500's with DEBUG on.
path("500/", handler500),
# Put this as a unique path for the webhook, so we don't clobber existing Stripe URL's
path("djstripe/", include("djstripe.urls", namespace="djstripe")),
]
Expand Down Expand Up @@ -125,6 +121,11 @@
"style-catalog/",
TemplateView.as_view(template_name="style_catalog.html"),
),
# For testing error responses and templates
path(
"error/<int:status_code>/",
ErrorView.as_view(base_path="errors/dashboard"),
),
# This must come last after the build output files
path(
"media/<path:remainder>",
Expand Down

0 comments on commit 23330ff

Please sign in to comment.