crud.create_account leaks sqlalchemy IntegrityError on duplicate name #36
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
crud.create_account()does an uncheckeddb.insert("accounts", account). When the name is already in theaccountstable — which happens routinely via the background account-sync task that auto-imports Beancount Open directives — the underlying SQLite UNIQUE constraint raises:That bubbles out of the FastAPI endpoint as a 500 Internal Server Error with the raw SQL exception text.
Reproduce
Easy to hit from the test harness:
MINIMAL_LEDGERdeclares2020-01-01 open Assets:Cash EUR,SATS.wait_for_account_syncbackground task ingests that Open into libra'saccountstable./libra/api/v1/accountswith{"name": "Assets:Cash", "account_type": "asset"}to register it via the API.Caller can't easily distinguish "the account I wanted to make already exists" from "the database is broken."
Fix
Catch
IntegrityErrorincrud.create_account(or in the endpoint wrapper) and translate to a clean 409 Conflict with a structured detail. A minimal change:Workaround in place
The test harness (
tests/conftest.py's_create) currently does a get-then-create — list/accountsfirst and only POST if missing. That avoids the race but masks the underlying bug from any production caller. Filing this so the proper fix lands.Scope
crud.py: create_account/api/v1/accountsPOST endpoint if we want the HTTPException translation there instead of in CRUD.Small cleanup, not blocking.