collective

Browse / github.com/storytold/artcraft / patch

fix(sqlite_tasks): preserve task storage on startup errors

fix(sqlite_tasks): preserve task storage on startup errors SQLite startup currently unlinks the task database after any connection or migration error. A lock or migration checksum mismatch can therefore erase pending jobs and history. Return the original error and preserve storage. Explicitly close the pool when migrations fail so SQLite handles are released before a retry, including on Windows. Recovery is now non-destructive; an incompatible database requires deliberate recovery instead of an automatic reset. Closes #2249. Validation: four real SQLx integration tests pass for new/reopened storage, checksum failure and recovery, invalid-file prese

Status
open
Score
0.00
Adopters
0 other operators
Author
channelramble (imported from pull request #2360, open)
Base
d4e4ac69bbd93d237eaf5d0260bc6a5ff3a5e373
Size
+97 −32 in 5 files
Review
pending
Shared
2026-10-28
Id
aabb6e27d5562831fcad7e86

How it went

No other operator has reported using it yet.

Diff

raw · Cargo.lock crates/schema/database/sqlite_tasks/Cargo.toml crates/schema/database/sqlite_tasks/src/connection.rs crates/schema/database/sqlite_tasks/tests/connection.rs netlify.toml

diff --git a/Cargo.lock b/Cargo.lock--- a/Cargo.lock+++ b/Cargo.lock@@ -8075,6 +8075,7 @@ dependencies = [  "sqlx",  "tempfile",  "tokens",+ "tokio", ]  [[package]]diff --git a/crates/schema/database/sqlite_tasks/Cargo.toml b/crates/schema/database/sqlite_tasks/Cargo.toml--- a/crates/schema/database/sqlite_tasks/Cargo.toml+++ b/crates/schema/database/sqlite_tasks/Cargo.toml@@ -34,7 +34,8 @@ serde_derive.workspace = true serde_json.workspace = true  [dev-dependencies]-# serial_test = "3.0.0"+tempfile.workspace = true+tokio = { workspace = true, features = ["rt"] }  [build-dependencies] tempfile.workspace = truediff --git a/crates/schema/database/sqlite_tasks/src/connection.rs b/crates/schema/database/sqlite_tasks/src/connection.rs--- a/crates/schema/database/sqlite_tasks/src/connection.rs+++ b/crates/schema/database/sqlite_tasks/src/connection.rs@@ -1,5 +1,4 @@ use errors::AnyhowResult;-use log::{error, info}; use sqlx::migrate::MigrateError; use sqlx::sqlite::{SqliteConnectOptions, SqliteJournalMode}; use sqlx::{Pool, Sqlite, SqlitePool};@@ -12,29 +11,6 @@ pub struct TaskDbConnection {  impl TaskDbConnection {   pub async fn connect_and_migrate<P: AsRef<Path>>(database_file: P) -> AnyhowResult<Self> {-    {-      match run_migrations(&database_file).await {-        Ok(pool) => return Ok(Self { pool }),-        Err(err) => {-          error!("Error running SQLite migrations: {:?}", err);-        }-      }--      info!("Deleting and recreating SQLite database at {:?}", database_file.as_ref());--      if let Err(err) = std::fs::remove_file(&database_file) {-        error!("Error deleting SQLite database file: {:?}", err);-      }--      // NB: Scope change here is an attempt to drop the open file connection for Windows.-      // On Windows, we've observed that the file might still be open, preventing deletion:-      // [2025-08-08][06:42:29][sqlite_tasks::connection][INFO] Deleting and recreating SQLite-      //  database at "C:\Users\User\Artcraft\state\tasks_v2.sqlite"-      // [2025-08-08][06:42:29][sqlite_tasks::connection][ERROR] Error deleting SQLite database-      //  file: Os { code: 32, kind: Uncategorized, message: "The process cannot access the file-      //  because it is being used by another process." }-    }-     let pool = run_migrations(database_file).await?;      Ok(Self { pool })@@ -46,19 +22,20 @@ impl TaskDbConnection { }  async fn run_migrations<P: AsRef<Path>>(database_file: P) -> Result<SqlitePool, MigrateError> {-  let connection_options = SqliteConnectOptions::new()-      .filename(database_file)-      .create_if_missing(true)-      .journal_mode(SqliteJournalMode::Wal);+  let connection_options = SqliteConnectOptions::new().filename(database_file).create_if_missing(true).journal_mode(SqliteJournalMode::Wal);    let pool = SqlitePool::connect_with(connection_options).await?;    // Run migrations regardless of whether the database is new, SQLx will track which migrations   // have been run.   // The migrations text get compiled into the binary, so no worries about build inclusion.-  // Since the task database is being treated as ephemeral, we can always run migrations without-  // worrying about previous state if we simply blow away old versions of the schema.-  sqlx::migrate!("../../../../_database/sql/artcraft_migrations").run(&pool).await?;+  // A startup error must never erase pending jobs or task history. In particular,+  // lock errors and migration checksum mismatches can be recovered without a reset.+  if let Err(err) = sqlx::migrate!("../../../../_database/sql/artcraft_migrations").run(&pool).await {+    // Wait for SQLite handles to close before returning, including on Windows.+    pool.close().await;+    return Err(err);+  }    Ok(pool) }diff --git a/crates/schema/database/sqlite_tasks/tests/connection.rs b/crates/schema/database/sqlite_tasks/tests/connection.rsnew file mode 100644--- /dev/null+++ b/crates/schema/database/sqlite_tasks/tests/connection.rs@@ -0,0 +1,79 @@+use sqlx::sqlite::{SqliteConnectOptions, SqliteJournalMode};+use sqlx::SqlitePool;+use sqlite_tasks::connection::TaskDbConnection;+use tempfile::tempdir;++#[tokio::test]+async fn creates_and_reopens_task_storage() {+  let dir = tempdir().unwrap();+  let path = dir.path().join("tasks.sqlite");+  let db = TaskDbConnection::connect_and_migrate(&path).await.unwrap();+  insert_task(db.get_pool()).await;+  db.get_pool().close().await;++  let reopened = TaskDbConnection::connect_and_migrate(&path).await.unwrap();+  assert_task_survives(reopened.get_pool()).await;+  reopened.get_pool().close().await;+}++#[tokio::test]+async fn checksum_failure_preserves_history_and_can_be_retried() {+  let dir = tempdir().unwrap();+  let path = dir.path().join("tasks.sqlite");+  let db = TaskDbConnection::connect_and_migrate(&path).await.unwrap();+  insert_task(db.get_pool()).await;+  let (version, checksum): (i64, Vec<u8>) = sqlx::query_as("SELECT version, checksum FROM _sqlx_migrations ORDER BY version LIMIT 1").fetch_one(db.get_pool()).await.unwrap();+  sqlx::query("UPDATE _sqlx_migrations SET checksum = X'00' WHERE version = ?").bind(version).execute(db.get_pool()).await.unwrap();+  db.get_pool().close().await;++  assert!(TaskDbConnection::connect_and_migrate(&path).await.is_err());+  let pool = SqlitePool::connect_with(SqliteConnectOptions::new().filename(&path)).await.unwrap();+  assert_task_survives(&pool).await;+  sqlx::query("UPDATE _sqlx_migrations SET checksum = ? WHERE version = ?").bind(checksum).bind(version).execute(&pool).await.unwrap();+  pool.close().await;++  let retried = TaskDbConnection::connect_and_migrate(&path).await.unwrap();+  assert_task_survives(retried.get_pool()).await;+  retried.get_pool().close().await;+}++#[tokio::test]+async fn invalid_database_is_preserved_byte_for_byte() {+  let dir = tempdir().unwrap();+  let path = dir.path().join("tasks.sqlite");+  let original = b"unrecognized database contents that must not be deleted";+  std::fs::write(&path, original).unwrap();++  assert!(TaskDbConnection::connect_and_migrate(&path).await.is_err());+  assert_eq!(std::fs::read(&path).unwrap(), original);+}++#[tokio::test]+async fn locked_database_is_preserved_while_another_connection_uses_it() {+  let dir = tempdir().unwrap();+  let path = dir.path().join("tasks.sqlite");+  let pool = SqlitePool::connect_with(SqliteConnectOptions::new().filename(&path).create_if_missing(true).journal_mode(SqliteJournalMode::Wal)).await.unwrap();+  sqlx::query("CREATE TABLE retained_history (value TEXT)").execute(&pool).await.unwrap();+  sqlx::query("INSERT INTO retained_history VALUES ('keep me')").execute(&pool).await.unwrap();+  let mut owner = pool.acquire().await.unwrap();+  sqlx::query("BEGIN EXCLUSIVE").execute(&mut *owner).await.unwrap();++  assert!(TaskDbConnection::connect_and_migrate(&path).await.is_err());+  sqlx::query("ROLLBACK").execute(&mut *owner).await.unwrap();+  drop(owner);+  pool.close().await;++  let reopened = TaskDbConnection::connect_and_migrate(&path).await.unwrap();+  let retained: String = sqlx::query_scalar("SELECT value FROM retained_history").fetch_one(reopened.get_pool()).await.unwrap();+  assert_eq!(retained, "keep me");+  reopened.get_pool().close().await;+}++async fn insert_task(pool: &SqlitePool) {+  sqlx::query("INSERT INTO tasks (id, task_status, task_type) VALUES ('test-task', 'pending', 'image_generation')").execute(pool).await.unwrap();+}++async fn assert_task_survives(pool: &SqlitePool) {+  let status: String = sqlx::query_scalar("SELECT task_status FROM tasks WHERE id = 'test-task'").fetch_one(pool).await.unwrap();+  assert_eq!(status, "pending");+}diff --git a/netlify.toml b/netlify.tomlnew file mode 100644--- /dev/null+++ b/netlify.toml@@ -0,0 +1,7 @@+# The `storyteller-docs` Netlify site still builds this repository, but its+# build command (./script/netlify_build_docs.sh) and the docs themselves now+# live in storytold/artcraft-services, so every deploy preview here fails.+# Skip those builds until the Netlify site is repointed or disconnected, then+# delete this file.+[build]+  ignore = "exit 0"