datasette
datasette copied to clipboard
simonw
`handle_exception` plugin hook for custom error handling
I need this for a couple of plugins, both of which are broken at the moment:
- https://github.com/simonw/datasette-sentry/issues/1
- https://github.com/simonw/datasette-show-errors/issues/2
I think the hook gets called any time any exception makes it to this function: https://github.com/simonw/datasette/blob/950cc7677f65aa2543067b3bbfc2b6acb98b62c8/datasette/app.py#L1374-L1440
Multiple plugins can register for the hook. If they return a Response then that's returned to the user - if they return None then they can quietly do something with the error (log it to Sentry for example) and let some other handler return a response.
I think Datasette should have a default plugin hook implementation which returns the 500 error page.
Proposed hook design:
@hookspec
def handle_exception(datasette, request, exception):
"""Handle an uncaught exception"""
It takes request in case it needs to render a template and pass the request to it.
... maybe it should take send? But then how would plugins know that another plugin hadn't already used send to send a response, and avoid two trying to send at the same time?
It's weird to return a Response though because the code in question lives in DatasetteRouter which currently works outside of the layer where responses happen - everything in that class at the moment works using send directly.
Returning a Response is by far the most intuitive way to design this though. Plugin authors shouldn't care that DatasetteRouter (an undocumented internal API of Datasette) doesn't currently handle responses.
Design decision:
@hookspec
def handle_exception(datasette, request, exception):
"""Handle an uncaught exception. Can return a Response or None."""
It will also support returning an awaitable, using the await_me_maybe pattern.
In DatasetteRouter I'm going to rename handle_500 to handle_exception because it already deals with error codes other than 500 (Forbidden exceptions become 403 and Base400 become 400).
Had to lookup that Base400 thing: https://github.com/simonw/datasette/blob/950cc7677f65aa2543067b3bbfc2b6acb98b62c8/datasette/utils/asgi.py#L16-L29
I need to refactor this code so that Forbidden exceptions are handled separately.
Reason is that those already have a plugin hook of their own:
https://github.com/simonw/datasette/blob/8188f55efc0fcca1be692b0d0c875f2d1ee99f17/datasette/app.py#L1384-L1395
My first attempt at this refactored that entire handle_exception method to call the new plugin hook, moving its current implementation into a datasette/handle_exception.py default plugin implementation - but it felt weird having that plugin implementation then itself call the pm.hook.forbidden() hook.
I think this is the right place to move the code to catch Forbidden exceptions (and forwards them to that plugin hook): https://github.com/simonw/datasette/blob/8188f55efc0fcca1be692b0d0c875f2d1ee99f17/datasette/app.py#L1269-L1278
Keeping this issue open until I've proven the new plugin hook works by releasing a plugin that uses it.
Documentation for the new hook: https://docs.datasette.io/en/latest/plugin_hooks.html#handle-exception-datasette-request-exception