diff --git a/datasette/handle_exception.py b/datasette/handle_exception.py index fc290dbc..e255ddf2 100644 --- a/datasette/handle_exception.py +++ b/datasette/handle_exception.py @@ -28,6 +28,7 @@ def handle_exception(datasette, request, exception): rich.get_console().print_exception(show_locals=True) title = None + plain_message = None if isinstance(exception, Base400): status = exception.status info = {} @@ -36,6 +37,7 @@ def handle_exception(datasette, request, exception): status = exception.status info = exception.error_dict message = exception.message + plain_message = exception.plain_message if exception.message_is_html: message = Markup(message) title = exception.title @@ -50,7 +52,7 @@ def handle_exception(datasette, request, exception): add_cors_headers(headers) if request.path.split("?")[0].endswith(".json"): body = dict(info) - body.update(error_body(message, status)) + body.update(error_body(plain_message or message, status)) return Response.json(body, status=status, headers=headers) info.update( { diff --git a/datasette/views/base.py b/datasette/views/base.py index a12ae050..88d16753 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -29,12 +29,15 @@ class DatasetteError(Exception): status=500, template=None, message_is_html=False, + plain_message=None, ): self.message = message self.title = title self.error_dict = error_dict or {} self.status = status self.message_is_html = message_is_html + # Plain text used for JSON error responses when message is HTML + self.plain_message = plain_message class View: diff --git a/datasette/views/database.py b/datasette/views/database.py index c9dcfa89..10dc66ae 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -819,6 +819,10 @@ class QueryView(View): title="SQL Interrupted", status=400, message_is_html=True, + plain_message=( + "SQL query took too long. The time limit is" + " controlled by the sql_time_limit_ms setting." + ), ) except sqlite3.DatabaseError as ex: query_error = str(ex) diff --git a/datasette/views/row.py b/datasette/views/row.py index 2a103180..d9a3deeb 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -200,6 +200,10 @@ class RowView(BaseView): title="SQL Interrupted", status=400, message_is_html=True, + plain_message=( + "SQL query took too long. The time limit is" + " controlled by the sql_time_limit_ms setting." + ), ) except (sqlite3.OperationalError, InvalidSql) as e: raise DatasetteError(str(e), title="Invalid SQL", status=400) diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index 4da23934..0d817612 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -394,9 +394,11 @@ Concerns: ✅ Done — now 400 (fixed with §1b). 4. ~~**Row delete 500** (§1c) — inconsistent with every sibling endpoint.~~ ✅ Done — now 400. -5. **The "SQL Interrupted" error embeds an HTML fragment in the JSON `error` +5. ~~**The "SQL Interrupted" error embeds an HTML fragment in the JSON `error` value** (views/database.py:805-820). Error strings in the JSON API should - be plain text. + be plain text.~~ ✅ **Done** — `DatasetteError` gained a `plain_message` + used for JSON responses; the HTML error page keeps the rich version with + the SQL textarea. §8 is now fully resolved. --- diff --git a/tests/test_api.py b/tests/test_api.py index da0ebc07..9a96f14f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -329,14 +329,8 @@ def test_sql_time_limit(app_client_shorter_time_limit): ) assert 400 == response.status expected_message = ( - "

SQL query took too long. The time limit is controlled by the\n" - 'sql_time_limit_ms\n' - "configuration option.

\n" - '\n' - "" + "SQL query took too long. The time limit is" + " controlled by the sql_time_limit_ms setting." ) assert response.json == { "ok": False, @@ -356,7 +350,7 @@ async def test_custom_sql_time_limit(ds_client): "/fixtures/-/query.json?sql=select+sleep(0.01)&_timelimit=5", ) assert response.status_code == 400 - assert response.json()["error"].startswith("

SQL query took too long.") + assert response.json()["error"].startswith("SQL query took too long.") @pytest.mark.asyncio diff --git a/tests/test_error_shape.py b/tests/test_error_shape.py index 040e645a..44856b36 100644 --- a/tests/test_error_shape.py +++ b/tests/test_error_shape.py @@ -717,3 +717,25 @@ async def test_alter_and_set_column_type_ignore_content_type(ds_error_shape): actor={"id": "root"}, ) assert sct.status_code == 200, sct.text + + +# SQL Interrupted errors carry plain text in JSON, not an HTML fragment + + +@pytest.mark.asyncio +async def test_sql_interrupted_json_error_is_plain_text(ds_client): + response = await ds_client.get( + "/fixtures/-/query.json?sql=select+sleep(0.01)&_timelimit=5" + ) + data = assert_canonical_error(response, 400) + assert "<" not in data["error"] + assert data["error"].startswith("SQL query took too long.") + + +@pytest.mark.asyncio +async def test_sql_interrupted_html_page_keeps_rich_error(ds_client): + response = await ds_client.get( + "/fixtures/-/query?sql=select+sleep(0.01)&_timelimit=5" + ) + assert response.status_code == 400 + assert "