handle_exception plugin hook for custom error handling

[simonw]

I need this for a couple of plugins, both of which are broken at the moment:

• Errors no longer get passed to Sentry datasette-sentry#1
• Make sure this still works with latest Datasette datasette-show-errors#2

[simonw]

I think the hook gets called any time any exception makes it to this function:

datasette/datasette/app.py

Lines 1374 to 1440 in 950cc76

	async def handle_500(self, request, send, exception):
	if self.ds.pdb:
	import pdb
	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,
	)

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.

[simonw]

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.

[simonw]

... 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?

[simonw]

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.

[simonw]

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.

[simonw]

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.

[simonw]

I'll implement this hook and then release it as 0.62a1.

[simonw]

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).

[simonw]

Had to lookup that Base400 thing:

datasette/datasette/utils/asgi.py

Lines 16 to 29 in 950cc76

	class Base400(Exception):
	status = 400
	class NotFound(Base400):
	status = 404
	class Forbidden(Base400):
	status = 403
	class BadRequest(Base400):
	status = 400

[simonw]

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:

datasette/datasette/app.py

Lines 1384 to 1395 in 8188f55

	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

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.

[simonw]

I think this is the right place to move the code to catch Forbidden exceptions (and forwards them to that plugin hook):

datasette/datasette/app.py

Lines 1269 to 1278 in 8188f55

	try:
	response = await view(request, send)
	if response:
	self.ds._write_messages_to_response(request, response)
	await response.asgi_send(send)
	return
	except NotFound as exception:
	return await self.handle_404(request, send, exception)
	except Exception as exception:
	return await self.handle_exception(request, send, exception)

[simonw]

Keeping this issue open until I've proven the new plugin hook works by releasing a plugin that uses it.

[simonw]

Documentation for the new hook: https://docs.datasette.io/en/latest/plugin_hooks.html#handle-exception-datasette-request-exception

[simonw]

I've tested this with datasette-sentry and it works well.