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, + }