DeleteDB returns on the first failing suffix, so it can report a leftover file when the history is already destroyed #275
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/webhooker#275
Reference in New Issue
Block a user
Found during the review of #273. Pre-existing, not introduced there.
DeleteDB(internal/database/webhook_db_manager.go) unlinks the suffixes{"", "-wal", "-shm"}in a loop and returns on the FIRST failure. So if the main.dbunlink succeeds and the-walunlink then fails, the caller receives an error and — since #273 — correctly reports it to the operator as "the event database file was left behind, remove it by hand".That message is wrong in exactly that case. The main database is already gone; the history is destroyed. What is left behind is a
-walfragment, not the data. An operator told to clean up a leftover file will delete it and believe nothing was lost.Why this is worth filing now rather than later
It is currently close to unreachable, because the databases open in rollback-journal mode and no
-walor-shmsidecars normally exist. #263 switches every handle to WAL, at which point those sidecars exist routinely and this path becomes genuinely reachable.So the severity of this issue changes the moment that PR lands. Filing it while the connection is visible rather than rediscovering it later.
Still narrow — it needs
os.Removeto fail on a sidecar after succeeding on the main file, which means a permissions or filesystem problem — and the consequence is a misleading message rather than additional data loss. Not milestoned.Definition of done
DeleteDBattempts every suffix rather than returning on the first failure, and reports what actually happened: which files were removed and which were not.deleteWebhookResourcessurfaces that distinction rather than flattening it.Note the interaction: once WAL lands, verify a normal delete removes all three files cleanly, since that is the common path and it currently has no coverage against sidecars existing at all.
Plan. Still true on
next, and now reachable: event databases run in WAL mode, so-waland-shmfiles exist routinely, andDeleteDBininternal/database/webhook_db_manager.gostill returns on the first file it fails to remove.DeleteDBtries all three files and returns an error that says which case happened: the database file itself is still there (deleting again may work), or it is gone and only a-walor-shmfile is left (the events are lost; the leftover can be removed by hand).Model: opus-5-5
Done in #451.
DeleteDBnow tries the event database file and both sidecars even when one cannot be removed. Its error says which case happened, and names each file left: the database file is still on disk, or the database file is gone (so the events are lost) and only a-walor-shmsidecar is left. The webhook delete handler logs a different message for each, and the second never says the events survive. The tests cover a normal delete with both sidecars present, and a failed removal of the database file or of a sidecar, made by putting a non-empty directory in the file's place.Judgement call: the page the operator sees is still the generic error page; the two cases differ in the logged line, which is where #273 put the record of what is left.
Judgement call: when the database file cannot be removed, the sidecars are still removed, so that message does not promise the events are intact.
Model: opus-5-5