From 52565ea8b8e64d033c26bca54c6f82f3e87e0c1a Mon Sep 17 00:00:00 2001 From: Jake Date: Tue, 12 May 2026 22:57:17 +0100 Subject: [PATCH] =?UTF-8?q?agent:=20area=20A=20commit=201=20=E2=80=94=20ty?= =?UTF-8?q?ped=20magnotia=5Fstorage::Error=20+=20boundary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 1 of 2 for engine-slop residuals Area A. Adds a typed `magnotia_storage::Error` enum and rewires every error construction site in the storage crate to use it. The crate boundary into `magnotia_core::error::MagnotiaError` is handled by a `From for MagnotiaError` impl that lives inside the storage crate (not in core) to avoid a `core -> storage` dependency cycle. The old `MagnotiaError::StorageError(String)` variant is kept in this commit as an unused placeholder; commit 2 deletes it. New types in crates/storage/src/error.rs: - pub enum Error { DatabaseOpen, Migration, Query, NotFound, InvalidReference, Filesystem } with #[derive(thiserror::Error)] - pub enum OpenOp { Connect, ReadOnlyConnect, ForeignKeysPragma } - pub enum MigrationStep { SchemaVersionTableCreate, SchemaVersionQuery, TxBegin, Apply, RecordVersion, Commit } - pub enum Entity { Transcript, Task, Profile, ImplementationRule, Feedback } — seeded only with entities that have a real NotFound/InvalidReference case in the codebase - pub type Result = std::result::Result - impl Error { pub fn kind() -> StorageKind, pub fn operation_label() -> Cow<'static, str> } - impl From for MagnotiaError New types in crates/core/src/error.rs: - MagnotiaError::Storage { kind: StorageKind, operation: String, detail: String } with #[error("{detail}")] so the boundary doesn't double-prefix Display output - pub enum StorageKind { DatabaseOpen, Migration, Query, NotFound, InvalidReference, Filesystem } #[derive(Serialize)] #[serde(rename_all = "snake_case")] for future Area E Storage crate dependency added: thiserror = "1" Migration scope: - migrations.rs: 6 sites → Error::Migration with structured step + version - database.rs: 72 sites broken down as: * 3 → Error::DatabaseOpen (init / readonly / pragma) * 1 → Error::Filesystem (init's create_dir_all with path context) * ~58 → Error::Query with operation labels matching the prior message stem in snake_case; transaction sub-steps use dotted labels ("complete_subtask_and_check_parent.commit_transaction" etc.) so transaction internals are not collapsed * 5 → Error::NotFound (transcript / task×2 / implementation_rule×2) * 5 → Error::InvalidReference (insert_transcript unknown profile; Default profile rename/delete invariants ×3; feedback rating value validation) Survey ambiguities resolved per the locked answers: - Q1 (schema_version_query): Migration step, not Query - Q2 (transaction sub-steps): preserved granularity with dotted operation labels; no collapsing - Q3 (Entity cardinality): seeded with the 3 from the survey + 2 more discovered during migration (ImplementationRule, Feedback), per "add when an actual case needs it" - Q4 (operation label type): Cow<'static, str> - Q5 (Serialize): storage::Error is not serializable; flattens only at the MagnotiaError boundary - Q6 (re-exports): pub use error::{Entity, Error, MigrationStep, OpenOp, Result} in storage::lib.rs; StorageKind belongs to magnotia_core Two scope-discovered additions beyond the original 5 variants: - Error::Filesystem { path, source } variant + matching StorageKind — required because init() now returns storage::Result and the create_dir_all call needs a typed path; doing this via From for storage::Error would have lost the path so it's explicit - Entity::ImplementationRule and Entity::Feedback — two NotFound / InvalidReference sites the original survey missed in the rule-CRUD and feedback-validation areas Verification: - cargo fmt --all - cargo check --workspace --all-targets — clean - cargo test -p magnotia-storage — 60 passed, 0 failed - cargo test --workspace — all green, ~330 tests - rg 'StorageError\(' crates/ src-tauri/src/ — only hit is the variant declaration in core that commit 2 will delete - rg 'Other\(String' crates/storage/src/ — zero - rg 'format!\("[A-Z].* failed' crates/storage/src/ — zero Commit 2 will delete MagnotiaError::StorageError(String) once this is in. Pre-existing working-tree churn in crates/llm/, src/lib/pages/, src/lib/utils/saveMarkdown.ts and the untracked phase10a dogfood notes deliberately left unstaged. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 1 + crates/core/src/error.rs | 25 ++ crates/storage/Cargo.toml | 3 + crates/storage/src/database.rs | 423 ++++++++++++++++++++++--------- crates/storage/src/error.rs | 194 ++++++++++++++ crates/storage/src/lib.rs | 3 + crates/storage/src/migrations.rs | 38 ++- 7 files changed, 550 insertions(+), 137 deletions(-) create mode 100644 crates/storage/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index a85223c..62c007a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2924,6 +2924,7 @@ dependencies = [ "magnotia-core", "serde", "sqlx", + "thiserror 1.0.69", "tokio", "uuid", ] diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index 91696ce..b8c32f9 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -37,6 +37,16 @@ pub enum MagnotiaError { #[error("storage error: {0}")] StorageError(String), + /// Structured storage failure flowed up from `magnotia_storage::Error` via + /// its `From` impl. Display reads through to `detail` so the operation + + /// source context produced by the storage crate isn't double-prefixed. + #[error("{detail}")] + Storage { + kind: StorageKind, + operation: String, + detail: String, + }, + #[error("provider not registered: {0}")] ProviderNotRegistered(String), @@ -48,6 +58,21 @@ pub enum MagnotiaError { }, } +/// Coarse discriminator for `MagnotiaError::Storage`. The storage crate maps +/// its full typed error onto one of these kinds at the boundary. Backend code +/// that wants finer-grained branching pattern-matches on +/// `magnotia_storage::Error` directly. +#[derive(Debug, Clone, Copy, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum StorageKind { + DatabaseOpen, + Migration, + Query, + NotFound, + InvalidReference, + Filesystem, +} + impl From for MagnotiaError { fn from(err: std::io::Error) -> Self { Self::Io { diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index 30be52d..990b730 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -24,5 +24,8 @@ serde = { version = "1", features = ["derive"] } # Logging log = "0.4" +# Structured error derivation for magnotia_storage::Error. +thiserror = "1" + # UUIDs for profile + profile_terms ids (v7 random). uuid = { version = "1", features = ["v4"] } diff --git a/crates/storage/src/database.rs b/crates/storage/src/database.rs index 3858965..7414744 100644 --- a/crates/storage/src/database.rs +++ b/crates/storage/src/database.rs @@ -3,12 +3,15 @@ use std::path::Path; use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; use sqlx::{Row, SqlitePool}; -use magnotia_core::error::{MagnotiaError, Result}; +use crate::error::{Entity, Error, OpenOp, Result}; /// Initialise the SQLite database with connection pool and run migrations. pub async fn init(db_path: &Path) -> Result { if let Some(parent) = db_path.parent() { - std::fs::create_dir_all(parent)?; + std::fs::create_dir_all(parent).map_err(|source| Error::Filesystem { + path: parent.to_path_buf(), + source, + })?; } let options = SqliteConnectOptions::new() @@ -19,12 +22,18 @@ pub async fn init(db_path: &Path) -> Result { .max_connections(5) .connect_with(options) .await - .map_err(|e| MagnotiaError::StorageError(format!("Database connect failed: {e}")))?; + .map_err(|source| Error::DatabaseOpen { + operation: OpenOp::Connect, + source, + })?; sqlx::query("PRAGMA foreign_keys = ON") .execute(&pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("foreign_keys pragma failed: {e}")))?; + .map_err(|source| Error::DatabaseOpen { + operation: OpenOp::ForeignKeysPragma, + source, + })?; run_migrations(&pool).await?; @@ -47,7 +56,10 @@ pub async fn init_readonly(db_path: &Path) -> Result { .max_connections(2) .connect_with(options) .await - .map_err(|e| MagnotiaError::StorageError(format!("Read-only connect failed: {e}"))) + .map_err(|source| Error::DatabaseOpen { + operation: OpenOp::ReadOnlyConnect, + source, + }) } /// Run schema migrations via the versioned migration system. @@ -82,10 +94,10 @@ pub async fn insert_transcript( params: &InsertTranscriptParams<'_>, ) -> Result<()> { if !profile_exists(pool, params.profile_id).await? { - return Err(MagnotiaError::StorageError(format!( - "Insert transcript failed: unknown profile id '{}'", - params.profile_id - ))); + return Err(Error::InvalidReference { + entity: Entity::Profile, + reason: format!("unknown profile id '{}'", params.profile_id).into(), + }); } sqlx::query( @@ -110,7 +122,10 @@ pub async fn insert_transcript( .bind(params.anti_hallucination) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Insert transcript failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "insert_transcript".into(), + source, + })?; Ok(()) } @@ -121,7 +136,10 @@ pub async fn get_transcript(pool: &SqlitePool, id: &str) -> Result Result { let n: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM transcripts") .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Count transcripts failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "count_transcripts".into(), + source, + })?; Ok(n) } @@ -182,8 +206,9 @@ pub async fn update_transcript( .bind(id) .execute(pool) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Update transcript failed: {e}")) + .map_err(|source| Error::Query { + operation: "update_transcript".into(), + source, })?; Ok(res.rows_affected()) } @@ -193,8 +218,9 @@ pub async fn update_transcript( .bind(id) .execute(pool) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Update transcript failed: {e}")) + .map_err(|source| Error::Query { + operation: "update_transcript".into(), + source, })?; Ok(res.rows_affected()) } @@ -204,8 +230,9 @@ pub async fn update_transcript( .bind(id) .execute(pool) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Update transcript failed: {e}")) + .map_err(|source| Error::Query { + operation: "update_transcript".into(), + source, })?; Ok(res.rows_affected()) } @@ -253,11 +280,17 @@ pub async fn update_transcript_meta( .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("update_transcript_meta: {e}")))?; + .map_err(|source| Error::Query { + operation: "update_transcript_meta".into(), + source, + })?; - get_transcript(pool, id).await?.ok_or_else(|| { - MagnotiaError::StorageError(format!("update_transcript_meta: transcript {id} not found")) - }) + get_transcript(pool, id) + .await? + .ok_or_else(|| Error::NotFound { + entity: Entity::Transcript, + key: id.to_string(), + }) } pub async fn delete_transcript(pool: &SqlitePool, id: &str) -> Result<()> { @@ -265,7 +298,10 @@ pub async fn delete_transcript(pool: &SqlitePool, id: &str) -> Result<()> { .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Delete transcript failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "delete_transcript".into(), + source, + })?; Ok(()) } @@ -289,7 +325,10 @@ pub async fn search_transcripts( .bind(limit) .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("FTS search failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "search_transcripts".into(), + source, + })?; Ok(rows.iter().map(transcript_row_from).collect()) } @@ -327,7 +366,10 @@ pub async fn insert_task( .bind(energy) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Insert task failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "insert_task".into(), + source, + })?; Ok(()) } @@ -339,7 +381,10 @@ pub async fn list_tasks(pool: &SqlitePool) -> Result> { ) .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("List tasks failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_tasks".into(), + source, + })?; Ok(rows.into_iter().map(task_row_from).collect()) } @@ -352,7 +397,10 @@ pub async fn get_task_by_id(pool: &SqlitePool, id: &str) -> Result) .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("set_task_energy failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "set_task_energy".into(), + source, + })?; - get_task_by_id(pool, id).await?.ok_or_else(|| { - MagnotiaError::StorageError(format!("set_task_energy: task {id} not found after update")) - }) + get_task_by_id(pool, id) + .await? + .ok_or_else(|| Error::NotFound { + entity: Entity::Task, + key: id.to_string(), + }) } pub async fn insert_subtask( @@ -430,7 +490,10 @@ pub async fn insert_subtask( .bind(parent_task_id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Insert subtask failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "insert_subtask".into(), + source, + })?; Ok(()) } @@ -443,7 +506,10 @@ pub async fn list_subtasks(pool: &SqlitePool, parent_id: &str) -> Result Result Result<()> { - let mut tx = pool - .begin() - .await - .map_err(|e| MagnotiaError::StorageError(format!("Begin transaction failed: {e}")))?; + let mut tx = pool.begin().await.map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.begin_transaction".into(), + source, + })?; sqlx::query("UPDATE tasks SET done = 1, done_at = datetime('now') WHERE id = ?") .bind(subtask_id) .execute(&mut *tx) .await - .map_err(|e| MagnotiaError::StorageError(format!("Complete subtask failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.complete_subtask".into(), + source, + })?; let parent_id: Option = sqlx::query_scalar("SELECT parent_task_id FROM tasks WHERE id = ?") .bind(subtask_id) .fetch_one(&mut *tx) .await - .map_err(|e| MagnotiaError::StorageError(format!("Get parent_task_id failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.get_parent_task_id".into(), + source, + })?; if let Some(pid) = parent_id { let pending: i64 = @@ -475,8 +547,9 @@ pub async fn complete_subtask_and_check_parent(pool: &SqlitePool, subtask_id: &s .bind(&pid) .fetch_one(&mut *tx) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Count pending subtasks failed: {e}")) + .map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.count_pending_subtasks".into(), + source, })?; if pending == 0 { @@ -490,15 +563,17 @@ pub async fn complete_subtask_and_check_parent(pool: &SqlitePool, subtask_id: &s .bind(&pid) .execute(&mut *tx) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Auto-complete parent failed: {e}")) + .map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.auto_complete_parent".into(), + source, })?; } } - tx.commit() - .await - .map_err(|e| MagnotiaError::StorageError(format!("Commit transaction failed: {e}")))?; + tx.commit().await.map_err(|source| Error::Query { + operation: "complete_subtask_and_check_parent.commit_transaction".into(), + source, + })?; Ok(()) } @@ -508,21 +583,27 @@ pub async fn complete_task(pool: &SqlitePool, id: &str) -> Result<()> { .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Complete task failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "complete_task".into(), + source, + })?; Ok(()) } pub async fn uncomplete_task(pool: &SqlitePool, id: &str) -> Result<()> { - let mut tx = pool - .begin() - .await - .map_err(|e| MagnotiaError::StorageError(format!("Begin transaction failed: {e}")))?; + let mut tx = pool.begin().await.map_err(|source| Error::Query { + operation: "uncomplete_task.begin_transaction".into(), + source, + })?; sqlx::query("UPDATE tasks SET done = 0, done_at = NULL, auto_completed = 0 WHERE id = ?") .bind(id) .execute(&mut *tx) .await - .map_err(|e| MagnotiaError::StorageError(format!("Uncomplete task failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "uncomplete_task.uncomplete_row".into(), + source, + })?; // Mirror the auto-complete invariant from // `complete_subtask_and_check_parent`: a parent task is done iff @@ -534,7 +615,10 @@ pub async fn uncomplete_task(pool: &SqlitePool, id: &str) -> Result<()> { .bind(id) .fetch_optional(&mut *tx) .await - .map_err(|e| MagnotiaError::StorageError(format!("Get parent_task_id failed: {e}")))? + .map_err(|source| Error::Query { + operation: "uncomplete_task.get_parent_task_id".into(), + source, + })? .flatten(); if let Some(pid) = parent_id { @@ -545,12 +629,16 @@ pub async fn uncomplete_task(pool: &SqlitePool, id: &str) -> Result<()> { .bind(&pid) .execute(&mut *tx) .await - .map_err(|e| MagnotiaError::StorageError(format!("Reopen parent failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "uncomplete_task.reopen_parent".into(), + source, + })?; } - tx.commit() - .await - .map_err(|e| MagnotiaError::StorageError(format!("Commit transaction failed: {e}")))?; + tx.commit().await.map_err(|source| Error::Query { + operation: "uncomplete_task.commit_transaction".into(), + source, + })?; Ok(()) } @@ -560,7 +648,10 @@ pub async fn delete_task(pool: &SqlitePool, id: &str) -> Result<()> { .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Delete task failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "delete_task".into(), + source, + })?; Ok(()) } @@ -607,7 +698,10 @@ pub async fn list_recent_completions( .bind(format!("-{} days", days - 1)) .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("List recent completions failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_recent_completions.fetch_grouped".into(), + source, + })?; let lookup: std::collections::HashMap = rows .into_iter() @@ -618,7 +712,10 @@ pub async fn list_recent_completions( let today_row: (String,) = sqlx::query_as("SELECT DATE('now', 'localtime')") .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Get local today failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_recent_completions.get_local_today".into(), + source, + })?; let today = today_row.0; let mut series = Vec::with_capacity(days as usize); @@ -630,7 +727,10 @@ pub async fn list_recent_completions( .bind(format!("-{offset} days")) .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Compute spine day failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_recent_completions.compute_spine_day".into(), + source, + })?; let count = lookup.get(&day).copied().unwrap_or(0); series.push(DailyCompletionCount { day, count }); } @@ -663,13 +763,17 @@ pub async fn insert_implementation_rule( .bind(last_fired_key) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Insert implementation rule failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "insert_implementation_rule".into(), + source, + })?; - get_implementation_rule(pool, id).await?.ok_or_else(|| { - MagnotiaError::StorageError(format!( - "insert_implementation_rule: rule {id} not found after insert" - )) - }) + get_implementation_rule(pool, id) + .await? + .ok_or_else(|| Error::NotFound { + entity: Entity::ImplementationRule, + key: id.to_string(), + }) } pub async fn list_implementation_rules(pool: &SqlitePool) -> Result> { @@ -680,7 +784,10 @@ pub async fn list_implementation_rules(pool: &SqlitePool) -> Result Result<()> { @@ -757,8 +871,9 @@ pub async fn delete_implementation_rule(pool: &SqlitePool, id: &str) -> Result<( .bind(id) .execute(pool) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Delete implementation rule failed: {e}")) + .map_err(|source| Error::Query { + operation: "delete_implementation_rule".into(), + source, })?; Ok(()) } @@ -771,7 +886,10 @@ pub async fn set_setting(pool: &SqlitePool, key: &str, value: &str) -> Result<() .bind(value) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Set setting failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "set_setting".into(), + source, + })?; Ok(()) } @@ -780,7 +898,10 @@ pub async fn get_setting(pool: &SqlitePool, key: &str) -> Result> .bind(key) .fetch_optional(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Get setting failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "get_setting".into(), + source, + })?; Ok(row.map(|r| r.get("value"))) } @@ -958,15 +1079,19 @@ fn implementation_rule_row_from(r: sqlx::sqlite::SqliteRow) -> ImplementationRul // 1. The DB triggers `trg_protect_default_profile_delete` / // `trg_protect_default_profile_rename` from migration v6. // 2. Rust-layer fail-fast checks below that short-circuit BEFORE the -// query hits sqlite, so UI callers get a friendly MagnotiaError::StorageError -// instead of an opaque `SQLITE_CONSTRAINT_TRIGGER` wrapped in text. +// query hits sqlite, so UI callers get a friendly typed +// `Error::InvalidReference` instead of an opaque +// `SQLITE_CONSTRAINT_TRIGGER` wrapped in text. pub async fn list_profiles(pool: &SqlitePool) -> Result> { let rows = sqlx::query("SELECT id, name, initial_prompt, created_at FROM profiles ORDER BY name ASC") .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("List profiles failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_profiles".into(), + source, + })?; Ok(rows.iter().map(profile_row_from).collect()) } @@ -975,7 +1100,10 @@ pub async fn get_profile(pool: &SqlitePool, id: &str) -> Result Result<()> { if id == crate::DEFAULT_PROFILE_ID && name != "Default" { - return Err(MagnotiaError::StorageError( - "Default profile cannot be renamed".into(), - )); + return Err(Error::InvalidReference { + entity: Entity::Profile, + reason: "Default profile cannot be renamed".into(), + }); } if id != crate::DEFAULT_PROFILE_ID && name == "Default" { - return Err(MagnotiaError::StorageError( - "Cannot rename another profile to 'Default'".into(), - )); + return Err(Error::InvalidReference { + entity: Entity::Profile, + reason: "cannot rename another profile to 'Default'".into(), + }); } sqlx::query("UPDATE profiles SET name = ?, initial_prompt = ? WHERE id = ?") .bind(name) @@ -1028,7 +1161,10 @@ pub async fn update_profile( .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Update profile failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "update_profile".into(), + source, + })?; Ok(()) } @@ -1037,21 +1173,28 @@ pub async fn update_profile( /// ON DELETE CASCADE on `profile_terms.profile_id` cleans up children. pub async fn delete_profile(pool: &SqlitePool, id: &str) -> Result<()> { if id == crate::DEFAULT_PROFILE_ID { - return Err(MagnotiaError::StorageError( - "Default profile cannot be deleted".into(), - )); + return Err(Error::InvalidReference { + entity: Entity::Profile, + reason: "Default profile cannot be deleted".into(), + }); } let transcript_count = transcript_count_for_profile(pool, id).await?; if transcript_count > 0 { - return Err(MagnotiaError::StorageError(format!( - "Cannot delete profile while {transcript_count} transcript(s) still reference it; reassign transcripts first" - ))); + return Err(Error::InvalidReference { + entity: Entity::Profile, + reason: format!( + "cannot delete profile while {transcript_count} transcript(s) still reference it; reassign transcripts first" + ).into(), + }); } sqlx::query("DELETE FROM profiles WHERE id = ?") .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Delete profile failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "delete_profile".into(), + source, + })?; Ok(()) } @@ -1066,7 +1209,10 @@ pub async fn list_profile_terms( .bind(profile_id) .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("List profile terms failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_profile_terms".into(), + source, + })?; Ok(rows.iter().map(profile_term_row_from).collect()) } @@ -1092,7 +1238,10 @@ pub async fn add_profile_term( .bind(note) .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Add profile term failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "add_profile_term".into(), + source, + })?; Ok(profile_term_row_from(&row)) } @@ -1101,7 +1250,10 @@ pub async fn delete_profile_term(pool: &SqlitePool, id: &str) -> Result<()> { .bind(id) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Delete profile term failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "delete_profile_term".into(), + source, + })?; Ok(()) } @@ -1110,7 +1262,10 @@ async fn profile_exists(pool: &SqlitePool, id: &str) -> Result { .bind(id) .fetch_optional(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Profile existence check failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "profile_exists".into(), + source, + })?; Ok(exists.is_some()) } @@ -1119,7 +1274,10 @@ async fn transcript_count_for_profile(pool: &SqlitePool, id: &str) -> Result Result { .bind(format!("-{keep_days} days")) .execute(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Prune error_log failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "prune_error_log".into(), + source, + })?; Ok(result.rows_affected()) } @@ -1197,7 +1361,10 @@ pub async fn list_recent_errors(pool: &SqlitePool, limit: i64) -> Result Result { if !matches!(params.rating, -1..=1) { - return Err(MagnotiaError::StorageError(format!( - "invalid feedback rating {} (must be -1, 0, or 1)", - params.rating - ))); + return Err(Error::InvalidReference { + entity: Entity::Feedback, + reason: format!("invalid rating {} (must be -1, 0, or 1)", params.rating).into(), + }); } let profile_id = params .profile_id @@ -1302,7 +1469,10 @@ pub async fn record_feedback(pool: &SqlitePool, params: RecordFeedbackParams) -> .bind(profile_id) .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("record_feedback failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "record_feedback".into(), + source, + })?; Ok(row.get::("id")) } @@ -1339,7 +1509,10 @@ pub async fn list_feedback_examples( .bind(limit) .fetch_all(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("list_feedback_examples failed: {e}")))?; + .map_err(|source| Error::Query { + operation: "list_feedback_examples".into(), + source, + })?; Ok(rows .into_iter() diff --git a/crates/storage/src/error.rs b/crates/storage/src/error.rs new file mode 100644 index 0000000..f566ee6 --- /dev/null +++ b/crates/storage/src/error.rs @@ -0,0 +1,194 @@ +//! Storage-local typed error. +//! +//! Backend code that wants to branch on storage failure modes pattern-matches +//! on [`Error`] directly. The crate boundary into [`magnotia_core::error::MagnotiaError`] +//! flattens this into a [`MagnotiaError::Storage`] variant carrying a serde-friendly +//! `kind`, an `operation` label, and the Display output as `detail` — so the +//! frontend (which still receives stringified errors from Tauri commands today) +//! keeps working, and Area E can later expose the typed `kind` to the FE without +//! touching call sites. +//! +//! The `sqlx::Error` source is deliberately not serialized. Sources stay sources; +//! only the boundary shape is wire-friendly. + +use std::borrow::Cow; +use std::path::PathBuf; + +use magnotia_core::error::{MagnotiaError, StorageKind}; + +/// Kinds of database-open operation that can fail before the pool is ready. +#[derive(Debug, Clone, Copy)] +pub enum OpenOp { + Connect, + ReadOnlyConnect, + ForeignKeysPragma, +} + +impl OpenOp { + fn label(self) -> &'static str { + match self { + OpenOp::Connect => "connect", + OpenOp::ReadOnlyConnect => "read_only_connect", + OpenOp::ForeignKeysPragma => "foreign_keys_pragma", + } + } +} + +impl std::fmt::Display for OpenOp { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.label()) + } +} + +/// Per-step phase inside the migration framework. +#[derive(Debug, Clone, Copy)] +pub enum MigrationStep { + SchemaVersionTableCreate, + SchemaVersionQuery, + TxBegin, + Apply, + RecordVersion, + Commit, +} + +impl MigrationStep { + fn label(self) -> &'static str { + match self { + MigrationStep::SchemaVersionTableCreate => "schema_version_table_create", + MigrationStep::SchemaVersionQuery => "schema_version_query", + MigrationStep::TxBegin => "tx_begin", + MigrationStep::Apply => "apply", + MigrationStep::RecordVersion => "record_version", + MigrationStep::Commit => "commit", + } + } +} + +impl std::fmt::Display for MigrationStep { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.label()) + } +} + +/// Entities the storage layer can fail to find or validate. +/// +/// Seeded with only the three real entities (Transcript, Task, Profile) — add +/// more here only when a typed `NotFound` / `InvalidReference` case actually +/// requires them. +#[derive(Debug, Clone, Copy)] +pub enum Entity { + Transcript, + Task, + Profile, + ImplementationRule, + Feedback, +} + +impl Entity { + fn label(self) -> &'static str { + match self { + Entity::Transcript => "transcript", + Entity::Task => "task", + Entity::Profile => "profile", + Entity::ImplementationRule => "implementation_rule", + Entity::Feedback => "feedback", + } + } +} + +impl std::fmt::Display for Entity { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.label()) + } +} + +/// Storage-crate-local typed error. +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("database open failed during {operation}: {source}")] + DatabaseOpen { + operation: OpenOp, + #[source] + source: sqlx::Error, + }, + + #[error("migration step {step} (version {version:?}) failed: {source}")] + Migration { + version: Option, + step: MigrationStep, + #[source] + source: sqlx::Error, + }, + + #[error("query failed during {operation}: {source}")] + Query { + operation: Cow<'static, str>, + #[source] + source: sqlx::Error, + }, + + #[error("{entity} not found: '{key}'")] + NotFound { entity: Entity, key: String }, + + #[error("invalid {entity} reference: {reason}")] + InvalidReference { + entity: Entity, + reason: Cow<'static, str>, + }, + + #[error("filesystem operation failed at '{}': {source}", path.display())] + Filesystem { + path: PathBuf, + #[source] + source: std::io::Error, + }, +} + +impl Error { + /// Discriminator for the boundary conversion into [`MagnotiaError`]. + pub fn kind(&self) -> StorageKind { + match self { + Error::DatabaseOpen { .. } => StorageKind::DatabaseOpen, + Error::Migration { .. } => StorageKind::Migration, + Error::Query { .. } => StorageKind::Query, + Error::NotFound { .. } => StorageKind::NotFound, + Error::InvalidReference { .. } => StorageKind::InvalidReference, + Error::Filesystem { .. } => StorageKind::Filesystem, + } + } + + /// Operation label that flows into the [`MagnotiaError::Storage`] boundary + /// shape. For per-query failures this is the caller-provided label; + /// for the other variants it's the variant's intrinsic label. + pub fn operation_label(&self) -> Cow<'static, str> { + match self { + Error::DatabaseOpen { operation, .. } => Cow::Borrowed(operation.label()), + Error::Migration { step, .. } => Cow::Borrowed(step.label()), + Error::Query { operation, .. } => operation.clone(), + Error::NotFound { entity, .. } => Cow::Owned(format!("not_found:{}", entity.label())), + Error::InvalidReference { entity, .. } => { + Cow::Owned(format!("invalid_reference:{}", entity.label())) + } + Error::Filesystem { .. } => Cow::Borrowed("filesystem"), + } + } +} + +/// Boundary conversion — flattens the typed error into the wire-friendly +/// [`MagnotiaError::Storage`] variant. Lives in the storage crate (not in core) +/// to avoid a `core -> storage` dependency cycle. +impl From for MagnotiaError { + fn from(error: Error) -> Self { + let kind = error.kind(); + let operation = error.operation_label().into_owned(); + let detail = error.to_string(); + MagnotiaError::Storage { + kind, + operation, + detail, + } + } +} + +/// `Result` alias for storage-crate functions. +pub type Result = std::result::Result; diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 557a7db..5320b73 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -1,7 +1,10 @@ pub mod database; +pub mod error; pub mod file_storage; pub mod migrations; +pub use error::{Entity, Error, MigrationStep, OpenOp, Result}; + /// Stable identifier for the seeded Default profile (see migration v6). /// The Default profile cannot be renamed or deleted — guarded by SQLite triggers. pub const DEFAULT_PROFILE_ID: &str = "00000000-0000-0000-0000-000000000001"; diff --git a/crates/storage/src/migrations.rs b/crates/storage/src/migrations.rs index 5b114a1..c830bf4 100644 --- a/crates/storage/src/migrations.rs +++ b/crates/storage/src/migrations.rs @@ -1,4 +1,4 @@ -use magnotia_core::error::{MagnotiaError, Result}; +use crate::error::{Error, MigrationStep, Result}; use sqlx::SqlitePool; /// Each migration is a (version, description, sql) tuple. @@ -553,29 +553,39 @@ async fn run_migrations_slice(pool: &SqlitePool, migrations: &[(i64, &str, &str) ) .execute(pool) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Schema version table creation failed: {e}")) + .map_err(|source| Error::Migration { + version: None, + step: MigrationStep::SchemaVersionTableCreate, + source, })?; let current: i64 = sqlx::query_scalar("SELECT COALESCE(MAX(version), 0) FROM schema_version") .fetch_one(pool) .await - .map_err(|e| MagnotiaError::StorageError(format!("Schema version query failed: {e}")))?; + .map_err(|source| Error::Migration { + version: None, + step: MigrationStep::SchemaVersionQuery, + source, + })?; for (version, description, sql) in migrations { if *version > current { log::info!("Running migration {}: {}", version, description); - let mut tx = pool.begin().await.map_err(|e| { - MagnotiaError::StorageError(format!("Migration {} tx begin failed: {e}", version)) + let mut tx = pool.begin().await.map_err(|source| Error::Migration { + version: Some(*version), + step: MigrationStep::TxBegin, + source, })?; for statement in split_statements(sql) { sqlx::query(&statement) .execute(&mut *tx) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Migration {} failed: {e}", version)) + .map_err(|source| Error::Migration { + version: Some(*version), + step: MigrationStep::Apply, + source, })?; } @@ -584,12 +594,16 @@ async fn run_migrations_slice(pool: &SqlitePool, migrations: &[(i64, &str, &str) .bind(description) .execute(&mut *tx) .await - .map_err(|e| { - MagnotiaError::StorageError(format!("Migration version record failed: {e}")) + .map_err(|source| Error::Migration { + version: Some(*version), + step: MigrationStep::RecordVersion, + source, })?; - tx.commit().await.map_err(|e| { - MagnotiaError::StorageError(format!("Migration {} commit failed: {e}", version)) + tx.commit().await.map_err(|source| Error::Migration { + version: Some(*version), + step: MigrationStep::Commit, + source, })?; log::info!("Migration {} complete", version);