mirror of
https://github.com/simonw/datasette.git
synced 2026-07-08 08:34:42 +02:00
Return 400 instead of 500 for row delete write failures
Row delete previously returned 500 when the write failed (for example a constraint violation raised by a trigger or foreign key), while row update and every other write endpoint report the same failure class as 400. Delete now matches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
e8048e023f
commit
b2cdc81d34
4 changed files with 40 additions and 6 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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`.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue