diff --git a/datasette/app.py b/datasette/app.py index 43e60dbc..edd05bb3 100644 --- a/datasette/app.py +++ b/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/") diff --git a/datasette/forbidden.py b/datasette/forbidden.py new file mode 100644 index 00000000..156a44d4 --- /dev/null +++ b/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 diff --git a/datasette/handle_exception.py b/datasette/handle_exception.py new file mode 100644 index 00000000..8b7e83e3 --- /dev/null +++ b/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 diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py index c84db0a3..a5fb536f 100644 --- a/datasette/hookspecs.py +++ b/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.""" diff --git a/datasette/plugins.py b/datasette/plugins.py index 76b46a47..fef0c8e9 100644 --- a/datasette/plugins.py +++ b/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") diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index f5c3ee83..6020a941 100644 --- a/docs/plugin_hooks.rst +++ b/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 `. 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 `__ 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 `. -``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. diff --git a/tests/fixtures.py b/tests/fixtures.py index e0e4ec7b..c145ac78 100644 --- a/tests/fixtures.py +++ b/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", diff --git a/tests/plugins/my_plugin_2.py b/tests/plugins/my_plugin_2.py index f5ce36b3..4df02343 100644 --- a/tests/plugins/my_plugin_2.py +++ b/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[^/]+)/(?P[^/]+?$)", 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),) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index f4169dbe..2a519e76 100644 --- a/tests/test_permissions.py +++ b/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}, ] diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 4a7ad7c6..948a40b8 100644 --- a/tests/test_plugins.py +++ b/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")