New handle_exception plugin hook, refs #1770

Also refs:
- https://github.com/simonw/datasette-sentry/issues/1
- https://github.com/simonw/datasette-show-errors/issues/2

datasette/app.py

@@ -16,7 +16,6 @@ import re
 import secrets
 import sys
 import threading
-import traceback
 import urllib.parse
 from concurrent import futures
 from pathlib import Path
@@ -27,7 +26,7 @@ from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader
 from jinja2.environment import Template
 from jinja2.exceptions import TemplateNotFound
 
-from .views.base import DatasetteError, ureg
+from .views.base import ureg
 from .views.database import DatabaseDownload, DatabaseView
 from .views.index import IndexView
 from .views.special import (
@@ -49,7 +48,6 @@ from .utils import (
     PrefixedUrlString,
     SPATIALITE_FUNCTIONS,
     StartupError,
-    add_cors_headers,
     async_call_with_supported_arguments,
     await_me_maybe,
     call_with_supported_arguments,
@@ -87,11 +85,6 @@ from .tracer import AsgiTracer
 from .plugins import pm, DEFAULT_PLUGINS, get_plugins
 from .version import __version__
 
-try:
-    import rich
-except ImportError:
-    rich = None
-
 app_root = Path(__file__).parent.parent
 
 # https://github.com/simonw/datasette/issues/283#issuecomment-781591015
@@ -1274,6 +1267,16 @@ class DatasetteRouter:
             return
         except NotFound as exception:
             return await self.handle_404(request, send, exception)
+        except Forbidden as exception:
+            # Try the forbidden() plugin hook
+            for custom_response in pm.hook.forbidden(
+                datasette=self.ds, request=request, message=exception.args[0]
+            ):
+                custom_response = await await_me_maybe(custom_response)
+                assert (
+                    custom_response
+                ), "Default forbidden() hook should have been called"
+                return await custom_response.asgi_send(send)
         except Exception as exception:
             return await self.handle_exception(request, send, exception)
 
@@ -1372,72 +1375,20 @@ class DatasetteRouter:
                 await self.handle_exception(request, send, exception or NotFound("404"))
 
     async def handle_exception(self, request, send, exception):
-        if self.ds.pdb:
-            import pdb
+        responses = []
+        for hook in pm.hook.handle_exception(
+            datasette=self.ds,
+            request=request,
+            exception=exception,
+        ):
+            response = await await_me_maybe(hook)
+            if response is not None:
+                responses.append(response)
 
-            pdb.post_mortem(exception.__traceback__)
-
-        if rich is not None:
-            rich.get_console().print_exception(show_locals=True)
-
-        title = None
-        if isinstance(exception, Forbidden):
-            status = 403
-            info = {}
-            message = exception.args[0]
-            # Try the forbidden() plugin hook
-            for custom_response in pm.hook.forbidden(
-                datasette=self.ds, request=request, message=message
-            ):
-                custom_response = await await_me_maybe(custom_response)
-                if custom_response is not None:
-                    await custom_response.asgi_send(send)
-                    return
-        elif isinstance(exception, Base400):
-            status = exception.status
-            info = {}
-            message = exception.args[0]
-        elif isinstance(exception, DatasetteError):
-            status = exception.status
-            info = exception.error_dict
-            message = exception.message
-            if exception.message_is_html:
-                message = Markup(message)
-            title = exception.title
-        else:
-            status = 500
-            info = {}
-            message = str(exception)
-            traceback.print_exc()
-        templates = [f"{status}.html", "error.html"]
-        info.update(
-            {
-                "ok": False,
-                "error": message,
-                "status": status,
-                "title": title,
-            }
-        )
-        headers = {}
-        if self.ds.cors:
-            add_cors_headers(headers)
-        if request.path.split("?")[0].endswith(".json"):
-            await asgi_send_json(send, info, status=status, headers=headers)
-        else:
-            template = self.ds.jinja_env.select_template(templates)
-            await asgi_send_html(
-                send,
-                await template.render_async(
-                    dict(
-                        info,
-                        urls=self.ds.urls,
-                        app_css_hash=self.ds.app_css_hash(),
-                        menu_links=lambda: [],
-                    )
-                ),
-                status=status,
-                headers=headers,
-            )
+        assert responses, "Default exception handler should have returned something"
+        # Even if there are multiple responses use just the first one
+        response = responses[0]
+        await response.asgi_send(send)
 
 
 _cleaner_task_str_re = re.compile(r"\S*site-packages/")

datasette/forbidden.py

@@ -0,0 +1,20 @@
+from os import stat
+from datasette import hookimpl, Response
+
+
+@hookimpl(trylast=True)
+def forbidden(datasette, request, message):
+    async def inner():
+        return Response.html(
+            await datasette.render_template(
+                "error.html",
+                {
+                    "title": "Forbidden",
+                    "error": message,
+                },
+                request=request,
+            ),
+            status=403,
+        )
+
+    return inner

datasette/handle_exception.py

@@ -0,0 +1,74 @@
+from datasette import hookimpl, Response
+from .utils import await_me_maybe, add_cors_headers
+from .utils.asgi import (
+    Base400,
+    Forbidden,
+)
+from .views.base import DatasetteError
+from markupsafe import Markup
+import pdb
+import traceback
+from .plugins import pm
+
+try:
+    import rich
+except ImportError:
+    rich = None
+
+
+@hookimpl(trylast=True)
+def handle_exception(datasette, request, exception):
+    async def inner():
+        if datasette.pdb:
+            pdb.post_mortem(exception.__traceback__)
+
+        if rich is not None:
+            rich.get_console().print_exception(show_locals=True)
+
+        title = None
+        if isinstance(exception, Base400):
+            status = exception.status
+            info = {}
+            message = exception.args[0]
+        elif isinstance(exception, DatasetteError):
+            status = exception.status
+            info = exception.error_dict
+            message = exception.message
+            if exception.message_is_html:
+                message = Markup(message)
+            title = exception.title
+        else:
+            status = 500
+            info = {}
+            message = str(exception)
+            traceback.print_exc()
+        templates = [f"{status}.html", "error.html"]
+        info.update(
+            {
+                "ok": False,
+                "error": message,
+                "status": status,
+                "title": title,
+            }
+        )
+        headers = {}
+        if datasette.cors:
+            add_cors_headers(headers)
+        if request.path.split("?")[0].endswith(".json"):
+            return Response.json(info, status=status, headers=headers)
+        else:
+            template = datasette.jinja_env.select_template(templates)
+            return Response.html(
+                await template.render_async(
+                    dict(
+                        info,
+                        urls=datasette.urls,
+                        app_css_hash=datasette.app_css_hash(),
+                        menu_links=lambda: [],
+                    )
+                ),
+                status=status,
+                headers=headers,
+            )
+
+    return inner

datasette/hookspecs.py

@@ -138,3 +138,8 @@ def database_actions(datasette, actor, database, request):
 @hookspec
 def skip_csrf(datasette, scope):
     """Mechanism for skipping CSRF checks for certain requests"""
+
+
+@hookspec
+def handle_exception(datasette, request, exception):
+    """Handle an uncaught exception. Can return a Response or None."""

datasette/plugins.py

@@ -15,6 +15,8 @@ DEFAULT_PLUGINS = (
     "datasette.default_magic_parameters",
     "datasette.blob_renderer",
     "datasette.default_menu_links",
+    "datasette.handle_exception",
+    "datasette.forbidden",
 )
 
 pm = pluggy.PluginManager("datasette")

docs/plugin_hooks.rst

@@ -107,8 +107,8 @@ Extra template variables that should be made available in the rendered template
 ``view_name`` - string
     The name of the view being displayed. (``index``, ``database``, ``table``, and ``row`` are the most important ones.)
 
-``request`` - object or None
-    The current HTTP :ref:`internals_request`. This can be ``None`` if the request object is not available.
+``request`` - :ref:`internals_request` or None
+    The current HTTP request. This can be ``None`` if the request object is not available.
 
 ``datasette`` - :ref:`internals_datasette`
     You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``
@@ -504,7 +504,7 @@ When a request is received, the ``"render"`` callback function is called with ze
     The table or view, if one is being rendered.
 
 ``request`` - :ref:`internals_request`
-    The incoming HTTP request.
+    The current HTTP request.
 
 ``view_name`` - string
     The name of the current view being called. ``index``, ``database``, ``table``, and ``row`` are the most important ones.
@@ -599,8 +599,8 @@ The optional view function arguments are as follows:
 ``datasette`` - :ref:`internals_datasette`
     You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``, or to execute SQL queries.
 
-``request`` - Request object
-    The current HTTP :ref:`internals_request`.
+``request`` - :ref:`internals_request`
+    The current HTTP request.
 
 ``scope`` - dictionary
     The incoming ASGI scope dictionary.
@@ -947,8 +947,8 @@ actor_from_request(datasette, request)
 ``datasette`` - :ref:`internals_datasette`
     You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``, or to execute SQL queries.
 
-``request`` - object
-    The current HTTP :ref:`internals_request`.
+``request`` - :ref:`internals_request`
+    The current HTTP request.
 
 This is part of Datasette's :ref:`authentication and permissions system <authentication>`. The function should attempt to authenticate an actor (either a user or an API actor of some sort) based on information in the request.
 
@@ -1010,8 +1010,8 @@ Example: `datasette-auth-tokens <https://datasette.io/plugins/datasette-auth-tok
 filters_from_request(request, database, table, datasette)
 ---------------------------------------------------------
 
-``request`` - object
-    The current HTTP :ref:`internals_request`.
+``request`` - :ref:`internals_request`
+    The current HTTP request.
 
 ``database`` - string
     The name of the database.
@@ -1178,8 +1178,8 @@ forbidden(datasette, request, message)
 ``datasette`` - :ref:`internals_datasette`
     You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``, or to execute SQL queries.
 
-``request`` - object
-    The current HTTP :ref:`internals_request`.
+``request`` - :ref:`internals_request`
+    The current HTTP request.
 
 ``message`` - string
     A message hinting at why the request was forbidden.
@@ -1206,21 +1206,55 @@ The function can alternatively return an awaitable function if it needs to make
 
 .. code-block:: python
 
-    from datasette import hookimpl
-    from datasette.utils.asgi import Response
+    from datasette import hookimpl, Response
 
 
     @hookimpl
     def forbidden(datasette):
         async def inner():
             return Response.html(
-                await datasette.render_template(
-                    "forbidden.html"
-                )
+                await datasette.render_template("render_message.html", request=request)
             )
 
         return inner
 
+.. _plugin_hook_handle_exception:
+
+handle_exception(datasette, request, exception)
+-----------------------------------------------
+
+``datasette`` - :ref:`internals_datasette`
+    You can use this to access plugin configuration options via ``datasette.plugin_config(your_plugin_name)``, or to execute SQL queries.
+
+``request`` - :ref:`internals_request`
+    The current HTTP request.
+
+``exception`` - ``Exception``
+    The exception that was raised.
+
+This hook is called any time an unexpected exception is raised. You can use it to record the exception.
+
+If your handler returns a ``Response`` object it will be returned to the client in place of the default Datasette error page.
+
+The handler can return a response directly, or it can return return an awaitable function that returns a response.
+
+This example logs an error to `Sentry <https://sentry.io/>`__ and then renders a custom error page:
+
+.. code-block:: python
+
+    from datasette import hookimpl, Response
+    import sentry_sdk
+
+
+    @hookimpl
+    def handle_exception(datasette, exception):
+        sentry_sdk.capture_exception(exception)
+        async def inner():
+            return Response.html(
+                await datasette.render_template("custom_error.html", request=request)
+            )
+        return inner
+
 .. _plugin_hook_menu_links:
 
 menu_links(datasette, actor, request)
@@ -1232,8 +1266,8 @@ menu_links(datasette, actor, request)
 ``actor`` - dictionary or None
     The currently authenticated :ref:`actor <authentication_actor>`.
 
-``request`` - object or None
-    The current HTTP :ref:`internals_request`. This can be ``None`` if the request object is not available.
+``request`` - :ref:`internals_request`
+    The current HTTP request. This can be ``None`` if the request object is not available.
 
 This hook allows additional items to be included in the menu displayed by Datasette's top right menu icon.
 
@@ -1281,8 +1315,8 @@ table_actions(datasette, actor, database, table, request)
 ``table`` - string
     The name of the table.
 
-``request`` - object
-    The current HTTP :ref:`internals_request`. This can be ``None`` if the request object is not available.
+``request`` - :ref:`internals_request`
+    The current HTTP request. This can be ``None`` if the request object is not available.
 
 This hook allows table actions to be displayed in a menu accessed via an action icon at the top of the table page. It should return a list of ``{"href": "...", "label": "..."}`` menu items.
 
@@ -1325,8 +1359,8 @@ database_actions(datasette, actor, database, request)
 ``database`` - string
     The name of the database.
 
-``request`` - object
-    The current HTTP :ref:`internals_request`.
+``request`` - :ref:`internals_request`
+    The current HTTP request.
 
 This hook is similar to :ref:`plugin_hook_table_actions` but populates an actions menu on the database page.
 

tests/fixtures.py

@@ -68,6 +68,7 @@ EXPECTED_PLUGINS = [
             "canned_queries",
             "extra_js_urls",
             "extra_template_vars",
+            "handle_exception",
             "menu_links",
             "permission_allowed",
             "register_routes",

tests/plugins/my_plugin_2.py

@@ -185,3 +185,21 @@ def register_routes(datasette):
         # Also serves to demonstrate over-ride of default paths:
         (r"/(?P<db_name>[^/]+)/(?P<table_and_format>[^/]+?$)", new_table),
     ]
+
+
+@hookimpl
+def handle_exception(datasette, request, exception):
+    datasette._exception_hook_fired = (request, exception)
+    if request.args.get("_custom_error"):
+        return Response.text("_custom_error")
+    elif request.args.get("_custom_error_async"):
+
+        async def inner():
+            return Response.text("_custom_error_async")
+
+        return inner
+
+
+@hookimpl(specname="register_routes")
+def register_triger_error():
+    return ((r"/trigger-error", lambda: 1 / 0),)

tests/test_permissions.py

@@ -332,6 +332,7 @@ def test_permissions_debug(app_client):
     assert checks == [
         {"action": "permissions-debug", "result": True, "used_default": False},
         {"action": "view-instance", "result": None, "used_default": True},
+        {"action": "debug-menu", "result": False, "used_default": True},
         {"action": "permissions-debug", "result": False, "used_default": True},
         {"action": "view-instance", "result": None, "used_default": True},
     ]

tests/test_plugins.py

@@ -824,6 +824,20 @@ def test_hook_forbidden(restore_working_directory):
         assert "view-database" == client.ds._last_forbidden_message
 
 
+def test_hook_handle_exception(app_client):
+    app_client.get("/trigger-error?x=123")
+    assert hasattr(app_client.ds, "_exception_hook_fired")
+    request, exception = app_client.ds._exception_hook_fired
+    assert request.url == "http://localhost/trigger-error?x=123"
+    assert isinstance(exception, ZeroDivisionError)
+
+
+@pytest.mark.parametrize("param", ("_custom_error", "_custom_error_async"))
+def test_hook_handle_exception_custom_response(app_client, param):
+    response = app_client.get("/trigger-error?{}=1".format(param))
+    assert response.text == param
+
+
 def test_hook_menu_links(app_client):
     def get_menu_links(html):
         soup = Soup(html, "html.parser")