Merge pull request #5020 from minrk/custom_error

make sure custom error messages are shown on regular error pages

jupyterhub/handlers/base.py

@@ -1487,6 +1487,7 @@ class BaseHandler(RequestHandler):
         """render custom error pages"""
         exc_info = kwargs.get('exc_info')
         message = ''
+        message_html = ''
         exception = None
         status_message = responses.get(status_code, 'Unknown HTTP Error')
         if exc_info:
@@ -1496,12 +1497,17 @@ class BaseHandler(RequestHandler):
                 message = exception.log_message % exception.args
             except Exception:
                 pass
+            # allow custom html messages
+            message_html = getattr(exception, "jupyterhub_html_message", "")
 
             # construct the custom reason, if defined
             reason = getattr(exception, 'reason', '')
             if reason:
                 message = reasons.get(reason, reason)
 
+            # get special jupyterhub_message, if defined
+            message = getattr(exception, "jupyterhub_message", message)
+
         if exception and isinstance(exception, SQLAlchemyError):
             self.log.warning("Rolling back session due to database error %s", exception)
             self.db.rollback()
@@ -1511,6 +1517,7 @@ class BaseHandler(RequestHandler):
             status_code=status_code,
             status_message=status_message,
             message=message,
+            message_html=message_html,
             extra_error_html=getattr(self, 'extra_error_html', ''),
             exception=exception,
         )

jupyterhub/handlers/pages.py

@@ -375,7 +375,10 @@ class SpawnPendingHandler(BaseHandler):
             spawn_url = url_path_join(
                 self.hub.base_url, "spawn", user.escaped_name, escaped_server_name
             )
-            self.set_status(500)
+            status_code = 500
+            if isinstance(exc, web.HTTPError):
+                status_code = exc.status_code
+            self.set_status(status_code)
             html = await self.render_template(
                 "not_running.html",
                 user=user,

jupyterhub/tests/test_pages.py

@@ -2,11 +2,14 @@
 
 import asyncio
 import sys
+from contextlib import nullcontext
+from functools import partial
 from unittest import mock
 from urllib.parse import parse_qs, urlencode, urlparse
 
 import pytest
 from bs4 import BeautifulSoup
+from tornado import web
 from tornado.httputil import url_concat
 
 from .. import orm, roles, scopes
@@ -1335,3 +1338,86 @@ async def test_services_nav_links(
         assert service.href in nav_urls
     else:
         assert service.href not in nav_urls
+
+
+class TeapotError(web.HTTPError):
+    text = "I'm a <🫖>"
+    html = "<b>🕸️🫖</b>"
+
+    def __init__(self, log_msg, kind="text"):
+        super().__init__(418, log_msg)
+        self.jupyterhub_message = self.text
+        if kind == "html":
+            self.jupyterhub_html_message = self.html
+
+
+def hook_fail_fast(spawner, kind):
+    if kind == "unhandled":
+        raise RuntimeError("unhandle me!!!")
+    raise TeapotError("log_msg", kind=kind)
+
+
+async def hook_fail_slow(spawner, kind):
+    await asyncio.sleep(1)
+    hook_fail_fast(spawner, kind)
+
+
+@pytest.mark.parametrize("speed", ["fast", "slow"])
+@pytest.mark.parametrize("kind", ["text", "html", "unhandled"])
+async def test_spawn_fails_custom_message(app, user, kind, speed):
+    if speed == 'slow':
+        speed_context = mock.patch.dict(
+            app.tornado_settings, {'slow_spawn_timeout': 0.1}
+        )
+        hook = hook_fail_slow
+    else:
+        speed_context = nullcontext()
+        hook = hook_fail_fast
+    # test the response when spawn fails before redirecting to progress
+    with mock.patch.dict(
+        app.config.Spawner, {"pre_spawn_hook": partial(hook, kind=kind)}
+    ), speed_context:
+        cookies = await app.login_user(user.name)
+        assert user.spawner.pre_spawn_hook
+        r = await get_page("spawn", app, cookies=cookies)
+        if speed == "slow":
+            # go through spawn_pending, render not_running.html
+            assert r.ok
+            assert "spawn-pending" in r.url
+            # wait for ready signal before checking next redirect
+            while user.spawner.active:
+                await asyncio.sleep(0.1)
+            app.log.info(
+                f"pending {user.spawner.active=}, {user.spawner._spawn_future=}"
+            )
+            # this should fetch the not-running page
+            app.log.info("getting again")
+            r = await get_page(
+                f"spawn-pending/{user.escaped_name}", app, cookies=cookies
+            )
+            target_class = "container"
+            unhandled_text = "Spawn failed"
+        else:
+            unhandled_text = "Unhandled error"
+            target_class = "error"
+        page = BeautifulSoup(r.content)
+        if kind == "unhandled":
+            assert r.status_code == 500
+        else:
+            assert r.status_code == 418
+        error = page.find(class_=target_class)
+        # check escaping properly
+        error_html = str(error)
+        if kind == "text":
+            assert "<🫖>" in error.text
+            assert "🕸️" not in error.text
+            assert "&lt;🫖&gt;" in error_html
+        elif kind == "html":
+            assert "<🫖>" not in error.text
+            assert "🕸️" in error.text
+            assert "<b>🕸️🫖</b>" in error_html
+        elif kind == "unhandled":
+            assert unhandled_text in error.text
+            assert "unhandle me" not in error.text
+        else:
+            raise ValueError(f"unexpected {kind=}")

share/jupyterhub/templates/error.html

@@ -7,8 +7,11 @@
       <h1>{{ status_code }} : {{ status_message }}</h1>
     {% endblock h1_error %}
     {% block error_detail %}
-      {% if message %}<p>{{ message }}</p>{% endif %}
-      {% if message_html %}<p>{{ message_html | safe }}</p>{% endif %}
+      {% if message_html %}
+        <p>{{ message_html | safe }}</p>
+      {% elif message %}
+        <p>{{ message }}</p>
+      {% endif %}
       {% if extra_error_html %}<p>{{ extra_error_html | safe }}</p>{% endif %}
     {% endblock error_detail %}
   </div>