From 6ca297579e4b035b9afcc833043b751b94034f7c Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Wed, 13 May 2026 21:24:51 +0000 Subject: [PATCH] round-5 follow-up r2: migrate all thiserror Display attributes to catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the i18n sweep started in the previous commit. All remaining hand-rolled user-facing English strings inside thiserror #[error(...)] attributes have been moved into the catalog. Drops the thiserror dependency entirely. Twelve error types migrated: - dsl::action::UnknownAction → parse.custom.unknown_action - dsl::parser::ParseError → parse.error_wrapper + parse.empty - dsl::value::ValueError → value.{type_mismatch,format} - persistence::csv_io::CsvError → persistence.csv.* - persistence::mod::PersistenceError → persistence.{io,encode} - persistence::yaml::YamlError → persistence.yaml.* - persistence::migrations::MigrateError → persistence.migrate.* - project::lock::LockError → project.lock.* - project::naming::NamingError → project.naming.* - project::naming::UserNameError → project.user_name.* - project::mod::ProjectError → project.{path_not_found,...} - project::mod::SafeDeleteError → project.safe_delete.* - archive::ArchiveError → archive.* - cli::ArgsError → cli.* - db::DbError → db.error.* Pattern per type: drop thiserror::Error derive, write manual Display calling crate::t!(), keep #[from] semantics via explicit From impls, override Error::source() where applicable so #[source]-style chaining is preserved. Why this matters (user rationale): "fine to have fallbacks for errors that are purely technical, but lift the output to a place where it can be localized later and where an adjustment with friendly text is easily possible if any of them become part of the happy path." All surface strings now live in en-US.yaml and can be reworded or localized without touching Rust source. Tests: 769 passing, 0 failed, 1 ignored. Clippy clean with -D warnings. Cargo.toml: drop thiserror = "2.0.18". --- Cargo.lock | 1 - Cargo.toml | 1 - src/archive.rs | 92 ++++++++++++++++++++------- src/cli.rs | 48 +++++++++++--- src/db.rs | 65 +++++++++++++++---- src/dsl/action.rs | 19 +++++- src/dsl/parser.rs | 18 +++++- src/dsl/value.rs | 37 +++++++++-- src/friendly/keys.rs | 70 +++++++++++++++++++++ src/friendly/strings/en-US.yaml | 107 +++++++++++++++++++++++++++++++- src/persistence/csv_io.rs | 18 ++++-- src/persistence/migrations.rs | 61 +++++++++++++----- src/persistence/mod.rs | 41 ++++++++++-- src/persistence/yaml.rs | 33 ++++++++-- src/project/lock.rs | 45 +++++++++++--- src/project/mod.rs | 100 ++++++++++++++++++++++++----- src/project/naming.rs | 41 +++++++++--- 17 files changed, 680 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f96a7f2..778f367 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1429,7 +1429,6 @@ dependencies = [ "serde_yml", "sysinfo", "tempfile", - "thiserror 2.0.18", "tokio", "tracing", "tracing-subscriber", diff --git a/Cargo.toml b/Cargo.toml index 126c6af..a1d72cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ rusqlite = { version = "0.39.0", features = ["bundled"] } serde = { version = "1.0.228", features = ["derive"] } serde_yml = "0.0.12" sysinfo = { version = "0.39.0", default-features = false, features = ["system"] } -thiserror = "2.0.18" tokio = { version = "1.52.2", features = ["full"] } tracing = "0.1.44" tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } diff --git a/src/archive.rs b/src/archive.rs index b296ce9..0a8c89c 100644 --- a/src/archive.rs +++ b/src/archive.rs @@ -57,45 +57,91 @@ const IMPORT_SUFFIX_LIMIT: u32 = 99; /// explicit filename. const EXPORT_SEQUENCE_LIMIT: u32 = 99; -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum ArchiveError { - #[error("io error on `{}`: {source}", path.display())] Io { path: PathBuf, - #[source] source: io::Error, }, - #[error("zip error on `{}`: {message}", path.display())] - Zip { path: PathBuf, message: String }, - #[error( - "could not pick an export filename for `{project}` in `{}`: \ - all sequence numbers up to {limit} are taken", - target_dir.display(), - )] + Zip { + path: PathBuf, + message: String, + }, ExportSequenceExhausted { project: String, target_dir: PathBuf, limit: u32, }, - #[error( - "destination `{}` already exists and the auto-suffix retries \ - (-02 through -{limit:02}) are also taken; use \ - `import as ` to choose a different name", - path.display(), - )] - ImportCollisionExhausted { path: PathBuf, limit: u32 }, - #[error("zip is malformed: {0}")] + ImportCollisionExhausted { + path: PathBuf, + limit: u32, + }, InvalidZip(String), - #[error("zip does not contain a project (no `project.yaml` under a single top-level folder)")] NotAProjectArchive, - #[error("zip contains more than one top-level folder; refusing to extract")] MultipleTopFolders, - #[error( - "zip entry `{0}` would escape the target directory; refusing to extract" - )] UnsafeEntry(String), } +impl std::fmt::Display for ArchiveError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io { path, source } => f.write_str(&crate::t!( + "archive.io", + path = path.display(), + source = source, + )), + Self::Zip { path, message } => f.write_str(&crate::t!( + "archive.zip", + path = path.display(), + message = message, + )), + Self::ExportSequenceExhausted { + project, + target_dir, + limit, + } => f.write_str(&crate::t!( + "archive.export_sequence_exhausted", + project = project, + target_dir = target_dir.display(), + limit = limit, + )), + Self::ImportCollisionExhausted { path, limit } => { + // {limit:02} is a format specifier — the catalog + // helper rejects those (ADR-0019 §8.4). Pre-format + // the limit before substitution. + f.write_str(&crate::t!( + "archive.import_collision_exhausted", + path = path.display(), + limit = format_args!("{limit:02}"), + )) + } + Self::InvalidZip(detail) => f.write_str(&crate::t!( + "archive.invalid_zip", + detail = detail, + )), + Self::NotAProjectArchive => { + f.write_str(&crate::t!("archive.not_a_project_archive")) + } + Self::MultipleTopFolders => { + f.write_str(&crate::t!("archive.multiple_top_folders")) + } + Self::UnsafeEntry(entry) => f.write_str(&crate::t!( + "archive.unsafe_entry", + entry = entry, + )), + } + } +} + +impl std::error::Error for ArchiveError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + _ => None, + } + } +} + /// Default export-zip filename for `--export-NN.zip`. /// /// `date_yyyymmdd` is the today-local prefix (8 digits); diff --git a/src/cli.rs b/src/cli.rs index dfdcf1e..20012fe 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -42,27 +42,55 @@ pub fn help_text() -> String { crate::t!("help.cli_banner") } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum ArgsError { - #[error("missing value for --{0}")] MissingValue(&'static str), - #[error("invalid value for --{flag}: {value} (expected one of: {expected})")] InvalidValue { flag: &'static str, value: String, expected: &'static str, }, - #[error("unknown argument: {0}")] Unknown(String), - #[error("only one project path may be supplied; got both `{first}` and `{second}`")] - MultiplePaths { first: String, second: String }, - #[error( - "--resume and a positional are mutually exclusive; \ - pass one or the other" - )] + MultiplePaths { + first: String, + second: String, + }, ResumeWithPath, } +impl std::fmt::Display for ArgsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::MissingValue(flag) => f.write_str(&crate::t!( + "cli.missing_value", + flag = flag, + )), + Self::InvalidValue { + flag, + value, + expected, + } => f.write_str(&crate::t!( + "cli.invalid_value", + flag = flag, + value = value, + expected = expected, + )), + Self::Unknown(arg) => f.write_str(&crate::t!( + "cli.unknown_argument", + arg = arg, + )), + Self::MultiplePaths { first, second } => f.write_str(&crate::t!( + "cli.multiple_paths", + first = first, + second = second, + )), + Self::ResumeWithPath => f.write_str(&crate::t!("cli.resume_with_path")), + } + } +} + +impl std::error::Error for ArgsError {} + impl Args { /// Parse `Args` from the process command line. pub fn from_env() -> Result { diff --git a/src/db.rs b/src/db.rs index a57a5e4..35c66fb 100644 --- a/src/db.rs +++ b/src/db.rs @@ -110,36 +110,77 @@ pub struct ColumnDescription { pub primary_key: bool, } -#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum DbError { - #[error("database error: {message}")] - Sqlite { message: String, kind: SqliteErrorKind }, - #[error("operation not supported: {0}")] + Sqlite { + message: String, + kind: SqliteErrorKind, + }, Unsupported(String), - #[error("invalid value: {0}")] InvalidValue(String), - #[error("could not {operation} `{path}`: {message}")] PersistenceFatal { operation: &'static str, path: std::path::PathBuf, message: String, }, - #[error( - "unable to load row {row_number} from `{}` into table `{table}`: {detail}", - csv_path.display() - )] RebuildRowFailed { table: String, csv_path: std::path::PathBuf, row_number: usize, detail: String, }, - #[error("database worker is no longer available")] WorkerGone, - #[error("io error: {0}")] Io(String), } +impl std::fmt::Display for DbError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Sqlite { message, .. } => f.write_str(&crate::t!( + "db.error.sqlite", + message = message, + )), + Self::Unsupported(detail) => f.write_str(&crate::t!( + "db.error.unsupported", + detail = detail, + )), + Self::InvalidValue(detail) => f.write_str(&crate::t!( + "db.error.invalid_value", + detail = detail, + )), + Self::PersistenceFatal { + operation, + path, + message, + } => f.write_str(&crate::t!( + "db.error.persistence_fatal", + operation = operation, + path = path.display(), + message = message, + )), + Self::RebuildRowFailed { + table, + csv_path, + row_number, + detail, + } => f.write_str(&crate::t!( + "db.error.rebuild_row_failed", + row_number = row_number, + csv_path = csv_path.display(), + table = table, + detail = detail, + )), + Self::WorkerGone => f.write_str(&crate::t!("db.error.worker_gone")), + Self::Io(detail) => f.write_str(&crate::t!( + "db.error.io", + detail = detail, + )), + } + } +} + +impl std::error::Error for DbError {} + /// Result of a query / show-data call. /// /// `None` cells render as NULL; `Some(s)` renders as the diff --git a/src/dsl/action.rs b/src/dsl/action.rs index 3f5b954..5aefaab 100644 --- a/src/dsl/action.rs +++ b/src/dsl/action.rs @@ -72,13 +72,28 @@ impl fmt::Display for ReferentialAction { } /// Error returned when parsing an unknown action keyword. -#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] -#[error("unknown referential action '{found}' (expected one of: {expected})")] +/// +/// Display flows through the i18n catalog +/// (`parse.custom.unknown_action`) so the wording can be +/// localised or adjusted without touching the type. +#[derive(Debug, Clone, PartialEq, Eq)] pub struct UnknownAction { pub found: String, pub expected: String, } +impl std::fmt::Display for UnknownAction { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&crate::t!( + "parse.custom.unknown_action", + found = self.found, + expected = self.expected, + )) + } +} + +impl std::error::Error for UnknownAction {} + impl FromStr for ReferentialAction { type Err = UnknownAction; diff --git a/src/dsl/parser.rs b/src/dsl/parser.rs index 94afdee..79467a4 100644 --- a/src/dsl/parser.rs +++ b/src/dsl/parser.rs @@ -24,9 +24,8 @@ use crate::dsl::lexer::{LexError, Token, TokenKind, lex}; use crate::dsl::types::Type; use crate::dsl::value::Value; -#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ParseError { - #[error("could not parse command: {message}")] Invalid { message: String, position: usize, @@ -57,10 +56,23 @@ pub enum ParseError { /// framing). expected: Vec, }, - #[error("empty input")] Empty, } +impl std::fmt::Display for ParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Invalid { message, .. } => f.write_str(&crate::t!( + "parse.error_wrapper", + detail = message, + )), + Self::Empty => f.write_str(&crate::t!("parse.empty")), + } + } +} + +impl std::error::Error for ParseError {} + impl ParseError { #[must_use] pub const fn position(&self) -> Option { diff --git a/src/dsl/value.rs b/src/dsl/value.rs index fdaab48..83d356d 100644 --- a/src/dsl/value.rs +++ b/src/dsl/value.rs @@ -45,18 +45,47 @@ pub enum Bound { Null, } -#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +/// Per-column value validation error. +/// +/// Display flows through the i18n catalog (`value.*`); callers +/// that format this error get the localised wording for free. +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ValueError { - #[error("column `{column}` expects {expected_human}, got {got}")] TypeMismatch { column: String, expected_human: String, got: String, }, - #[error("column `{column}`: {message}")] - Format { column: String, message: String }, + Format { + column: String, + message: String, + }, } +impl std::fmt::Display for ValueError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::TypeMismatch { + column, + expected_human, + got, + } => f.write_str(&crate::t!( + "value.type_mismatch", + column = column, + expected_human = expected_human, + got = got, + )), + Self::Format { column, message } => f.write_str(&crate::t!( + "value.format", + column = column, + message = message, + )), + } + } +} + +impl std::error::Error for ValueError {} + impl Value { /// Validate `self` against `column`'s user-facing type and /// produce a value ready for binding. diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 49188fe..f0fa72d 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -146,9 +146,11 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("parse.custom.create_table_needs_pk", &[]), ("parse.custom.on_action_specified_twice", &["target"]), ("parse.custom.replay_path_expected", &[]), + ("parse.custom.unknown_action", &["found", "expected"]), ("parse.custom.unknown_type", &["found", "expected"]), ("parse.empty", &[]), ("parse.error", &["detail"]), + ("parse.error_wrapper", &["detail"]), // Per-command usage templates (ADR-0021 §1). One key per // command. Multi-entry families (`add`, `drop`, `show`) // each have multiple keys. Templates are pure prose with @@ -253,7 +255,40 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("project.import_empty_target", &[]), ("project.import_usage", &[]), ("project.import_zip_missing", &["path"]), + ("persistence.csv.empty", &[]), + ("persistence.csv.invalid_utf8", &[]), + ("persistence.csv.unterminated_quote", &[]), + ("persistence.encode", &["kind", "path", "message"]), + ("persistence.io", &["operation", "path", "source"]), + ("persistence.migrate.bad_output", &["detail"]), + ("persistence.migrate.io", &["path", "source"]), + ( + "persistence.migrate.newer_than_supported", + &["file", "latest"], + ), + ("persistence.migrate.no_migrator", &["version"]), + ("persistence.migrate.step_failed", &["from", "to", "source"]), + ("persistence.migrate.version_parse", &["detail"]), + ("persistence.yaml.syntax", &["detail"]), + ("persistence.yaml.unknown_action", &["raw"]), + ("persistence.yaml.unknown_type", &["table", "column", "raw"]), + ("persistence.yaml.unsupported_version", &["version"]), + ("project.already_exists", &["path"]), + ("project.data_root_unavailable", &[]), + ("project.io", &["path", "source"]), ("project.load_path_missing", &["path"]), + ("project.lock.already_held", &["pid", "hostname", "path"]), + ("project.lock.read", &["path", "source"]), + ("project.lock.write", &["path", "source"]), + ("project.naming.too_many_collisions", &["attempts"]), + ("project.naming.wordlist_too_small", &["count"]), + ("project.not_a_project", &["path"]), + ("project.path_not_found", &["path"]), + ("project.safe_delete.io", &["path", "source"]), + ("project.safe_delete.refused", &["path", "reason"]), + ("project.user_name.empty", &[]), + ("project.user_name.invalid_char", &["ch"]), + ("project.user_name.leading_dot", &[]), ("project.resume_no_previous", &["data_root"]), ("project.resume_recorded_missing", &["path"]), ("project.saveas_target_exists", &["path"]), @@ -263,6 +298,25 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("project.switched_ok", &["display_name"]), // ---- Advanced-mode placeholder ---- ("advanced_mode.not_implemented", &["input"]), + ( + "cli.invalid_value", + &["flag", "value", "expected"], + ), + ("cli.missing_value", &["flag"]), + ("cli.multiple_paths", &["first", "second"]), + ("cli.resume_with_path", &[]), + ("cli.unknown_argument", &["arg"]), + ( + "archive.export_sequence_exhausted", + &["project", "target_dir", "limit"], + ), + ("archive.import_collision_exhausted", &["path", "limit"]), + ("archive.invalid_zip", &["detail"]), + ("archive.io", &["path", "source"]), + ("archive.multiple_top_folders", &[]), + ("archive.not_a_project_archive", &[]), + ("archive.unsafe_entry", &["entry"]), + ("archive.zip", &["path", "message"]), // ---- DSL failure wrapper / running echo ---- ("dsl.failed", &["verb", "subject", "rendered"]), ("dsl.running", &["input"]), @@ -288,6 +342,8 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("panel.tables_title", &[]), ("status.no_project", &[]), ("status.project_label", &[]), + ("value.format", &["column", "message"]), + ("value.type_mismatch", &["column", "expected_human", "got"]), // ---- Save / save-as surfaces ---- ("save.already_saved", &[]), ("save.path_prompt", &[]), @@ -321,6 +377,20 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("mode.show_simple", &[]), ("mode.unknown", &["value"]), ("mode.usage", &[]), + // ---- DbError Display fallback ---- + ("db.error.invalid_value", &["detail"]), + ("db.error.io", &["detail"]), + ( + "db.error.persistence_fatal", + &["operation", "path", "message"], + ), + ( + "db.error.rebuild_row_failed", + &["row_number", "csv_path", "table", "detail"], + ), + ("db.error.sqlite", &["message"]), + ("db.error.unsupported", &["detail"]), + ("db.error.worker_gone", &[]), // ---- Cascade-effect summaries (per ADR-0014) ---- ("db.cascade.action_blocked", &[]), ("db.cascade.action_deleted", &[]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 0ed6068..6648a8c 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -153,6 +153,15 @@ error: headline: "UPDATE requires at least one assignment." # ---- Help text (CLI banner + in-app `help` command) ------------------ +# ---- CLI argument-parsing errors (stderr before TUI starts) --------- +cli: + missing_value: "missing value for --{flag}" + invalid_value: "invalid value for --{flag}: {value} (expected one of: {expected})" + unknown_argument: "unknown argument: {arg}" + multiple_paths: "only one project path may be supplied; got both `{first}` and `{second}`" + resume_with_path: |- + --resume and a positional are mutually exclusive; pass one or the other + help: # CLI usage banner. Printed by `--help` / `-h` and on # argument-parse failure. Multi-line block; consumers @@ -279,6 +288,10 @@ parse: # caret pointer (visualising the failure column) is printed # on its own preceding line via `parse.caret`. error: "parse error: {detail}" + # Wrapper used by `ParseError::Display` (so any to_string() + # call on a parse error renders consistently with the in-app + # error rendering). + error_wrapper: "could not parse command: {detail}" # Custom (try_map / source-slice) error messages raised by # the DSL parser. These were hand-written strings in # `src/dsl/parser.rs` until the catalog migration brought @@ -292,6 +305,7 @@ parse: on_action_specified_twice: "`on {target}` specified twice" change_column_flags_exclusive: "`--force-conversion` and `--dont-convert` are mutually exclusive — pick one." unknown_type: "unknown type '{found}' (expected one of: {expected})" + unknown_action: "unknown referential action '{found}' (expected one of: {expected})" # Caret pointer showing where in the input the parser # failed. `{padding}` is the leading whitespace; the # template appends `^` so the rendered line places the @@ -444,9 +458,38 @@ project: # --resume CLI failures printed to stderr before the TUI # starts (ADR-0015 §7). Wording stays one line for clean # piping; the runtime prepends `rdbms-playground: ` from - # `cli.binary_prefix` itself. + # the calling code itself. resume_recorded_missing: "--resume: recorded project `{path}` no longer exists" resume_no_previous: "--resume: no previous project recorded under `{data_root}`" + # Project-lock errors (single-instance enforcement, ADR-0015 §6). + lock: + already_held: |- + project is already open in another rdbms-playground process (pid {pid} on host `{hostname}`); close that process or remove `{path}` if you're sure it's not running + write: "could not write lock file `{path}`: {source}" + read: "could not read existing lock file `{path}`: {source}" + # Temp-project name generation failures. + naming: + wordlist_too_small: "wordlist must contain at least 3 entries; found {count}" + too_many_collisions: |- + could not generate a non-colliding temp project name after {attempts} attempts + # User-typed project name validation failures. + user_name: + empty: "project name cannot be empty" + leading_dot: "project name cannot start with `.`" + invalid_char: "project name cannot contain `{ch}`; use letters, digits, `-`, `_`, or `.` only" + # ProjectError variants (ProjectError Display path). + data_root_unavailable: |- + could not determine the OS-standard data directory; pass --data-dir to override + path_not_found: "project path `{path}` does not exist" + not_a_project: |- + path `{path}` does not look like a project directory (no `project.yaml` and no `playground.db`) + already_exists: "path `{path}` already exists; pick a different name or remove it first" + io: "filesystem error at `{path}`: {source}" + # SafeDeleteError — surfaces in logs when temp-project cleanup + # refuses to delete a path (ADR-0015 §13). + safe_delete: + refused: "refusing to delete `{path}`: {reason}" + io: "io error on `{path}`: {source}" # ---- DSL failure wrapper + advanced-mode placeholder + fatal -------- dsl: @@ -458,6 +501,51 @@ dsl: # output that follows. running: "running: {input}" +# ---- Value-validation errors (per-column at bind time) -------------- +value: + type_mismatch: "column `{column}` expects {expected_human}, got {got}" + format: "column `{column}`: {message}" + +# ---- Archive / zip errors (export / import) ------------------------- +archive: + io: "io error on `{path}`: {source}" + zip: "zip error on `{path}`: {message}" + export_sequence_exhausted: |- + could not pick an export filename for `{project}` in `{target_dir}`: all sequence numbers up to {limit} are taken + import_collision_exhausted: |- + destination `{path}` already exists and the auto-suffix retries (-02 through -{limit}) are also taken; use `import as ` to choose a different name + invalid_zip: "zip is malformed: {detail}" + not_a_project_archive: |- + zip does not contain a project (no `project.yaml` under a single top-level folder) + multiple_top_folders: "zip contains more than one top-level folder; refusing to extract" + unsafe_entry: "zip entry `{entry}` would escape the target directory; refusing to extract" + +# ---- Persistence-layer errors (CSV/YAML/IO) ------------------------- +# These were thiserror Display attributes pre-round-6. Most surface +# only as the inner `{message}` of `fatal.persistence` or as the +# wrapped detail inside `DbError::PersistenceFatal`. +persistence: + io: "could not {operation} `{path}`: {source}" + encode: "could not encode {kind} for `{path}`: {message}" + csv: + empty: "CSV is empty" + invalid_utf8: "invalid UTF-8 in CSV body" + unterminated_quote: "unterminated quoted field" + yaml: + syntax: "project.yaml syntax error: {detail}" + unsupported_version: "unsupported project.yaml version: {version} (expected 1)" + unknown_type: "unknown user-facing column type `{raw}` for `{table}.{column}`" + unknown_action: "unknown referential action `{raw}`" + migrate: + version_parse: "could not read version field from project.yaml: {detail}" + newer_than_supported: |- + project.yaml is at version {file} but this build only understands up to version {latest}; upgrade the application or restore an older project.yaml + no_migrator: |- + no migrator registered for version {version} (programmer error: registry latest_version disagrees with migrators length) + step_failed: "migrator from v{from} to v{to} failed: {source}" + bad_output: "migrator produced an unparseable result: {detail}" + io: "io error during migration on `{path}`: {source}" + # ---- Advanced-mode placeholder until SQL parser lands (Q1) ---------- advanced_mode: not_implemented: "advanced mode SQL not implemented yet — echo: {input}" @@ -542,8 +630,23 @@ messages: set_verbose: "messages: verbose" unknown: "unknown messages mode '{value}' (expected 'short' or 'verbose')" -# ---- Cascade-effect summaries (per ADR-0014 delete reporting) ------- +# ---- Database-error fallback wording + cascade summaries ------------ db: + # DbError variants — fallback Display wording for paths that + # bypass the structured friendly translator (fatal banners, + # plain to_string() calls). The normal path goes through + # `friendly::translate_error` which routes by the `kind` + # field and renders catalog wording from `error.*` instead. + error: + sqlite: "database error: {message}" + unsupported: "operation not supported: {detail}" + invalid_value: "invalid value: {detail}" + persistence_fatal: "could not {operation} `{path}`: {message}" + rebuild_row_failed: |- + unable to load row {row_number} from `{csv_path}` into table `{table}`: {detail} + worker_gone: "database worker is no longer available" + io: "io error: {detail}" + cascade: # Per-relationship cascade summary appended to a delete # success note. The same template handles cascade, diff --git a/src/persistence/csv_io.rs b/src/persistence/csv_io.rs index 88690a2..3dd0de5 100644 --- a/src/persistence/csv_io.rs +++ b/src/persistence/csv_io.rs @@ -197,16 +197,26 @@ pub(crate) struct RawCell { pub was_quoted: bool, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub(crate) enum CsvError { - #[error("CSV is empty")] Empty, - #[error("invalid UTF-8 in CSV body")] InvalidUtf8, - #[error("unterminated quoted field")] UnterminatedQuote, } +impl std::fmt::Display for CsvError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let key = match self { + Self::Empty => "persistence.csv.empty", + Self::InvalidUtf8 => "persistence.csv.invalid_utf8", + Self::UnterminatedQuote => "persistence.csv.unterminated_quote", + }; + f.write_str(&crate::friendly::translate(key, &[])) + } +} + +impl std::error::Error for CsvError {} + /// Tokenize a CSV body. Returns the header (column names from /// the first record) and the data rows. Each cell preserves a /// `was_quoted` flag so the caller can distinguish an empty diff --git a/src/persistence/migrations.rs b/src/persistence/migrations.rs index b88e827..4099f71 100644 --- a/src/persistence/migrations.rs +++ b/src/persistence/migrations.rs @@ -79,37 +79,68 @@ impl Default for MigratorRegistry { } } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum MigrateError { - #[error("could not read version field from project.yaml: {0}")] VersionParse(String), - #[error( - "project.yaml is at version {file} but this build only understands \ - up to version {latest}; upgrade the application or restore an \ - older project.yaml" - )] NewerThanSupported { file: u32, latest: u32 }, - #[error( - "no migrator registered for version {0} (programmer error: \ - registry latest_version disagrees with migrators length)" - )] NoMigratorForVersion(u32), - #[error("migrator from v{from} to v{to} failed: {source}")] StepFailed { from: u32, to: u32, source: Box, }, - #[error("migrator produced an unparseable result: {0}")] BadOutput(String), - #[error("io error during migration on `{}`: {source}", path.display())] Io { path: std::path::PathBuf, - #[source] source: std::io::Error, }, } +impl std::fmt::Display for MigrateError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::VersionParse(detail) => f.write_str(&crate::t!( + "persistence.migrate.version_parse", + detail = detail, + )), + Self::NewerThanSupported { file, latest } => f.write_str(&crate::t!( + "persistence.migrate.newer_than_supported", + file = file, + latest = latest, + )), + Self::NoMigratorForVersion(v) => f.write_str(&crate::t!( + "persistence.migrate.no_migrator", + version = v, + )), + Self::StepFailed { from, to, source } => f.write_str(&crate::t!( + "persistence.migrate.step_failed", + from = from, + to = to, + source = source, + )), + Self::BadOutput(detail) => f.write_str(&crate::t!( + "persistence.migrate.bad_output", + detail = detail, + )), + Self::Io { path, source } => f.write_str(&crate::t!( + "persistence.migrate.io", + path = path.display(), + source = source, + )), + } + } +} + +impl std::error::Error for MigrateError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::StepFailed { source, .. } => Some(source.as_ref()), + Self::Io { source, .. } => Some(source), + _ => None, + } + } +} + /// Result of running [`migrate_to_latest`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct MigrationOutcome { diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs index e0cc6e6..3f04a35 100644 --- a/src/persistence/mod.rs +++ b/src/persistence/mod.rs @@ -39,16 +39,13 @@ pub struct Persistence { project_path: PathBuf, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum PersistenceError { - #[error("could not {operation} `{path}`: {source}")] Io { operation: &'static str, path: PathBuf, - #[source] source: std::io::Error, }, - #[error("could not encode {kind} for `{path}`: {message}")] Encode { kind: &'static str, path: PathBuf, @@ -56,6 +53,42 @@ pub enum PersistenceError { }, } +impl std::fmt::Display for PersistenceError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Io { + operation, + path, + source, + } => f.write_str(&crate::t!( + "persistence.io", + operation = operation, + path = path.display(), + source = source, + )), + Self::Encode { + kind, + path, + message, + } => f.write_str(&crate::t!( + "persistence.encode", + kind = kind, + path = path.display(), + message = message, + )), + } + } +} + +impl std::error::Error for PersistenceError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + Self::Encode { .. } => None, + } + } +} + impl PersistenceError { /// Path the failure was associated with. #[must_use] diff --git a/src/persistence/yaml.rs b/src/persistence/yaml.rs index 89a935e..a0b77ad 100644 --- a/src/persistence/yaml.rs +++ b/src/persistence/yaml.rs @@ -222,22 +222,45 @@ pub(crate) fn parse_schema(body: &str) -> Result { }) } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub(crate) enum YamlError { - #[error("project.yaml syntax error: {0}")] Syntax(String), - #[error("unsupported project.yaml version: {0} (expected 1)")] UnsupportedVersion(u32), - #[error("unknown user-facing column type `{raw}` for `{table}.{column}`")] UnknownType { table: String, column: String, raw: String, }, - #[error("unknown referential action `{0}`")] UnknownAction(String), } +impl std::fmt::Display for YamlError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Syntax(msg) => f.write_str(&crate::t!( + "persistence.yaml.syntax", + detail = msg, + )), + Self::UnsupportedVersion(v) => f.write_str(&crate::t!( + "persistence.yaml.unsupported_version", + version = v, + )), + Self::UnknownType { table, column, raw } => f.write_str(&crate::t!( + "persistence.yaml.unknown_type", + table = table, + column = column, + raw = raw, + )), + Self::UnknownAction(raw) => f.write_str(&crate::t!( + "persistence.yaml.unknown_action", + raw = raw, + )), + } + } +} + +impl std::error::Error for YamlError {} + fn parse_action(s: &str) -> Option { match s { "no_action" => Some(ReferentialAction::NoAction), diff --git a/src/project/lock.rs b/src/project/lock.rs index d3b074f..958972c 100644 --- a/src/project/lock.rs +++ b/src/project/lock.rs @@ -33,30 +33,59 @@ pub struct Lock { path: PathBuf, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum LockError { - #[error( - "project is already open in another rdbms-playground process \ - (pid {pid} on host `{hostname}`); close that process or \ - remove `{path}` if you're sure it's not running" - )] AlreadyHeld { pid: u32, hostname: String, path: PathBuf, }, - #[error("could not write lock file `{path}`: {source}")] Write { path: PathBuf, source: std::io::Error, }, - #[error("could not read existing lock file `{path}`: {source}")] Read { path: PathBuf, source: std::io::Error, }, } +impl std::fmt::Display for LockError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::AlreadyHeld { + pid, + hostname, + path, + } => f.write_str(&crate::t!( + "project.lock.already_held", + pid = pid, + hostname = hostname, + path = path.display(), + )), + Self::Write { path, source } => f.write_str(&crate::t!( + "project.lock.write", + path = path.display(), + source = source, + )), + Self::Read { path, source } => f.write_str(&crate::t!( + "project.lock.read", + path = path.display(), + source = source, + )), + } + } +} + +impl std::error::Error for LockError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::AlreadyHeld { .. } => None, + Self::Write { source, .. } | Self::Read { source, .. } => Some(source), + } + } +} + impl Lock { /// Attempt to acquire the lock for `project_dir`. The /// directory itself must exist; `Project::open` / diff --git a/src/project/mod.rs b/src/project/mod.rs index 57e905c..8fec435 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -238,28 +238,72 @@ pub enum ProjectKind { Named, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum ProjectError { - #[error("could not determine the OS-standard data directory; pass --data-dir to override")] DataRootUnavailable, - #[error("project path `{0}` does not exist")] PathNotFound(PathBuf), - #[error( - "path `{0}` does not look like a project directory \ - (no `project.yaml` and no `playground.db`)" - )] NotAProject(PathBuf), - #[error("path `{0}` already exists; pick a different name or remove it first")] AlreadyExists(PathBuf), - #[error("filesystem error at `{path}`: {source}")] Io { path: PathBuf, source: std::io::Error, }, - #[error(transparent)] - Naming(#[from] NamingError), - #[error(transparent)] - Lock(#[from] LockError), + Naming(NamingError), + Lock(LockError), +} + +impl std::fmt::Display for ProjectError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::DataRootUnavailable => { + f.write_str(&crate::t!("project.data_root_unavailable")) + } + Self::PathNotFound(p) => f.write_str(&crate::t!( + "project.path_not_found", + path = p.display(), + )), + Self::NotAProject(p) => f.write_str(&crate::t!( + "project.not_a_project", + path = p.display(), + )), + Self::AlreadyExists(p) => f.write_str(&crate::t!( + "project.already_exists", + path = p.display(), + )), + Self::Io { path, source } => f.write_str(&crate::t!( + "project.io", + path = path.display(), + source = source, + )), + // Naming and Lock are transparent — their own Display + // impls already route through the catalog. + Self::Naming(inner) => std::fmt::Display::fmt(inner, f), + Self::Lock(inner) => std::fmt::Display::fmt(inner, f), + } + } +} + +impl std::error::Error for ProjectError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + Self::Naming(inner) => Some(inner), + Self::Lock(inner) => Some(inner), + _ => None, + } + } +} + +impl From for ProjectError { + fn from(e: NamingError) -> Self { + Self::Naming(e) + } +} + +impl From for ProjectError { + fn from(e: LockError) -> Self { + Self::Lock(e) + } } impl Project { @@ -463,20 +507,44 @@ const ALLOWED_PROJECT_ENTRIES: &[&str] = &[ /// Reasons `safely_delete_temp_project` refuses to delete a /// path. Each variant carries enough detail to surface in a /// log warning. -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum SafeDeleteError { - #[error("refusing to delete `{}`: {reason}", path.display())] Refused { path: PathBuf, reason: &'static str, }, - #[error("io error on `{}`: {source}", path.display())] Io { path: PathBuf, source: std::io::Error, }, } +impl std::fmt::Display for SafeDeleteError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Refused { path, reason } => f.write_str(&crate::t!( + "project.safe_delete.refused", + path = path.display(), + reason = reason, + )), + Self::Io { path, source } => f.write_str(&crate::t!( + "project.safe_delete.io", + path = path.display(), + source = source, + )), + } + } +} + +impl std::error::Error for SafeDeleteError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + Self::Refused { .. } => None, + } + } +} + /// Conservatively delete a temp project's directory. /// /// Stacks every guard we can think of so a bug elsewhere (or diff --git a/src/project/naming.rs b/src/project/naming.rs index 3d59a9f..1f4e195 100644 --- a/src/project/naming.rs +++ b/src/project/naming.rs @@ -32,14 +32,29 @@ fn words() -> Vec<&'static str> { .collect() } -#[derive(Debug, thiserror::Error)] +#[derive(Debug)] pub enum NamingError { - #[error("wordlist must contain at least 3 entries; found {0}")] WordlistTooSmall(usize), - #[error("could not generate a non-colliding temp project name after {0} attempts")] TooManyCollisions(usize), } +impl std::fmt::Display for NamingError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::WordlistTooSmall(n) => f.write_str(&crate::t!( + "project.naming.wordlist_too_small", + count = n, + )), + Self::TooManyCollisions(n) => f.write_str(&crate::t!( + "project.naming.too_many_collisions", + attempts = n, + )), + } + } +} + +impl std::error::Error for NamingError {} + /// Generate a fresh temp project directory name. /// /// Checks for collisions against `parent_dir` (typically @@ -162,16 +177,28 @@ pub fn validate_user_name(name: &str) -> Result<(), UserNameError> { Ok(()) } -#[derive(Debug, thiserror::Error, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub enum UserNameError { - #[error("project name cannot be empty")] Empty, - #[error("project name cannot start with `.`")] LeadingDot, - #[error("project name cannot contain `{0}`; use letters, digits, `-`, `_`, or `.` only")] InvalidChar(char), } +impl std::fmt::Display for UserNameError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Empty => f.write_str(&crate::t!("project.user_name.empty")), + Self::LeadingDot => f.write_str(&crate::t!("project.user_name.leading_dot")), + Self::InvalidChar(c) => f.write_str(&crate::t!( + "project.user_name.invalid_char", + ch = c, + )), + } + } +} + +impl std::error::Error for UserNameError {} + #[cfg(test)] mod tests { use super::*;