From 23330ffe0ba208501d07c63a0df10b18e780ae11 Mon Sep 17 00:00:00 2001 From: Anthony Date: Wed, 10 Apr 2024 11:04:58 -0700 Subject: [PATCH] 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. --- readthedocs/core/views/__init__.py | 65 ++++++++++++++++++++++++++---- readthedocs/projects/views/base.py | 5 ++- readthedocs/templates/403.html | 1 + readthedocs/urls.py | 19 ++++----- 4 files changed, 72 insertions(+), 18 deletions(-) create mode 120000 readthedocs/templates/403.html diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index 817cb9025f7..c868163e6ae 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -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 @@ -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" @@ -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") diff --git a/readthedocs/projects/views/base.py b/readthedocs/projects/views/base.py index efca41e5104..711b5840693 100644 --- a/readthedocs/projects/views/base.py +++ b/readthedocs/projects/views/base.py @@ -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) diff --git a/readthedocs/templates/403.html b/readthedocs/templates/403.html new file mode 120000 index 00000000000..ec15dd1f74a --- /dev/null +++ b/readthedocs/templates/403.html @@ -0,0 +1 @@ +401.html \ No newline at end of file diff --git a/readthedocs/urls.py b/readthedocs/urls.py index 08ae4fc9efa..4abd1349b65 100644 --- a/readthedocs/urls.py +++ b/readthedocs/urls.py @@ -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"), @@ -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")), ] @@ -125,6 +121,11 @@ "style-catalog/", TemplateView.as_view(template_name="style_catalog.html"), ), + # For testing error responses and templates + path( + "error//", + ErrorView.as_view(base_path="errors/dashboard"), + ), # This must come last after the build output files path( "media/",