diff --git a/datasette/views/row.py b/datasette/views/row.py index c99b75d1..bdd78ed4 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -742,7 +742,7 @@ class RowDeleteView(BaseView): try: await resolved.db.execute_write_fn(delete_row, request=request) except Exception as e: - return _error([str(e)], 500) + return _error([str(e)], 400) await self.ds.track_event( DeleteRowEvent( diff --git a/existing-api.md b/existing-api.md index e0842804..d600179e 100644 --- a/existing-api.md +++ b/existing-api.md @@ -968,8 +968,8 @@ does not change the SQLite schema. - **Request:** no body required (any body is ignored — there is no confirmation step, unlike table drop). - **Response** — 200 `{"ok": true}`; with `?_redirect_to_table` a `redirect` - key is added. A failure during the write returns **500** with the message - (unlike update's 400). Emits `delete-row`. + key is added. A failure during the write returns 400 with the message, + matching update. Emits `delete-row`. --- diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index c878cb70..4fec5a00 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -98,10 +98,11 @@ key — see §2.) ### 1c. Wrong-status outliers (P2) -- Row **delete** write failures return **500** (views/row.py:757) while row +- ~~Row **delete** write failures return **500** (views/row.py:757) while row **update** write failures return **400** (views/row.py:832-835). Same failure class, different status; pick 400 (or 409 for constraint - violations) for both. + violations) for both.~~ ✅ **Done** — delete now returns 400, matching + update and the rest of the write API. - Invalid or expired bearer tokens silently degrade the request to anonymous, so clients see a 403 permission error (or worse, anonymous-permitted data) rather than a 401 (tokens.py:147-193). For 1.0, a malformed/expired @@ -337,7 +338,8 @@ Concerns: []}`** while `.csv` on the same request returns 400 `"?sql= is required"`. The JSON behavior masks caller bugs; return 400 on both. 3. **`_shape=object` HTTP 200 error** (§1b) — almost certainly unintended. -4. **Row delete 500** (§1c) — inconsistent with every sibling endpoint. +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` value** (views/database.py:805-820). Error strings in the JSON API should be plain text. diff --git a/tests/test_error_shape.py b/tests/test_error_shape.py index dd1fcfc9..21d37cc2 100644 --- a/tests/test_error_shape.py +++ b/tests/test_error_shape.py @@ -363,3 +363,35 @@ async def test_write_query_rejected_operation_is_canonical_403(ds_write_query): ) data = assert_canonical_error(response, 403) assert data["redirect"] is None + + +# Row delete write failures must be 400, matching row update + + +@pytest.mark.asyncio +async def test_row_delete_write_failure_is_400(tmp_path_factory): + db_directory = tmp_path_factory.mktemp("dbs") + db_path = str(db_directory / "data.db") + conn = sqlite3.connect(db_path) + conn.execute("vacuum") + conn.execute("create table docs (id integer primary key, title text)") + conn.execute("insert into docs (id, title) values (1, 'One')") + conn.execute( + "create trigger no_delete before delete on docs " + "begin select raise(abort, 'deletes are blocked'); end" + ) + conn.commit() + conn.close() + ds = Datasette([db_path]) + ds.root_enabled = True + try: + response = await ds.client.post( + "/data/docs/1/-/delete", + json={}, + headers={"Content-Type": "application/json"}, + actor={"id": "root"}, + ) + data = assert_canonical_error(response, 400) + assert "deletes are blocked" in data["error"] + finally: + ds.close()