fix(startup): make migration failure fatal instead of warn-and-continue #219

Merged

nrupard merged 1 commit from fix/pms-286-fatal-migrations into main

2026-06-13 19:49:15 +02:00

nrupard commented

2026-06-13 19:41:26 +02:00

Owner

Summary

Makes a startup migration failure fatal instead of warn-and-continue. Closes PMS-286.

main.rs logged a failed migration at WARN and kept booting, serving a half-migrated database. This is how the staging outage happened: migration 040 (the PMS-263 co-mingled-rows verification) RAISED every boot, but because it was only a WARN the binary kept starting and never reached a healthy serving state - the container went unhealthy, Traefik's Docker provider dropped the mokosh-server router, and every /api/v1 request 404'd with no hint at the real cause. Later migrations (041, RLS coverage) silently never applied.

Change

if config.run_migrations {
    if let Err(e) = db.run_migrations().await {
        tracing::error!("Failed to run database migrations: {e}");
        return Err(e.into());
    }
    tracing::info!("Database migrations complete");
}

ERROR-level log + return from main (which already returns Result<_, Box<dyn Error>>) → process exits non-zero. Consistent with the rest of the startup path, which already hard-fails on SMTP / Google OAuth / ENCRYPTION_KEY / CORS_ORIGIN misconfiguration. RUN_MIGRATIONS=false still skips the migration step entirely; no new env var.

Verification

cargo fmt --all --check clean; cargo clippy --bin mokosh-server clean; compiles.

AC #4 - manual repro (no automated test)

main() is the binary entrypoint and not unit-testable, and a real migration failure cannot be forced under #[sqlx::test] (every migration is applied clean before the test body runs), so this is documented rather than asserted by a test:

Against a database with a default-tenant co-mingled business row (the exact staging condition), start the server with RUN_MIGRATIONS=true.
Migration 040 raises; the log shows ERROR ... Failed to run database migrations: ... and the process exits non-zero.
The server never logs Database migrations complete, never binds, and /api/v1/health is unreachable - vs. the old behaviour where it logged a single WARN, bound, and served unhealthy.

Clear the co-mingled rows (or set RUN_MIGRATIONS=false) and startup proceeds normally.

## Summary Makes a startup migration failure fatal instead of warn-and-continue. Closes PMS-286. `main.rs` logged a failed migration at WARN and kept booting, serving a half-migrated database. This is how the staging outage happened: migration 040 (the PMS-263 co-mingled-rows verification) RAISED every boot, but because it was only a WARN the binary kept starting and never reached a healthy serving state - the container went `unhealthy`, Traefik's Docker provider dropped the `mokosh-server` router, and every `/api/v1` request 404'd with no hint at the real cause. Later migrations (041, RLS coverage) silently never applied. ## Change ```rust if config.run_migrations { if let Err(e) = db.run_migrations().await { tracing::error!("Failed to run database migrations: {e}"); return Err(e.into()); } tracing::info!("Database migrations complete"); } ``` ERROR-level log + return from `main` (which already returns `Result<_, Box<dyn Error>>`) → process exits non-zero. Consistent with the rest of the startup path, which already hard-fails on SMTP / Google OAuth / `ENCRYPTION_KEY` / `CORS_ORIGIN` misconfiguration. `RUN_MIGRATIONS=false` still skips the migration step entirely; no new env var. ## Verification - `cargo fmt --all --check` clean; `cargo clippy --bin mokosh-server` clean; compiles. ## AC #4 - manual repro (no automated test) `main()` is the binary entrypoint and not unit-testable, and a real migration failure cannot be forced under `#[sqlx::test]` (every migration is applied clean before the test body runs), so this is documented rather than asserted by a test: 1. Against a database with a default-tenant co-mingled business row (the exact staging condition), start the server with `RUN_MIGRATIONS=true`. 2. Migration 040 raises; the log shows `ERROR ... Failed to run database migrations: ...` and the process exits non-zero. 3. The server never logs `Database migrations complete`, never binds, and `/api/v1/health` is unreachable - vs. the old behaviour where it logged a single WARN, bound, and served unhealthy. Clear the co-mingled rows (or set `RUN_MIGRATIONS=false`) and startup proceeds normally.

nrupard added 1 commit

2026-06-13 19:41:26 +02:00

fix(startup): make migration failure fatal instead of warn-and-continue

E2E / Playwright against staging (pull_request) Failing after 45s

Details

Check / fmt + clippy + compile + tests (pull_request) Failing after 2m51s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

a3333df85d

mokosh-server logged a startup migration failure at WARN and kept booting, serving a half-migrated database. That let a failed verification migration (040, the PMS-263 co-mingled-rows check) boot a server that never became healthy: the container went unhealthy, Traefik dropped its router, and every /api/v1 request 404'd with no hint at the real cause, while later migrations (041 RLS coverage) silently never applied.

Treat a migration failure as fatal when RUN_MIGRATIONS=true: log the error at ERROR level and return it from main so the process exits non-zero, so the orchestrator's restart/rollback policy acts on a real exit code instead of a perpetually-unhealthy container. This matches the rest of the startup path, which already hard-fails on SMTP/Google/ENCRYPTION_KEY/CORS misconfiguration. RUN_MIGRATIONS=false still skips the step entirely; no new env var.

#PMS-286