Contextualize 404 page

media/css/core.css

@@ -1331,6 +1331,9 @@ div.module.project-subprojects li.subproject a.subproject-edit:before {
     content: "\f044";
 }
 
+#content ul.normal_list {list-style: disc; margin-left: 20px;}
+#content code {background: #eee; border: 1px solid #ccc; padding: 3px; display: inline-block;}
+
 /* Pygments */
 div.highlight pre .hll { background-color: #ffffcc }
 div.highlight pre .c { color: #60a0b0; font-style: italic } /* Comment */

readthedocs/proxito/exceptions.py

@@ -0,0 +1,100 @@
+from django.http import Http404
+from django.utils.translation import gettext_lazy as _
+
+
+class ContextualizedHttp404(Http404):
+
+    """
+    Base class for contextualized HTTP 404 handling.
+
+    Subclasses may define their own template name,
+    HTTP status and object that was not found.
+
+    The contextualized exception is handled by the project's 404 view.
+    """
+
+    template_name = "errors/404/base.html"
+    not_found_subject = _("page")
+
+    def __init__(self, **kwargs):
+        """
+        Constructor that all subclasses should call.
+
+        :param kwargs: all kwargs are added as page context for rendering the 404 template
+        :param http_status: 404 view should respect this and set the HTTP status.
+        :param path_not_found: Inform the template and 404 view about a different path from
+                               request.path
+        """
+        self.http_status = kwargs.pop("http_status", 404)
+        self.path_not_found = kwargs.pop("path_not_found", None)
+        self.kwargs = kwargs
+
+    def get_context(self):
+        c = {
+            "not_found_subject": self.not_found_subject,
+            "path_not_found": self.path_not_found,
+        }
+        c.update(self.kwargs)
+        return c
+
+
+class DomainDNSHttp404(ContextualizedHttp404):
+
+    """Raised if a DNS record points to us but we do not know the domain."""
+
+    template_name = "errors/404/dns.html"
+    not_found_subject = _("domain matching DNS record")
+
+
+class ProjectHttp404(ContextualizedHttp404):
+
+    """
+    Raised if a domain did not resolve to a project.
+
+    This is currently used very broadly.
+    It indicates a number of reasons for the user.
+    """
+
+    template_name = "errors/404/no_project.html"
+    not_found_subject = _("project")
+
+
+class SubprojectHttp404(ContextualizedHttp404):
+
+    """Raised if a subproject was not found."""
+
+    template_name = "errors/404/no_subproject.html"
+    not_found_subject = _("sub project")
+
+
+class ProxitoProjectFilenameHttp404(ContextualizedHttp404):
+
+    """Raised if a page inside an existing project was not found."""
+
+    template_name = "errors/404/no_project_page.html"
+    not_found_subject = _("documentation page")
+
+
+class ProjectTranslationHttp404(ContextualizedHttp404):
+
+    """
+    Raised if a translation of a project was not found.
+
+    This means that the project does not exist for requested language.
+    If a page isn't found, raise a ProjectPageHttp404.
+    """
+
+    template_name = "errors/404/no_language.html"
+    not_found_subject = _("translation")
+
+
+class ProjectVersionHttp404(ContextualizedHttp404):
+
+    """
+    Raised if a version was not found.
+
+    Note: The containing project can be both a project or a subproject inside another project.
+    """
+
+    template_name = "errors/404/no_version.html"
+    not_found_subject = _("documentation version")

readthedocs/proxito/middleware.py

@@ -12,8 +12,7 @@
 import structlog
 from django.conf import settings
 from django.core.exceptions import SuspiciousOperation
-from django.http import Http404
-from django.shortcuts import redirect, render
+from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.deprecation import MiddlewareMixin
 
@@ -28,6 +27,8 @@
 from readthedocs.core.utils import get_cache_tag
 from readthedocs.projects.models import Feature, Project
 
+from .exceptions import DomainDNSHttp404, ProjectHttp404
+
 log = structlog.get_logger(__name__)
 
 
@@ -213,20 +214,23 @@ def process_request(self, request):  # noqa
             unresolved_domain = unresolver.unresolve_domain_from_request(request)
         except SuspiciousHostnameError as exc:
             log.warning("Weird variation on our hostname.", domain=exc.domain)
-            return render(
-                request,
-                "core/dns-404.html",
-                context={"host": exc.domain},
-                status=400,
+            # Raise a contextualized 404 that will be handled by proxito's 404 handler
+            raise DomainDNSHttp404(
+                http_status=400,
+                host=exc.domain,
             )
-        except (InvalidSubdomainError, InvalidExternalDomainError):
+        except (InvalidSubdomainError, InvalidExternalDomainError) as exc:
             log.debug("Invalid project set on the subdomain.")
-            raise Http404
+            # Raise a contextualized 404 that will be handled by proxito's 404 handler
+            raise ProjectHttp404(
+                domain=exc.domain,
+            )
         except InvalidCustomDomainError as exc:
             # Some person is CNAMEing to us without configuring a domain - 404.
             log.debug("CNAME 404.", domain=exc.domain)
-            return render(
-                request, "core/dns-404.html", context={"host": exc.domain}, status=404
+            # Raise a contextualized 404 that will be handled by proxito's 404 handler
+            raise DomainDNSHttp404(
+                host=exc.domain,
             )
         except InvalidXRTDSlugHeaderError:
             raise SuspiciousOperation("Invalid X-RTD-Slug header.")

readthedocs/proxito/tests/test_middleware.py

@@ -4,14 +4,15 @@
 import pytest
 from django.core.exceptions import SuspiciousOperation
 from django.http import HttpRequest
-from django.test import TestCase
+from django.test import Client, TestCase
 from django.test.utils import override_settings
 from django_dynamic_fixture import get
 
 from readthedocs.builds.models import Version
 from readthedocs.projects.constants import PUBLIC
 from readthedocs.projects.models import Domain, Feature, Project, ProjectRelationship
 from readthedocs.proxito.constants import RedirectType
+from readthedocs.proxito.exceptions import DomainDNSHttp404
 from readthedocs.proxito.middleware import ProxitoMiddleware
 from readthedocs.rtd_tests.base import RequestFactoryTestMixin
 from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest
@@ -139,9 +140,18 @@ def test_proper_cname_uppercase(self):
     def test_invalid_cname(self):
         self.assertFalse(Domain.objects.filter(domain='my.host.com').exists())
         request = self.request(method='get', path=self.url, HTTP_HOST='my.host.com')
-        r = self.run_middleware(request)
-        # We show the 404 error page
-        self.assertContains(r, 'my.host.com', status_code=404)
+
+        with self.assertRaises(DomainDNSHttp404) as cm:
+            self.run_middleware(request)
+
+        assert cm.exception.http_status == 404
+
+        # test all the exception handling combined
+        client_without_exception_handling = Client(rause_request_exception=False)
+        r = client_without_exception_handling.request(
+            method="get", path=self.url, HTTP_HOST="my.host.com"
+        )
+        assert r.status_code == 404
 
     def test_proper_subdomain(self):
         request = self.request(method='get', path=self.url, HTTP_HOST='pip.dev.readthedocs.io')
@@ -194,8 +204,17 @@ def test_request_header_not_allowed(self):
     def test_long_bad_subdomain(self):
         domain = 'www.pip.dev.readthedocs.io'
         request = self.request(method='get', path=self.url, HTTP_HOST=domain)
-        res = self.run_middleware(request)
-        self.assertEqual(res.status_code, 400)
+        with self.assertRaises(DomainDNSHttp404) as cm:
+            self.run_middleware(request)
+
+        assert cm.exception.http_status == 400
+
+        # test all the exception handling combined
+        client_without_exception_handling = Client(rause_request_exception=False)
+        r = client_without_exception_handling.request(
+            method="get", path=self.url, HTTP_HOST=domain
+        )
+        assert r.status_code == 400
 
     def test_front_slash(self):
         domain = 'pip.dev.readthedocs.io'

readthedocs/proxito/views/decorators.py

@@ -20,7 +20,7 @@ def map_subproject_slug(view_func):
 
     @wraps(view_func)
     def inner_view(  # noqa
-            request, subproject=None, subproject_slug=None, *args, **kwargs
+        request, subproject=None, subproject_slug=None, *args, **kwargs
     ):
         if subproject is None and subproject_slug:
             # Try to fetch by subproject alias first, otherwise we might end up

readthedocs/proxito/views/serve.py

@@ -30,6 +30,12 @@
 from readthedocs.redirects.exceptions import InfiniteRedirectException
 from readthedocs.storage import build_media_storage
 
+from ..exceptions import (
+    ContextualizedHttp404,
+    ProjectTranslationHttp404,
+    ProjectVersionHttp404,
+    ProxitoProjectFilenameHttp404,
+)
 from .mixins import (
     InvalidPathError,
     ServeDocsMixin,
@@ -139,7 +145,13 @@ def get(
 
         original_version_slug = version_slug
         version_slug = self.get_version_from_host(request, version_slug)
-        final_project, lang_slug, version_slug, filename = _get_project_data_from_request(  # noqa
+
+        (
+            final_project,
+            lang_slug,
+            version_slug,
+            filename,
+        ) = _get_project_data_from_request(  # noqa
             request,
             project_slug=project_slug,
             subproject_slug=subproject_slug,
@@ -241,7 +253,7 @@ def get(
                 'Invalid URL for project with versions.',
                 filename=filename,
             )
-            raise Http404('Invalid URL for project with versions')
+            raise Http404("Invalid URL for project with versions")
 
         redirect_path, http_status = self.get_redirect(
             project=final_project,
@@ -341,6 +353,9 @@ def get_using_unresolver(self, request):
         if unresolved_domain.is_from_external_domain:
             self.version_type = EXTERNAL
 
+        # 404 errors aren't contextualized because they are sent to the HTTP proxy
+        # The path will be 'unresolved' again when HTTP server handles the 404 error
+        # See: ServeError404Base
         try:
             unresolved = unresolver.unresolve_path(
                 unresolved_domain=unresolved_domain,
@@ -516,13 +531,20 @@ def get(self, request, proxito_path, template_name="404.html"):
 
         version_slug = kwargs.get('version_slug')
         version_slug = self.get_version_from_host(request, version_slug)
-        final_project, lang_slug, version_slug, filename = _get_project_data_from_request(  # noqa
+        # This special treatment of Http404 happens because the decorator that
+        # resolves a project doesn't know if it's resolving a subproject or a normal project
+        (
+            final_project,
+            lang_slug,
+            version_slug,
+            filename,
+        ) = _get_project_data_from_request(  # noqa
             request,
-            project_slug=kwargs.get('project_slug'),
-            subproject_slug=kwargs.get('subproject_slug'),
-            lang_slug=kwargs.get('lang_slug'),
+            project_slug=kwargs.get("project_slug"),
+            subproject_slug=kwargs.get("subproject_slug"),
+            lang_slug=kwargs.get("lang_slug"),
             version_slug=version_slug,
-            filename=kwargs.get('filename', ''),
+            filename=kwargs.get("filename", ""),
         )
 
         log.bind(
@@ -585,7 +607,7 @@ def get(self, request, proxito_path, template_name="404.html"):
         if response:
             return response
 
-        raise Http404('No custom 404 page found.')
+        raise Http404("No custom 404 page found.")
 
     def _register_broken_link(self, project, version, path, full_path):
         try:
@@ -750,6 +772,9 @@ def get_using_unresolver(self, request, path):
         # Try to map the current path to a project/version/filename.
         # If that fails, we fill the variables with the information we have
         # available in the exceptions.
+
+        contextualized_404_class = ContextualizedHttp404
+
         try:
             unresolved = unresolver.unresolve_path(
                 unresolved_domain=unresolved_domain,
@@ -761,21 +786,26 @@ def get_using_unresolver(self, request, path):
             filename = unresolved.filename
             lang_slug = project.language
             version_slug = version.slug
+            contextualized_404_class = ProxitoProjectFilenameHttp404
         except VersionNotFoundError as exc:
             project = exc.project
             lang_slug = project.language
             version_slug = exc.version_slug
             filename = exc.filename
+            contextualized_404_class = ProjectVersionHttp404
         except TranslationNotFoundError as exc:
             project = exc.project
             lang_slug = exc.language
             version_slug = exc.version_slug
             filename = exc.filename
+            contextualized_404_class = ProjectTranslationHttp404
         except InvalidExternalVersionError as exc:
             project = exc.project
+            # TODO: Use a contextualized 404
         except InvalidPathForVersionedProjectError as exc:
             project = exc.project
             filename = exc.path
+            # TODO: Use a contextualized 404
 
         log.bind(
             project_slug=project.slug,
@@ -838,7 +868,15 @@ def get_using_unresolver(self, request, path):
         )
         if response:
             return response
-        raise Http404("No custom 404 page found.")
+
+        # No custom 404 page, use our contextualized 404 response
+        raise contextualized_404_class(
+            project=project,
+            version=version,
+            filename=filename,
+            lang_slug=project.language,
+            path_not_found=path,
+        )
 
 
 class ServeError404(SettingsOverrideObject):
@@ -1034,7 +1072,7 @@ def changefreqs_generator():
             only_active=True,
         )
         if not public_versions.exists():
-            raise Http404
+            raise Http404()
 
         sorted_versions = sort_version_aware(public_versions)