rubyforgood / rubyforgood/alongwithyou
destroyJournalDatabase() can report success when the journal file was not deleted
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 9
- Forks
- 4
- Avg merge
- 9h 34m
- Merged PRs (30d)
- 16
Description
Description
destroyJournalDatabase() in mobile/src/lib/db/database.ts is the mechanism behind the "Delete all my data" control that #116 tracks and ADR 0007 requires. It can report success while leaving the encrypted journal file on disk.
// database.ts:101-105
export async function destroyJournalDatabase(): Promise<void> {
await closeJournalDatabase();
await SQLite.deleteDatabaseAsync(DATABASE_NAME).catch(() => undefined);
await deleteDatabaseKey();
}
1. The blanket catch hides a real failure. expo-sqlite's iOS implementation throws two different errors from that call (node_modules/expo-sqlite/ios/SQLiteModule.swift, deleteDatabase(databasePath:)):
DatabaseNotFoundException— the file is already gone. Benign, and what the test atdatabase.test.ts:159intends to cover.DeleteDatabaseException— thrown when the database is still in the module's connection cache, i.e. it was not closed. Not benign.
Both are swallowed identically, so a genuinely failed deletion resolves as success and the UI tells the user their data is gone when it is not.
2. A failure to close skips key deletion entirely. closeJournalDatabase() awaits db?.closeAsync() unguarded (database.ts:89). If that rejects, destroyJournalDatabase rejects before either deletion runs, leaving the file and the key — the one outcome where the user keeps a fully readable journal after asking for it to be destroyed.
3. The stated ordering rationale does not hold. The comment at database.ts:96-99, repeated at database.test.ts:153, says file-before-key means an interruption "leaves an unreadable database rather than a readable one with no key". A database with no key is unreadable by definition, so that sentence describes a state which cannot exist. Deleting the key first is the stronger order: it guarantees unreadability at the earliest possible moment, and it survives the case where file deletion then fails.
Also worth handling while in here: deleteDatabaseAsync removes only the main file, not the -wal / -shm sidecars. A clean close checkpoints and removes those, so it only matters on the path where the close failed — which is exactly the path in (2).
Nothing calls destroyJournalDatabase yet, so this is latent. It is probably best done as part of #116, so the error handling is designed alongside the UI that has to report it.
Acceptance Criteria
- A failed file deletion is surfaced to the caller rather than swallowed; only "already absent" counts as success
- A failure to close the handle does not prevent the key from being deleted
- Key deletion happens before file deletion, or the comment is rewritten to describe the guarantee that actually holds
- The comments in
database.tsand the assertion indatabase.test.tsagree with whichever order is chosen - Tests cover: close fails, file deletion fails, file already absent
- The feature/s being implemented are covered by unit tests - If not, create tests for them on this ticket
Additional Info and Resources
- Belongs with #116 ("Delete all my data" in-app control) — the promise made to the user is what sets the bar here
- Found in review of #128; latent, not a regression
- Whatever the caller does with a failure, it must not be a silent success — that is the only outcome this issue rules out
QA
- With
deleteDatabaseAsyncmocked to reject, confirm the caller can distinguish failure from success - With
closeAsyncmocked to reject, confirm the key is still deleted -
cd mobile && npm test— green
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with destroyJournalDatabase() and closeJournalDatabase() in mobile/src/lib/db/database.ts, then read the related assertions around database.test.ts:153-159. Run the mobile test suite and inspect the existing mocks for closeAsync, deleteDatabaseAsync, and deleteDatabaseKey. Done means close, missing-file, and deletion-failure paths are distinguishable, key cleanup is covered, and the ordering comments and tests agree.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- sqlite, typescript
- Domain
- database, mobile, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100