From 8b159144a58992f8fe1f5a0489a3566f57e8d20c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 23:48:49 +0000 Subject: [PATCH] Add Response.error() for JSON errors in the standard format Response.error(messages, status=400) builds a JSON error response in Datasette's standard error format, alongside Response.json/html/text. messages can be a single string or a list. All internal error response construction now uses it - the private views.base._error() helper is gone and the verbose Response.json(error_body(...), status=...) sites are converted. error_body() remains for the cases that merge the error keys into a larger payload (the JSON renderer, handle_exception and the permission debug payload builders). Since Response is public plugin API, plugins that build JSON endpoints now have an obvious way to return errors in the canonical shape. Documented in the internals documentation, including the guidance to raise Forbidden/NotFound/BadRequest/DatasetteError instead when the error should content-negotiate. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ --- datasette/app.py | 5 +- datasette/forbidden.py | 4 +- datasette/utils/asgi.py | 14 ++++- datasette/views/base.py | 9 +--- datasette/views/execute_write.py | 16 +++--- datasette/views/row.py | 32 ++++++------ datasette/views/special.py | 18 ++----- datasette/views/stored_queries.py | 70 ++++++++++++++----------- datasette/views/table.py | 74 ++++++++++++++------------- datasette/views/table_create_alter.py | 50 +++++++++--------- docs/internals.rst | 6 ++- tests/test_internals_response.py | 24 +++++++++ 12 files changed, 182 insertions(+), 140 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 463c9be2..9c7e768b 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -113,7 +113,6 @@ from .utils import ( detect_json1, add_cors_headers, display_actor, - error_body, escape_css_string, escape_sqlite, find_spatialite, @@ -2958,9 +2957,7 @@ class DatasetteRouter: headers = {"www-authenticate": 'Bearer error="invalid_token"'} if self.ds.cors: add_cors_headers(headers) - response = Response.json( - error_body([str(exception)], 401), status=401, headers=headers - ) + response = Response.error([str(exception)], 401, headers=headers) await response.asgi_send(send) async def handle_404(self, request, send, exception=None): diff --git a/datasette/forbidden.py b/datasette/forbidden.py index 3a81ac4f..91b1ff96 100644 --- a/datasette/forbidden.py +++ b/datasette/forbidden.py @@ -1,5 +1,5 @@ from datasette import hookimpl, Response -from .utils import add_cors_headers, error_body +from .utils import add_cors_headers @hookimpl(trylast=True) @@ -13,7 +13,7 @@ def forbidden(datasette, request, message): headers = {} if datasette.cors: add_cors_headers(headers) - return Response.json(error_body(message, 403), status=403, headers=headers) + return Response.error(message, 403, headers=headers) return Response.html( await datasette.render_template( "error.html", diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 7777308f..610b86f2 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -1,6 +1,6 @@ import json from typing import Optional -from datasette.utils import MultiParams, calculate_etag, sha256_file +from datasette.utils import MultiParams, calculate_etag, error_body, sha256_file from datasette.utils.multipart import ( parse_form_data, MultipartParseError, @@ -575,6 +575,18 @@ class Response: content_type="application/json; charset=utf-8", ) + @classmethod + def error(cls, messages, status=400, headers=None): + """ + A JSON error response using Datasette's standard error format. + + messages can be a single string or a list of strings. For errors + that should content-negotiate between JSON and HTML, raise + Forbidden, NotFound, BadRequest or DatasetteError instead and let + Datasette's error handling hooks build the response. + """ + return cls.json(error_body(messages, status), status=status, headers=headers) + @classmethod def redirect(cls, path, status=302, headers=None): headers = headers or {} diff --git a/datasette/views/base.py b/datasette/views/base.py index 88d16753..66e14a6d 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -5,7 +5,6 @@ import sys from datasette.utils.asgi import Request from datasette.utils import ( add_cors_headers, - error_body, EscapeHtmlWriter, InvalidSql, LimitedWriter, @@ -53,7 +52,7 @@ class View: request.path.endswith(".json") or request.headers.get("content-type") == "application/json" ): - response = Response.json(error_body("Method not allowed", 405), status=405) + response = Response.error("Method not allowed", 405) else: response = Response.text("Method not allowed", status=405) return response @@ -92,7 +91,7 @@ class BaseView: request.path.endswith(".json") or request.headers.get("content-type") == "application/json" ): - response = Response.json(error_body("Method not allowed", 405), status=405) + response = Response.error("Method not allowed", 405) else: response = Response.text("Method not allowed", status=405) return response @@ -180,10 +179,6 @@ class BaseView: return view -def _error(messages, status=400): - return Response.json(error_body(messages, status), status=status) - - async def stream_csv(datasette, fetch_data, request, database): kwargs = {} stream = request.args.get("_stream") diff --git a/datasette/views/execute_write.py b/datasette/views/execute_write.py index 6806e69d..dd35b127 100644 --- a/datasette/views/execute_write.py +++ b/datasette/views/execute_write.py @@ -5,7 +5,7 @@ from datasette.resources import DatabaseResource from datasette.utils import UNSTABLE_API_MESSAGE, sqlite3 from datasette.utils.asgi import Response -from .base import BaseView, _error +from .base import BaseView from .database import display_rows as display_query_rows from .query_helpers import ( QueryValidationError, @@ -348,7 +348,7 @@ class ExecuteWriteView(BaseView): ) if not db.is_mutable: return _block_framing( - _error( + Response.error( ["Cannot execute write SQL because this database is immutable."], 403, ) @@ -367,10 +367,10 @@ class ExecuteWriteView(BaseView): actor=request.actor, ): return _block_framing( - _error(["Permission denied: need execute-write-sql"], 403) + Response.error(["Permission denied: need execute-write-sql"], 403) ) if not db.is_mutable: - return _block_framing(_error(["Database is immutable"], 403)) + return _block_framing(Response.error(["Database is immutable"], 403)) data = {} is_json = request.headers.get("content-type", "").startswith("application/json") @@ -384,7 +384,7 @@ class ExecuteWriteView(BaseView): ) except QueryValidationError as ex: if _wants_json(request, is_json, data): - return _block_framing(_error([ex.message], ex.status)) + return _block_framing(Response.error([ex.message], ex.status)) if ex.flash: self.ds.add_message(request, ex.message, self.ds.ERROR) return await self._render_form( @@ -405,7 +405,7 @@ class ExecuteWriteView(BaseView): except sqlite3.DatabaseError as ex: message = str(ex) if wants_json: - return _block_framing(_error([message], 400)) + return _block_framing(Response.error([message], 400)) return await self._render_form( request, db, @@ -488,13 +488,13 @@ class ExecuteWriteAnalyzeView(BaseView): actor=request.actor, ): return _block_framing( - _error(["Permission denied: need execute-write-sql"], 403) + Response.error(["Permission denied: need execute-write-sql"], 403) ) invalid_keys = set(request.args) - {"sql"} if invalid_keys: return _block_framing( - _error( + Response.error( ["Invalid keys: {}".format(", ".join(sorted(invalid_keys)))], 400, ) diff --git a/datasette/views/row.py b/datasette/views/row.py index d9a3deeb..c90a3bbe 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -12,7 +12,7 @@ from datasette.utils.asgi import NotFound, Forbidden, PayloadTooLarge, Response from datasette.database import QueryInterrupted from datasette.events import UpdateRowEvent, DeleteRowEvent from datasette.resources import TableResource -from .base import BaseView, DatasetteError, _error, stream_csv +from .base import BaseView, DatasetteError, stream_csv from datasette.utils import ( add_cors_headers, await_me_maybe, @@ -715,11 +715,13 @@ async def _resolve_row_and_check_permission(datasette, request, permission): try: resolved = await datasette.resolve_row(request) except DatabaseNotFound as e: - return False, _error(["Database not found: {}".format(e.database_name)], 404) + return False, Response.error( + ["Database not found: {}".format(e.database_name)], 404 + ) except TableNotFound as e: - return False, _error(["Table not found: {}".format(e.table)], 404) + return False, Response.error(["Table not found: {}".format(e.table)], 404) except RowNotFound as e: - return False, _error(["Record not found: {}".format(e.pk_values)], 404) + return False, Response.error(["Record not found: {}".format(e.pk_values)], 404) # Ensure user has permission to delete this row if not await datasette.allowed( @@ -727,7 +729,7 @@ async def _resolve_row_and_check_permission(datasette, request, permission): resource=TableResource(database=resolved.db.name, table=resolved.table), actor=request.actor, ): - return False, _error(["Permission denied"], 403) + return False, Response.error(["Permission denied"], 403) return True, resolved @@ -752,7 +754,7 @@ class RowDeleteView(BaseView): try: await resolved.db.execute_write_fn(delete_row, request=request) except Exception as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) await self.ds.track_event( DeleteRowEvent( @@ -791,24 +793,24 @@ class RowUpdateView(BaseView): try: data = await request.json() except json.JSONDecodeError as e: - return _error(["Invalid JSON: {}".format(e)]) + return Response.error(["Invalid JSON: {}".format(e)]) except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if not isinstance(data, dict): - return _error(["JSON must be a dictionary"]) + return Response.error(["JSON must be a dictionary"]) if "update" not in data or not isinstance(data["update"], dict): - return _error(["JSON must contain an update dictionary"]) + return Response.error(["JSON must contain an update dictionary"]) invalid_keys = set(data.keys()) - {"update", "return", "alter"} if invalid_keys: - return _error(["Invalid keys: {}".format(", ".join(invalid_keys))]) + return Response.error(["Invalid keys: {}".format(", ".join(invalid_keys))]) update = data["update"] try: update = decode_write_json_row(update) except WriteJsonValueError as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) # Validate column types from datasette.views.table import _validate_column_types @@ -817,7 +819,7 @@ class RowUpdateView(BaseView): self.ds, resolved.db.name, resolved.table, [update] ) if ct_errors: - return _error(ct_errors, 400) + return Response.error(ct_errors, 400) alter = data.get("alter") if alter and not await self.ds.allowed( @@ -825,7 +827,7 @@ class RowUpdateView(BaseView): resource=TableResource(database=resolved.db.name, table=resolved.table), actor=request.actor, ): - return _error(["Permission denied for alter-table"], 403) + return Response.error(["Permission denied for alter-table"], 403) def update_row(conn): sqlite_utils.Database(conn)[resolved.table].update( @@ -835,7 +837,7 @@ class RowUpdateView(BaseView): try: await resolved.db.execute_write_fn(update_row, request=request) except Exception as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) result = {"ok": True} returned_row = None diff --git a/datasette/views/special.py b/datasette/views/special.py index 9386440c..c13191a1 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -501,13 +501,9 @@ class PermissionRulesView(BaseView): # JSON API - action parameter is required action = request.args.get("action") if not action: - return Response.json( - error_body("action parameter is required", 400), status=400 - ) + return Response.error("action parameter is required", 400) if action not in self.ds.actions: - return Response.json( - error_body(f"Unknown action: {action}", 404), status=404 - ) + return Response.error(f"Unknown action: {action}", 404) actor = request.actor if isinstance(request.actor, dict) else None @@ -516,15 +512,13 @@ class PermissionRulesView(BaseView): if page < 1: raise ValueError except ValueError: - return Response.json( - error_body("_page must be a positive integer", 400), status=400 - ) + return Response.error("_page must be a positive integer", 400) try: page_size = parse_size_limit( request.args.get("_size"), default=50, maximum=200 ) except ValueError as ex: - return Response.json(error_body(str(ex), 400), status=400) + return Response.error(str(ex), 400) offset = (page - 1) * page_size from datasette.utils.actions_sql import build_permission_rules_sql @@ -673,9 +667,7 @@ class PermissionCheckView(BaseView): # JSON API - action parameter is required action = request.args.get("action") if not action: - return Response.json( - error_body("action parameter is required", 400), status=400 - ) + return Response.error("action parameter is required", 400) parent = request.args.get("parent") child = request.args.get("child") diff --git a/datasette/views/stored_queries.py b/datasette/views/stored_queries.py index 3273d13c..d64f37d2 100644 --- a/datasette/views/stored_queries.py +++ b/datasette/views/stored_queries.py @@ -5,7 +5,7 @@ from datasette.stored_queries import stored_query_to_dict from datasette.utils import UNSTABLE_API_MESSAGE, sqlite3, tilde_decode from datasette.utils.asgi import Response -from .base import BaseView, _error +from .base import BaseView from .query_helpers import ( QueryValidationError, _as_bool, @@ -34,12 +34,14 @@ class QueryParametersView(BaseView): resource=DatabaseResource(db.name), actor=request.actor, ): - return _block_framing(_error(["Permission denied: need execute-sql"], 403)) + return _block_framing( + Response.error(["Permission denied: need execute-sql"], 403) + ) invalid_keys = set(request.args) - {"sql"} if invalid_keys: return _block_framing( - _error( + Response.error( ["Invalid keys: {}".format(", ".join(sorted(invalid_keys)))], 400, ) @@ -47,7 +49,7 @@ class QueryParametersView(BaseView): try: parameters = _derived_query_parameters(request.args.get("sql") or "") except QueryValidationError as ex: - return _block_framing(_error([ex.message], ex.status)) + return _block_framing(Response.error([ex.message], ex.status)) return _block_framing( Response.json( { @@ -95,7 +97,7 @@ class QueryListView(BaseView): is_write = _as_optional_bool(request.args.get("is_write"), "is_write") is_private = _as_optional_bool(request.args.get("is_private"), "is_private") except QueryValidationError as ex: - return _error([ex.message], ex.status) + return Response.error([ex.message], ex.status) page = await self.ds.list_queries( database, @@ -306,18 +308,22 @@ class QueryCreateAnalyzeView(BaseView): resource=DatabaseResource(db.name), actor=request.actor, ): - return _block_framing(_error(["Permission denied: need execute-sql"], 403)) + return _block_framing( + Response.error(["Permission denied: need execute-sql"], 403) + ) if not await self.ds.allowed( action="store-query", resource=DatabaseResource(db.name), actor=request.actor, ): - return _block_framing(_error(["Permission denied: need store-query"], 403)) + return _block_framing( + Response.error(["Permission denied: need store-query"], 403) + ) invalid_keys = set(request.args) - {"sql"} if invalid_keys: return _block_framing( - _error( + Response.error( ["Invalid keys: {}".format(", ".join(sorted(invalid_keys)))], 400, ) @@ -352,13 +358,13 @@ class QueryStoreView(QueryCreateView): resource=DatabaseResource(db.name), actor=request.actor, ): - return _error(["Permission denied: need execute-sql"], 403) + return Response.error(["Permission denied: need execute-sql"], 403) if not await self.ds.allowed( action="store-query", resource=DatabaseResource(db.name), actor=request.actor, ): - return _error(["Permission denied: need store-query"], 403) + return Response.error(["Permission denied: need store-query"], 403) is_json = False query_data = {} @@ -375,7 +381,7 @@ class QueryStoreView(QueryCreateView): return await self._error_response( request, db, query_data, ex.message, ex.status ) - return _error([ex.message], ex.status) + return Response.error([ex.message], ex.status) prepared.pop("analysis") name = prepared.pop("name") @@ -384,7 +390,7 @@ class QueryStoreView(QueryCreateView): except sqlite3.IntegrityError as ex: if not is_json and isinstance(query_data, dict): return await self._error_response(request, db, query_data, str(ex), 400) - return _error([str(ex)], 400) + return Response.error([str(ex)], 400) query = await self.ds.get_query(db.name, name) assert query is not None @@ -409,13 +415,13 @@ class QueryDefinitionView(BaseView): query_name = tilde_decode(request.url_vars["query"]) query = await self.ds.get_query(db.name, query_name) if query is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) if not await self.ds.allowed( action="view-query", resource=QueryResource(db.name, query_name), actor=request.actor, ): - return _error(["Permission denied"], 403) + return Response.error(["Permission denied"], 403) return Response.json( { "ok": True, @@ -433,15 +439,17 @@ class QueryUpdateView(BaseView): query_name = tilde_decode(request.url_vars["query"]) existing = await self.ds.get_query(db.name, query_name) if existing is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) if not await self.ds.allowed( action="update-query", resource=QueryResource(db.name, query_name), actor=request.actor, ): - return _error(["Permission denied: need update-query"], 403) + return Response.error(["Permission denied: need update-query"], 403) if existing.is_trusted: - return _error(["Trusted queries cannot be updated using the API"], 403) + return Response.error( + ["Trusted queries cannot be updated using the API"], 403 + ) try: data, _ = await _json_or_form_payload(request) @@ -467,7 +475,7 @@ class QueryUpdateView(BaseView): self.ds, request, db, existing, update ) except QueryValidationError as ex: - return _error([ex.message], ex.status) + return Response.error([ex.message], ex.status) await self.ds.update_query(db.name, query_name, **update_kwargs) if data.get("return"): @@ -524,32 +532,32 @@ class QueryEditView(BaseView): async def get(self, request): db, query_name, existing = await self._load(request) if existing is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) await self.ds.ensure_permission( action="update-query", resource=QueryResource(db.name, query_name), actor=request.actor, ) if existing.is_trusted: - return _error(["Trusted queries cannot be edited"], 403) + return Response.error(["Trusted queries cannot be edited"], 403) return await self._render_form(request, db, existing) async def post(self, request): db, query_name, existing = await self._load(request) if existing is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) if not await self.ds.allowed( action="update-query", resource=QueryResource(db.name, query_name), actor=request.actor, ): - return _error(["Permission denied: need update-query"], 403) + return Response.error(["Permission denied: need update-query"], 403) if existing.is_trusted: - return _error(["Trusted queries cannot be edited"], 403) + return Response.error(["Trusted queries cannot be edited"], 403) data, _ = await _json_or_form_payload(request) if not isinstance(data, dict): - return _error(["Invalid form submission"], 400) + return Response.error(["Invalid form submission"], 400) sql = data.get("sql") sql = existing.sql if sql is None else sql.strip() title = data.get("title") or "" @@ -621,14 +629,16 @@ class QueryDeleteView(BaseView): async def get(self, request): db, query_name, existing = await self._load(request) if existing is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) await self.ds.ensure_permission( action="delete-query", resource=QueryResource(db.name, query_name), actor=request.actor, ) if existing.is_trusted: - return _error(["Trusted queries cannot be deleted using the API"], 403) + return Response.error( + ["Trusted queries cannot be deleted using the API"], 403 + ) return await self.render( ["query_delete.html"], request, @@ -643,15 +653,17 @@ class QueryDeleteView(BaseView): async def post(self, request): db, query_name, existing = await self._load(request) if existing is None: - return _error(["Query not found: {}".format(query_name)], 404) + return Response.error(["Query not found: {}".format(query_name)], 404) if not await self.ds.allowed( action="delete-query", resource=QueryResource(db.name, query_name), actor=request.actor, ): - return _error(["Permission denied: need delete-query"], 403) + return Response.error(["Permission denied: need delete-query"], 403) if existing.is_trusted: - return _error(["Trusted queries cannot be deleted using the API"], 403) + return Response.error( + ["Trusted queries cannot be deleted using the API"], 403 + ) data, is_json = await _json_or_form_payload(request) await self.ds.remove_query(db.name, query_name) diff --git a/datasette/views/table.py b/datasette/views/table.py index 80ad1aab..8ce4b711 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -60,7 +60,7 @@ from dataclasses import dataclass, field from datasette.extras import ExtraScope from . import Context, from_extra -from .base import BaseView, DatasetteError, _error, stream_csv +from .base import BaseView, DatasetteError, stream_csv from .database import QueryView from .table_create_alter import ( ALTER_TABLE_COLUMN_TYPES, @@ -1035,7 +1035,7 @@ class TableInsertView(BaseView): try: resolved = await self.ds.resolve_table(request) except NotFound as e: - return _error([e.args[0]], 404) + return Response.error([e.args[0]], 404) db = resolved.db database_name = db.name table_name = resolved.table @@ -1043,7 +1043,7 @@ class TableInsertView(BaseView): # Table must exist (may handle table creation in the future) db = self.ds.get_database(database_name) if not await db.table_exists(table_name): - return _error(["Table not found: {}".format(table_name)], 404) + return Response.error(["Table not found: {}".format(table_name)], 404) if upsert: # Must have insert-row AND upsert-row permissions @@ -1059,7 +1059,7 @@ class TableInsertView(BaseView): actor=request.actor, ) ): - return _error( + return Response.error( ["Permission denied: need both insert-row and update-row"], 403 ) else: @@ -1069,10 +1069,10 @@ class TableInsertView(BaseView): resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied"], 403) + return Response.error(["Permission denied"], 403) if not db.is_mutable: - return _error(["Database is immutable"], 403) + return Response.error(["Database is immutable"], 403) pks = await db.primary_keys(table_name) @@ -1081,20 +1081,20 @@ class TableInsertView(BaseView): request, db, table_name, pks, upsert ) except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if errors: - return _error(errors, 400) + return Response.error(errors, 400) try: rows = decode_write_json_rows(rows) except WriteJsonValueError as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) # Validate column types ct_errors = await _validate_column_types( self.ds, database_name, table_name, rows ) if ct_errors: - return _error(ct_errors, 400) + return Response.error(ct_errors, 400) num_rows = len(rows) @@ -1108,14 +1108,16 @@ class TableInsertView(BaseView): alter = extras.get("alter") if upsert and (ignore or replace): - return _error(["Upsert does not support ignore or replace"], 400) + return Response.error(["Upsert does not support ignore or replace"], 400) if replace and not await self.ds.allowed( action="update-row", resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(['Permission denied: need update-row to use "replace"'], 403) + return Response.error( + ['Permission denied: need update-row to use "replace"'], 403 + ) initial_schema = None if alter: @@ -1125,7 +1127,7 @@ class TableInsertView(BaseView): resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied for alter-table"], 403) + return Response.error(["Permission denied for alter-table"], 403) # Track initial schema to check if it changed later initial_schema = await db.execute_fn( lambda conn: sqlite_utils.Database(conn)[table_name].schema @@ -1165,7 +1167,7 @@ class TableInsertView(BaseView): try: rows = await db.execute_write_fn(insert_or_upsert_rows, request=request) except Exception as e: - return _error([str(e)]) + return Response.error([str(e)]) result = {"ok": True} if should_return: if upsert: @@ -1246,7 +1248,7 @@ class TableSetColumnTypeView(BaseView): try: resolved = await self.ds.resolve_table(request) except NotFound as e: - return _error([e.args[0]], 404) + return Response.error([e.args[0]], 404) database_name = resolved.db.name table_name = resolved.table @@ -1256,39 +1258,39 @@ class TableSetColumnTypeView(BaseView): resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied"], 403) + return Response.error(["Permission denied"], 403) try: data = await request.json() except json.JSONDecodeError as e: - return _error(["Invalid JSON: {}".format(e)], 400) + return Response.error(["Invalid JSON: {}".format(e)], 400) except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if not isinstance(data, dict): - return _error(["JSON must be a dictionary"], 400) + return Response.error(["JSON must be a dictionary"], 400) invalid_keys = set(data.keys()) - {"column", "column_type"} if invalid_keys: - return _error( + return Response.error( ['Invalid parameter: "{}"'.format('", "'.join(sorted(invalid_keys)))], 400, ) if "column" not in data: - return _error(['"column" is required'], 400) + return Response.error(['"column" is required'], 400) column = data["column"] if not isinstance(column, str): - return _error(['"column" must be a string'], 400) + return Response.error(['"column" must be a string'], 400) if "column_type" not in data: - return _error(['"column_type" is required'], 400) + return Response.error(['"column_type" is required'], 400) column_details = await self.ds._get_resource_column_details( database_name, table_name ) if column not in column_details: - return _error(["Column not found: {}".format(column)], 400) + return Response.error(["Column not found: {}".format(column)], 400) column_type_data = data["column_type"] if column_type_data is None: @@ -1305,11 +1307,11 @@ class TableSetColumnTypeView(BaseView): ) if not isinstance(column_type_data, dict): - return _error(['"column_type" must be an object or null'], 400) + return Response.error(['"column_type" must be an object or null'], 400) invalid_column_type_keys = set(column_type_data.keys()) - {"type", "config"} if invalid_column_type_keys: - return _error( + return Response.error( [ 'Invalid column_type parameter: "{}"'.format( '", "'.join(sorted(invalid_column_type_keys)) @@ -1319,24 +1321,24 @@ class TableSetColumnTypeView(BaseView): ) if "type" not in column_type_data: - return _error(['"column_type.type" is required'], 400) + return Response.error(['"column_type.type" is required'], 400) column_type = column_type_data["type"] if not isinstance(column_type, str): - return _error(['"column_type.type" must be a string'], 400) + return Response.error(['"column_type.type" must be a string'], 400) config = column_type_data.get("config") if config is not None and not isinstance(config, dict): - return _error(['"column_type.config" must be a dictionary'], 400) + return Response.error(['"column_type.config" must be a dictionary'], 400) if column_type not in self.ds._column_types: - return _error(["Unknown column type: {}".format(column_type)], 400) + return Response.error(["Unknown column type: {}".format(column_type)], 400) try: await self.ds.set_column_type( database_name, table_name, column, column_type, config ) except ValueError as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) return Response.json( { @@ -1360,22 +1362,22 @@ class TableDropView(BaseView): try: resolved = await self.ds.resolve_table(request) except NotFound as e: - return _error([e.args[0]], 404) + return Response.error([e.args[0]], 404) db = resolved.db database_name = db.name table_name = resolved.table # Table must exist db = self.ds.get_database(database_name) if not await db.table_exists(table_name): - return _error(["Table not found: {}".format(table_name)], 404) + return Response.error(["Table not found: {}".format(table_name)], 404) if not await self.ds.allowed( action="drop-table", resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied"], 403) + return Response.error(["Permission denied"], 403) if not db.is_mutable: - return _error(["Database is immutable"], 403) + return Response.error(["Database is immutable"], 403) confirm = False try: data = await request.json() @@ -1383,7 +1385,7 @@ class TableDropView(BaseView): except json.JSONDecodeError: pass except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if not confirm: return Response.json( diff --git a/datasette/views/table_create_alter.py b/datasette/views/table_create_alter.py index c3e06c29..4deeafcc 100644 --- a/datasette/views/table_create_alter.py +++ b/datasette/views/table_create_alter.py @@ -29,7 +29,7 @@ from datasette.utils import ( from datasette.utils.asgi import NotFound, PayloadTooLarge, Response from datasette.utils.sqlite import sqlite_hidden_table_names -from .base import BaseView, _error +from .base import BaseView CREATE_TABLE_COLUMN_TYPES = ["text", "integer", "float", "blob"] CREATE_TABLE_SQLITE_TYPES = { @@ -805,22 +805,22 @@ class TableCreateView(BaseView): resource=DatabaseResource(database=database_name), actor=request.actor, ): - return _error(["Permission denied"], 403) + return Response.error(["Permission denied"], 403) try: data = await request.json() except json.JSONDecodeError as e: - return _error(["Invalid JSON: {}".format(e)]) + return Response.error(["Invalid JSON: {}".format(e)]) except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if not isinstance(data, dict): - return _error(["JSON must be an object"]) + return Response.error(["JSON must be an object"]) try: create_request = CreateTableRequest.model_validate(data) except ValidationError as e: - return _error(_create_table_pydantic_errors(e)) + return Response.error(_create_table_pydantic_errors(e)) ignore = create_request.ignore replace = create_request.replace @@ -832,7 +832,7 @@ class TableCreateView(BaseView): resource=DatabaseResource(database=database_name), actor=request.actor, ): - return _error(["Permission denied: need update-row"], 403) + return Response.error(["Permission denied: need update-row"], 403) table_name = create_request.table table_exists = await db.table_exists(table_name) @@ -846,11 +846,11 @@ class TableCreateView(BaseView): resource=DatabaseResource(database=database_name), actor=request.actor, ): - return _error(["Permission denied: need insert-row"], 403) + return Response.error(["Permission denied: need insert-row"], 403) try: rows = decode_write_json_rows(rows) except WriteJsonValueError as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) alter = False if rows: @@ -865,7 +865,9 @@ class TableCreateView(BaseView): resource=DatabaseResource(database=database_name), actor=request.actor, ): - return _error(["Permission denied: need alter-table"], 403) + return Response.error( + ["Permission denied: need alter-table"], 403 + ) alter = True pk = create_request.pk @@ -881,7 +883,7 @@ class TableCreateView(BaseView): elif len(actual_pks) > 1 and pks and set(pks) != set(actual_pks): bad_pks = True if bad_pks: - return _error(["pk cannot be changed for existing table"]) + return Response.error(["pk cannot be changed for existing table"]) pks = actual_pks initial_schema = None @@ -924,7 +926,7 @@ class TableCreateView(BaseView): try: schema = await db.execute_write_fn(create_table, request=request) except Exception as e: - return _error([str(e)]) + return Response.error([str(e)]) if initial_schema is not None and initial_schema != schema: await self.ds.track_event( @@ -999,7 +1001,7 @@ class DatabaseForeignKeyTargetsView(BaseView): actor=request.actor, ) if not (can_create_table or can_alter_table): - return _error(["Permission denied: need create-table"], 403) + return Response.error(["Permission denied: need create-table"], 403) hidden_tables = await db.execute_fn( lambda conn: set(sqlite_hidden_table_names(conn)) @@ -1028,21 +1030,21 @@ class TableForeignKeySuggestionsView(BaseView): try: resolved = await self.ds.resolve_table(request) except NotFound as e: - return _error([e.args[0]], 404) + return Response.error([e.args[0]], 404) db = resolved.db database_name = db.name table_name = resolved.table if resolved.is_view: - return _error(["Cannot suggest foreign keys for a view"], 400) + return Response.error(["Cannot suggest foreign keys for a view"], 400) if not await self.ds.allowed( action="alter-table", resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied: need alter-table"], 403) + return Response.error(["Permission denied: need alter-table"], 403) source_columns, targets, current_by_column = await db.execute_fn( lambda conn: _foreign_key_suggestion_metadata(conn, table_name) @@ -1150,7 +1152,7 @@ class TableAlterView(BaseView): try: resolved = await self.ds.resolve_table(request) except NotFound as e: - return _error([e.args[0]], 404) + return Response.error([e.args[0]], 404) db = resolved.db database_name = db.name @@ -1161,25 +1163,25 @@ class TableAlterView(BaseView): resource=TableResource(database=database_name, table=table_name), actor=request.actor, ): - return _error(["Permission denied: need alter-table"], 403) + return Response.error(["Permission denied: need alter-table"], 403) if not db.is_mutable: - return _error(["Database is immutable"], 403) + return Response.error(["Database is immutable"], 403) try: data = await request.json() except json.JSONDecodeError as e: - return _error(["Invalid JSON: {}".format(e)], 400) + return Response.error(["Invalid JSON: {}".format(e)], 400) except PayloadTooLarge as e: - return _error([str(e)], 413) + return Response.error([str(e)], 413) if not isinstance(data, dict): - return _error(["JSON must be a dictionary"], 400) + return Response.error(["JSON must be a dictionary"], 400) try: alter_request = AlterTableRequest.model_validate(data) except ValidationError as e: - return _error(_pydantic_errors(e), 400) + return Response.error(_pydantic_errors(e), 400) def alter_table(conn): before_schema = _table_schema_from_conn(conn, table_name) @@ -1328,7 +1330,7 @@ class TableAlterView(BaseView): alter_table, request=request ) except Exception as e: - return _error([str(e)], 400) + return Response.error([str(e)], 400) altered = before_schema != after_schema if altered: diff --git a/docs/internals.rst b/docs/internals.rst index e5e7aca5..7258e6f3 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -284,7 +284,7 @@ For example: content_type="application/xml; charset=utf-8", ) -The quickest way to create responses is using the ``Response.text(...)``, ``Response.html(...)``, ``Response.json(...)`` or ``Response.redirect(...)`` helper methods: +The quickest way to create responses is using the ``Response.text(...)``, ``Response.html(...)``, ``Response.json(...)``, ``Response.error(...)`` or ``Response.redirect(...)`` helper methods: .. code-block:: python @@ -295,6 +295,8 @@ The quickest way to create responses is using the ``Response.text(...)``, ``Resp text_response = Response.text( "This will become utf-8 encoded text" ) + # A JSON error in Datasette's standard error format: + error_response = Response.error("Cannot do that", 400) # Redirects are served as 302, unless you pass status=301: redirect_response = Response.redirect( "https://latest.datasette.io/" @@ -304,6 +306,8 @@ Each of these responses will use the correct corresponding content-type - ``text Each of the helper methods take optional ``status=`` and ``headers=`` arguments, documented above. +``Response.error(messages, status=400)`` returns a JSON error in the :ref:`standard Datasette error format `. ``messages`` can be a single string or a list of strings. Use this for JSON-only endpoints; if your error should content-negotiate between JSON and HTML, raise ``Forbidden``, ``NotFound``, ``BadRequest`` or ``DatasetteError`` instead and Datasette's error handling will build the appropriate response. + .. _internals_response_asgi_send: Returning a response with .asgi_send(send) diff --git a/tests/test_internals_response.py b/tests/test_internals_response.py index 820b20b2..2366dcde 100644 --- a/tests/test_internals_response.py +++ b/tests/test_internals_response.py @@ -1,4 +1,5 @@ from datasette.utils.asgi import Response +import json import pytest @@ -52,3 +53,26 @@ async def test_response_set_cookie(): }, {"type": "http.response.body", "body": b""}, ] == events + + +def test_response_error_single_message(): + response = Response.error("Method not allowed", 405) + assert response.status == 405 + assert response.content_type == "application/json; charset=utf-8" + assert json.loads(response.body) == { + "ok": False, + "error": "Method not allowed", + "errors": ["Method not allowed"], + "status": 405, + } + + +def test_response_error_message_list_and_default_status(): + response = Response.error(["First problem", "Second problem"]) + assert response.status == 400 + assert json.loads(response.body) == { + "ok": False, + "error": "First problem; Second problem", + "errors": ["First problem", "Second problem"], + "status": 400, + }