Skip to content

Backups: stop double-opening the SQLite connection (silent write loss)#56

Merged
AdaInTheLab merged 1 commit into
mainfrom
fix/backup-double-open-silent-write-loss
May 9, 2026
Merged

Backups: stop double-opening the SQLite connection (silent write loss)#56
AdaInTheLab merged 1 commit into
mainfrom
fix/backup-double-open-silent-write-loss

Conversation

@AdaInTheLab
Copy link
Copy Markdown
Collaborator

Summary

Real bug, surfaced today during the first hands-on test of the backup feature.

What was wrong

`BackupService` had a redundant `conn.Open()` call inside every `using (var conn = _db.CreateConnection())` block — 6 occurrences across `CreateBackup`, `DeleteBackup`, `GetAll`, `GetBackup`, `LoadSettings`, `SaveSettings`. The factory already returns an opened connection, so this was opening twice.

The custom-built `System.Data.SQLite` we ship has a quirk on this code path:

  • No exception thrown on the second Open
  • Subsequent INSERT/UPDATE statements silently fail — the statement runs, returns affected-row counts, advances autoincrement counters, but nothing persists
  • Reads aren't affected — SELECTs work normally

Effect:

  • Click "Create Backup" → ZIP file appears on disk → controller returns `200 success` → no row in `backups` table → UI still says "No backups yet."
  • Toggle Auto Backup on, click Save → `SaveSettings` silently drops the write → no `backup_settings` row in `settings` → scheduler never starts on next boot
  • This made the entire feature look like it was wired but doing nothing

How it was caught

Patched `CreateBackup` with verbose logging on prod, clicked the button, log showed:
```
[BackupDebug] About to insert. Factory ConnString=Data Source=/...KitsuneCommand.db;Version=3;
[BackupDebug] conn type=System.Data.SQLite.SQLiteConnection state=Open db=main
[BackupDebug] INSERT rows=1 filename=backup_KitsuneDen_2026-05-09_06-43-35.zip
[BackupDebug] last_insert_rowid=2 total_rows_now=1
```

`last_insert_rowid=2` after a fresh table tells the story — earlier inserts had run far enough to advance the counter but never landed a row. Removing the second `.Open()` (which my diagnostic patch did as a side-effect of restructuring) made the INSERT persist.

Grep confirms this pattern is only in `BackupService` — all 19 other repositories use `_db.CreateConnection()` correctly.

Files

File What
`Services/BackupService.cs` Removed 6 redundant `conn.Open()` calls, added class-level doc comment explaining why

Test plan

  • Pull, build, deploy
  • Restart 7D2D
  • Click "Create Backup" → audit list shows the new row immediately
  • Toggle Auto Backup on, click Save → restart server → boot log shows `Backup scheduler started (every N min).`
  • Wait one interval → next ZIP appears on disk + new row in audit list

🤖 Generated with Claude Code

BackupService had a redundant conn.Open() call in every method that
used _db.CreateConnection(). The factory already returns an opened
connection, so this was double-opening on each call.

The custom-built System.Data.SQLite that ships with this mod
silently drops subsequent INSERT/UPDATE statements when Open is
invoked twice. No exception is thrown, the API returns 200, but no
row ends up in the table. Reads kept working — which is exactly why
this slipped past initial testing: GetAll never blew up, the panel
rendered "No backups yet", and the empty list looked normal because
the table really was empty.

Symptom that surfaced today: clicking Create Backup made a ZIP file
on disk but nothing in the backups table. The Auto Backup toggle in
the panel persisted to UI state but SaveSettings silently lost the
write, so no backup_settings row in the settings table, so the
scheduler never started either.

Confirmed via diagnostic logging on prod: removing the second .Open()
made the INSERT persist (rows=1, last_insert_rowid=2, total=1).
Verified the bug isn't elsewhere — grep across all repository code
shows BackupService is the only place that calls conn.Open() inside
the using block.

Added a class-level doc comment so future-me doesn't re-introduce it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@AdaInTheLab AdaInTheLab merged commit 8adf745 into main May 9, 2026
2 checks passed
@AdaInTheLab AdaInTheLab deleted the fix/backup-double-open-silent-write-loss branch May 9, 2026 10:49
AdaInTheLab added a commit that referenced this pull request May 13, 2026
Rolls up everything since v2.6.2 into a single release tag. v2.6.3
was bumped on main (#63) but never tagged or released — folding it
into v2.6.4 keeps the public release history clean.

Marquee additions since v2.6.2:

- Modpack (#64) — Bundle installed mods into a single zip players can
  download from the login page. No panel account required. Three-state
  workflow (draft → published → archived), top-right CTA on the login
  page, anonymous metadata + download endpoints. Atomic temp-file zip
  build so public downloaders can't grab a half-written archive.

- Graceful Restart (#57, #58) — Scheduled daily restarts with
  player-friendly in-game countdown warnings. Configurable warning
  ladder, IANA-timezone schedule (DST-aware), krestart console command,
  REST endpoints, panel Settings tab.

- Backups bug fix (#56) — BackupService had a redundant conn.Open()
  call that silently dropped writes due to a custom System.Data.SQLite
  quirk. Reads worked, so the bug went unnoticed until first hands-on
  test. Six call sites fixed, class doc comment added so future-me
  doesn't reintroduce it.

- German / French / Spanish locales (#59, #61, #62) — Full polite-form
  translations across ~250 keys / 30 namespaces. Browser auto-detect
  picks the right one for de-* / fr-* / es-* visitors. Tooltip on the
  language switcher noting that in-game broadcast messages don't
  auto-translate.

- Favicon (#60) — Regenerated all four favicon variants (svg / 16 /
  32 / ico) from kitsune-command-logo-transparent.png so the browser
  tab matches the panel's sidebar logo.

- README updates: features list now reflects all the above.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant