Run each migration and its bookkeeping row in one transaction #49

New issue

Open

opened 2026-10-23 10:22:39 +00:00 by james.oates · 0 comments

james.oates commented

2026-10-23 10:22:39 +00:00

Owner

The hazard

db.rs::run_if_pending executes a migration's statements and then, as a separate query, inserts its row into _migrations. There is no transaction around the pair.

If the process dies between the two, the schema change is applied but unrecorded. Every subsequent start replays the migration and fails — for a bare ALTER TABLE … ADD COLUMN that is duplicate column name: <col>, which needs manual SQL against the database to recover. ER will not boot in the meantime.

Not a regression

Migrations 003 through 011 are all bare ALTERs with exactly the same exposure. 013 only widens the window slightly by carrying three statements where the others carry one; its CREATE TABLE IF NOT EXISTS and DROP TABLE IF EXISTS are self-guarding, and the ALTER is not. Idempotency as actually invoked is fine — the run_if_pending gate means each runs once on a healthy path.

So this is a pre-existing robustness gap found while reviewing 013, not something that branch introduced, which is why it is filed rather than fixed in the release.

Fix

Wrap the statements and the _migrations insert in a single transaction. SQLite supports transactional DDL, including ALTER TABLE ADD COLUMN, so this works for every migration already written and needs no per-migration changes.

Worth doing at the same time: a migration that fails partway currently leaves whatever its earlier statements did. With a transaction the whole file rolls back, which makes a failed migration diagnosable instead of leaving a half-shaped schema.

Test to write first

Hard to test the crash window directly. The testable property is the one that matters: applying a migration whose later statement fails must leave the schema untouched and _migrations without its row, so the next start retries cleanly rather than finding a half-applied schema.

## The hazard `db.rs::run_if_pending` executes a migration's statements and then, as a **separate** query, inserts its row into `_migrations`. There is no transaction around the pair. If the process dies between the two, the schema change is applied but unrecorded. Every subsequent start replays the migration and fails — for a bare `ALTER TABLE … ADD COLUMN` that is `duplicate column name: <col>`, which needs manual SQL against the database to recover. ER will not boot in the meantime. ## Not a regression Migrations 003 through 011 are all bare `ALTER`s with exactly the same exposure. 013 only widens the window slightly by carrying three statements where the others carry one; its `CREATE TABLE IF NOT EXISTS` and `DROP TABLE IF EXISTS` are self-guarding, and the `ALTER` is not. Idempotency as actually invoked is fine — the `run_if_pending` gate means each runs once on a healthy path. So this is a pre-existing robustness gap found while reviewing 013, not something that branch introduced, which is why it is filed rather than fixed in the release. ## Fix Wrap the statements and the `_migrations` insert in a single transaction. SQLite supports transactional DDL, including `ALTER TABLE ADD COLUMN`, so this works for every migration already written and needs no per-migration changes. Worth doing at the same time: a migration that fails partway currently leaves whatever its earlier statements did. With a transaction the whole file rolls back, which makes a failed migration diagnosable instead of leaving a half-shaped schema. ## Test to write first Hard to test the crash window directly. The testable property is the one that matters: applying a migration whose later statement fails must leave the schema untouched and `_migrations` without its row, so the next start retries cleanly rather than finding a half-applied schema.

james.oates referenced this issue from a commit

2026-10-25 15:32:56 +00:00

fix: wait for the config file to settle before reloading it

james.oates referenced this issue

2026-10-26 17:53:55 +00:00

The WebSocket path has no connect timeout, no idle timeout and no connection cap #62