ADR-0019 implementation: friendly error layer + i18n catalog
All eight implementation steps from ADR-0019's §"Order of
operations":
Step 1 — `src/friendly/` module skeleton; `t!()` macro; YAML
catalog loader (`include_str!` + `serde_yml`); `{name}`
substitution helper that rejects format specifiers per §8.4.
Step 2 — `error.*` catalog populated for UNIQUE / FK /
NOT NULL / CHECK / type-mismatch / not_found / already_exists /
generic / invalid_value, with verbose hints per
pedagogical-voice rule (§5). Anchor phrases (§10) preserved
verbatim.
Step 3 — `FriendlyError { headline, hint, diagnostic_table }`
+ renderer composing the three blocks per §7.
Step 4 — `translate(&DbError, &TranslateContext) → FriendlyError`.
Classifies by `SqliteErrorKind` first, then by message text
for the constraint family. `change column` failures route to
the type-mismatch headline, subsuming the previous
`friendly_change_column_engine_error` helper.
Step 5 — `DbError::friendly_message()` delegates to the
translator with default context. Removed
`friendly_change_column_engine_error` (absorbed) and
`enrich_fk_message` (FK list moves to the deferred re-query
step). One test rewritten to assert on the engine-classified
payload rather than the removed enrichment text.
Step 6 — `messages (short|verbose)` app-level command parallel
to `mode`. `App::messages_verbosity` (default verbose)
threaded into `TranslateContext` via
`App::build_translate_context`. `AppEvent::DslFailed` now
carries the structured `DbError`, plus the App extracts the
user's attempted value from `Command::Insert` / `Update`
to fill the `{value}` placeholder for UNIQUE / NOT NULL.
Step 7 — Catalog validator (§8.6) checks for missing keys,
unused/undeclared placeholders, format specifiers, and
forbidden engine vocabulary. `main.rs` parses the embedded
catalog at startup so a corrupted build artefact fails
loudly there rather than at the first `t!()` call.
Step 8 — Anchor phrases (§10) held: existing tests asserting
on "no such table", "already exists", "cannot be converted",
etc. all pass without rewording.
## Tally
603 tests passing (was 561: +42 net). Clippy clean with
nursery lints. Release binary 7.7 MB.
## Deliberately deferred
- Schema-aware enrichment for FK violations (parent_table /
parent_column / child_table) and the multi-value
natural-order INSERT case for UNIQUE. Both need the
Database handle in scope at translation time, so they
bundle naturally with the row-pinpoint re-query work
(ADR-0019 §6) — that follow-on adds runtime-side
enrichment via a `Database` lookup and a structured
failure-context carried on `DslFailed`. Until then,
unfilled placeholders render as their `{name}` form for
visual consistency with the catalog.
- Migration sweep (§9). Only `error.*` is catalog-driven so
far; `help.*`, `ok.*`, `client_side.*`, `replay.*`,
`parse.*`, modal labels, etc. migrate per-PR.
- Settings persistence for `messages`. In-session state for
now; waits on the future settings ADR.
This commit is contained in:
+388
-8
@@ -61,6 +61,10 @@ impl EffectiveMode {
|
||||
#[derive(Debug)]
|
||||
pub struct App {
|
||||
pub mode: Mode,
|
||||
/// User's preferred verbosity for friendly-error rendering.
|
||||
/// In-session state per ADR-0019 §5; persistence will land
|
||||
/// alongside the broader settings-persistence ADR.
|
||||
pub messages_verbosity: crate::friendly::Verbosity,
|
||||
pub input: String,
|
||||
/// Byte offset into `input` where the next character will be
|
||||
/// inserted. Always lies on a UTF-8 character boundary.
|
||||
@@ -213,6 +217,7 @@ impl App {
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
mode: Mode::Simple,
|
||||
messages_verbosity: crate::friendly::Verbosity::default(),
|
||||
input: String::new(),
|
||||
input_cursor: 0,
|
||||
output: VecDeque::with_capacity(OUTPUT_CAPACITY),
|
||||
@@ -322,7 +327,7 @@ impl App {
|
||||
Vec::new()
|
||||
}
|
||||
AppEvent::DslFailed { command, error } => {
|
||||
self.handle_dsl_failure(&command, &error);
|
||||
self.handle_dsl_failure(&command, error);
|
||||
Vec::new()
|
||||
}
|
||||
AppEvent::TablesRefreshed(tables) => {
|
||||
@@ -712,6 +717,10 @@ impl App {
|
||||
self.handle_mode_command(other);
|
||||
return Vec::new();
|
||||
}
|
||||
other if other.starts_with("messages") => {
|
||||
self.handle_messages_command(other);
|
||||
return Vec::new();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
@@ -923,8 +932,17 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_dsl_failure(&mut self, command: &Command, error: &str) {
|
||||
warn!(verb = command.verb(), error, "dsl command failed");
|
||||
fn handle_dsl_failure(&mut self, command: &Command, error: crate::db::DbError) {
|
||||
// Render through the friendly-error layer (ADR-0019).
|
||||
// The translator picks operation-tailored wording from
|
||||
// the catalog and applies the user's current verbosity.
|
||||
let ctx = self.build_translate_context(command, &error);
|
||||
let rendered = crate::friendly::translate_error(&error, &ctx).render();
|
||||
warn!(
|
||||
verb = command.verb(),
|
||||
error = %rendered,
|
||||
"dsl command failed"
|
||||
);
|
||||
// Wrap the command portion in quotes so the message
|
||||
// reads cleanly: "...failed: <reason>" rather than the
|
||||
// command running into "failed: ..." with no break.
|
||||
@@ -932,12 +950,101 @@ impl App {
|
||||
// diagnostics from `change column …` (ADR-0017 §7) flow
|
||||
// through as a multi-line bordered table.
|
||||
self.note_error(format!(
|
||||
"\"{} {}\" failed: {error}",
|
||||
"\"{} {}\" failed: {rendered}",
|
||||
command.verb(),
|
||||
command.display_subject()
|
||||
));
|
||||
}
|
||||
|
||||
/// Construct a [`TranslateContext`] from a [`Command`] and
|
||||
/// the App's current verbosity. Drives the operation-tailored
|
||||
/// wording the catalog produces (ADR-0019 §4).
|
||||
///
|
||||
/// `error` is consulted to extract the attempted value where
|
||||
/// the engine's constraint message names a column — for
|
||||
/// UNIQUE / NOT NULL on INSERT the user's value flows through
|
||||
/// to the `{value}` placeholder so the headline reads
|
||||
/// "...already has the value `5`" rather than "...has the
|
||||
/// value `<value>`".
|
||||
fn build_translate_context<'a>(
|
||||
&self,
|
||||
command: &'a Command,
|
||||
error: &crate::db::DbError,
|
||||
) -> crate::friendly::TranslateContext<'a> {
|
||||
use crate::dsl::{Command as C, RelationshipSelector};
|
||||
use crate::friendly::{Operation, TranslateContext};
|
||||
let (operation, table, column) = match command {
|
||||
C::CreateTable { name, .. } => (Operation::CreateTable, Some(name.as_str()), None),
|
||||
C::DropTable { name } => (Operation::DropTable, Some(name.as_str()), None),
|
||||
C::AddColumn { table, column, .. } => (
|
||||
Operation::AddColumn,
|
||||
Some(table.as_str()),
|
||||
Some(column.as_str()),
|
||||
),
|
||||
C::DropColumn { table, column } => (
|
||||
Operation::DropColumn,
|
||||
Some(table.as_str()),
|
||||
Some(column.as_str()),
|
||||
),
|
||||
C::RenameColumn { table, .. } => (Operation::RenameColumn, Some(table.as_str()), None),
|
||||
C::ChangeColumnType { table, column, .. } => (
|
||||
Operation::ChangeColumnType,
|
||||
Some(table.as_str()),
|
||||
Some(column.as_str()),
|
||||
),
|
||||
C::AddRelationship {
|
||||
parent_table,
|
||||
parent_column,
|
||||
..
|
||||
} => (
|
||||
Operation::AddRelationship,
|
||||
Some(parent_table.as_str()),
|
||||
Some(parent_column.as_str()),
|
||||
),
|
||||
C::DropRelationship { selector } => match selector {
|
||||
RelationshipSelector::Endpoints {
|
||||
parent_table,
|
||||
parent_column,
|
||||
..
|
||||
} => (
|
||||
Operation::DropRelationship,
|
||||
Some(parent_table.as_str()),
|
||||
Some(parent_column.as_str()),
|
||||
),
|
||||
RelationshipSelector::Named { .. } => (Operation::DropRelationship, None, None),
|
||||
},
|
||||
C::Insert { table, .. } => (Operation::Insert, Some(table.as_str()), None),
|
||||
C::Update { table, .. } => (Operation::Update, Some(table.as_str()), None),
|
||||
C::Delete { table, .. } => (Operation::Delete, Some(table.as_str()), None),
|
||||
C::ShowData { name } | C::ShowTable { name } => {
|
||||
(Operation::Query, Some(name.as_str()), None)
|
||||
}
|
||||
C::Replay { .. } => (Operation::Replay, None, None),
|
||||
};
|
||||
|
||||
// Try to find the value the user attempted on the
|
||||
// offending column so the catalog `{value}` placeholder
|
||||
// gets a concrete substitution. Best-effort: works when
|
||||
// the engine reports a qualified `T.col` AND the
|
||||
// command supplies explicit columns (or has a
|
||||
// single-value short form). Multi-value natural-order
|
||||
// INSERTs hide which positional value belongs to which
|
||||
// column without a schema lookup — the translator's
|
||||
// fallback `<value>` reads sensibly there until the
|
||||
// runtime-side row-pinpoint follow-on (ADR-0019 §6)
|
||||
// adds schema awareness.
|
||||
let value = attempted_value_for(command, error);
|
||||
|
||||
TranslateContext {
|
||||
operation: Some(operation),
|
||||
table,
|
||||
column,
|
||||
value,
|
||||
verbosity: self.messages_verbosity,
|
||||
..TranslateContext::default()
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse the argument tail of an `import` command and
|
||||
/// return the corresponding `Action::Import`.
|
||||
///
|
||||
@@ -1251,6 +1358,8 @@ impl App {
|
||||
" quit / q — exit",
|
||||
" help — this list",
|
||||
" mode simple|advanced — switch input mode",
|
||||
" messages — show current verbosity",
|
||||
" messages short|verbose— switch error wording (verbose is the default)",
|
||||
" rebuild — rebuild .db from project.yaml + data/ (with confirmation)",
|
||||
" save — save current temp project under a name",
|
||||
" save as — copy current project to a new name/path",
|
||||
@@ -1294,6 +1403,30 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_messages_command(&mut self, raw: &str) {
|
||||
let arg = raw.strip_prefix("messages").unwrap_or(raw).trim();
|
||||
match arg {
|
||||
"" => {
|
||||
let current = match self.messages_verbosity {
|
||||
crate::friendly::Verbosity::Short => "short",
|
||||
crate::friendly::Verbosity::Verbose => "verbose",
|
||||
};
|
||||
self.note_system(format!("messages: {current}"));
|
||||
}
|
||||
"short" => {
|
||||
self.messages_verbosity = crate::friendly::Verbosity::Short;
|
||||
self.note_system("messages: short");
|
||||
}
|
||||
"verbose" => {
|
||||
self.messages_verbosity = crate::friendly::Verbosity::Verbose;
|
||||
self.note_system("messages: verbose");
|
||||
}
|
||||
other => self.note_error(format!(
|
||||
"unknown messages mode '{other}' (expected 'short' or 'verbose')"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_mode_command(&mut self, raw: &str) {
|
||||
let arg = raw.strip_prefix("mode").unwrap_or(raw).trim();
|
||||
match arg {
|
||||
@@ -1370,6 +1503,73 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
/// Best-effort extraction of the user's attempted value for the
|
||||
/// column an engine constraint error implicates. Used to fill
|
||||
/// the catalog's `{value}` placeholder so the headline reads
|
||||
/// "...already has the value `5`" rather than `<value>`.
|
||||
///
|
||||
/// Returns `None` when:
|
||||
/// - the error isn't a `Sqlite { … }` payload (no engine
|
||||
/// message to parse the column from);
|
||||
/// - the engine message doesn't carry a qualified `T.col`
|
||||
/// target (e.g. raw "FOREIGN KEY constraint failed");
|
||||
/// - the command isn't an INSERT/UPDATE (DELETE has no value
|
||||
/// to attribute);
|
||||
/// - the command supplies multiple natural-order values and
|
||||
/// we can't tell which slot belongs to the named column
|
||||
/// without a schema lookup.
|
||||
fn attempted_value_for(
|
||||
command: &Command,
|
||||
error: &crate::db::DbError,
|
||||
) -> Option<String> {
|
||||
use crate::dsl::Command as C;
|
||||
let crate::db::DbError::Sqlite { message, .. } = error else {
|
||||
return None;
|
||||
};
|
||||
let column = column_from_qualified_target(message)?;
|
||||
match command {
|
||||
C::Insert {
|
||||
columns, values, ..
|
||||
} => {
|
||||
if let Some(cols) = columns {
|
||||
let idx = cols.iter().position(|c| c == &column)?;
|
||||
values.get(idx).map(ToString::to_string)
|
||||
} else if values.len() == 1 {
|
||||
// Natural-order short form `insert into T (v)` —
|
||||
// a single value can only belong to a single
|
||||
// column. The schema would still need to confirm
|
||||
// the column name matches, but for the common
|
||||
// single-column-table case the value is right.
|
||||
values.first().map(ToString::to_string)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
C::Update { assignments, .. } => assignments
|
||||
.iter()
|
||||
.find(|(c, _)| c == &column)
|
||||
.map(|(_, v)| v.to_string()),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Pull the column name out of a qualified target like
|
||||
/// `"UNIQUE constraint failed: T.col"`. Used by
|
||||
/// `attempted_value_for`. Returns `None` when the message
|
||||
/// doesn't have the expected shape.
|
||||
fn column_from_qualified_target(message: &str) -> Option<String> {
|
||||
let after = message.split_once(':')?.1.trim();
|
||||
let first = after.split(',').next()?.trim();
|
||||
let mut parts = first.splitn(2, '.');
|
||||
let _table = parts.next()?;
|
||||
let column = parts.next()?.trim();
|
||||
if column.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(column.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_error_message(err: &ParseError) -> String {
|
||||
match err {
|
||||
ParseError::Invalid { message, .. } => message.clone(),
|
||||
@@ -1755,19 +1955,199 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dsl_failure_event_writes_error_with_friendly_message() {
|
||||
fn dsl_failure_event_renders_through_friendly_translator() {
|
||||
// Synthetic DslFailed carries a structured DbError;
|
||||
// the App applies its current verbosity and routes the
|
||||
// payload through `friendly::translate_error` (ADR-0019).
|
||||
let mut app = App::new();
|
||||
let cmd = Command::DropTable {
|
||||
name: "Ghost".to_string(),
|
||||
};
|
||||
app.update(AppEvent::DslFailed {
|
||||
command: cmd,
|
||||
error: "no such table: Ghost".to_string(),
|
||||
error: crate::db::DbError::Sqlite {
|
||||
message: "no such table: Ghost".to_string(),
|
||||
kind: crate::db::SqliteErrorKind::NoSuchTable,
|
||||
},
|
||||
});
|
||||
let last = app.output.back().unwrap();
|
||||
assert_eq!(last.kind, OutputKind::Error);
|
||||
assert!(last.text.contains("Ghost"));
|
||||
assert!(last.text.contains("no such table"));
|
||||
// Anchor phrase + table name (ADR-0019 §10).
|
||||
assert!(last.text.contains("no such table"), "{}", last.text);
|
||||
assert!(last.text.contains("Ghost"), "{}", last.text);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn messages_command_toggles_verbosity_and_reports() {
|
||||
let mut app = App::new();
|
||||
// Default is verbose.
|
||||
type_str(&mut app, "messages");
|
||||
submit(&mut app);
|
||||
let last = app.output.back().unwrap();
|
||||
assert!(last.text.ends_with("verbose"), "{}", last.text);
|
||||
|
||||
// Switch to short.
|
||||
type_str(&mut app, "messages short");
|
||||
submit(&mut app);
|
||||
assert_eq!(app.messages_verbosity, crate::friendly::Verbosity::Short);
|
||||
let last = app.output.back().unwrap();
|
||||
assert_eq!(last.text, "messages: short");
|
||||
|
||||
// And back.
|
||||
type_str(&mut app, "messages verbose");
|
||||
submit(&mut app);
|
||||
assert_eq!(app.messages_verbosity, crate::friendly::Verbosity::Verbose);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dsl_failure_substitutes_attempted_value_for_unique_insert() {
|
||||
// The user's reported case: `insert into thing (1)`
|
||||
// (single-value short form) hits a UNIQUE constraint;
|
||||
// the headline should show `1` rather than `<value>`.
|
||||
let mut app = App::new();
|
||||
let cmd = Command::Insert {
|
||||
table: "thing".to_string(),
|
||||
columns: None,
|
||||
values: vec![crate::dsl::Value::Number("1".to_string())],
|
||||
};
|
||||
let err = crate::db::DbError::Sqlite {
|
||||
message: "UNIQUE constraint failed: thing.id".to_string(),
|
||||
kind: crate::db::SqliteErrorKind::UniqueViolation,
|
||||
};
|
||||
app.update(AppEvent::DslFailed { command: cmd, error: err });
|
||||
let body = app
|
||||
.output
|
||||
.iter()
|
||||
.map(|l| l.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
body.contains("`1`"),
|
||||
"expected the attempted value `1` in headline:\n{body}"
|
||||
);
|
||||
assert!(
|
||||
!body.contains("<value>"),
|
||||
"<value> placeholder should have been substituted:\n{body}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dsl_failure_substitutes_attempted_value_for_unique_with_explicit_columns() {
|
||||
// Columns explicitly listed: the helper should match
|
||||
// the failing column to its position in the column list
|
||||
// and substitute the corresponding value.
|
||||
let mut app = App::new();
|
||||
let cmd = Command::Insert {
|
||||
table: "Customers".to_string(),
|
||||
columns: Some(vec![
|
||||
"name".to_string(),
|
||||
"id".to_string(),
|
||||
]),
|
||||
values: vec![
|
||||
crate::dsl::Value::Text("Alice".to_string()),
|
||||
crate::dsl::Value::Number("42".to_string()),
|
||||
],
|
||||
};
|
||||
let err = crate::db::DbError::Sqlite {
|
||||
message: "UNIQUE constraint failed: Customers.id".to_string(),
|
||||
kind: crate::db::SqliteErrorKind::UniqueViolation,
|
||||
};
|
||||
app.update(AppEvent::DslFailed { command: cmd, error: err });
|
||||
let body = app
|
||||
.output
|
||||
.iter()
|
||||
.map(|l| l.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
body.contains("`42`"),
|
||||
"expected attempted id `42`:\n{body}"
|
||||
);
|
||||
assert!(!body.contains("`Alice`"), "wrong column substituted:\n{body}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dsl_failure_substitutes_attempted_value_for_unique_update() {
|
||||
// UPDATE: the value comes from the SET assignment list.
|
||||
let mut app = App::new();
|
||||
let cmd = Command::Update {
|
||||
table: "Customers".to_string(),
|
||||
assignments: vec![(
|
||||
"id".to_string(),
|
||||
crate::dsl::Value::Number("7".to_string()),
|
||||
)],
|
||||
filter: crate::dsl::RowFilter::Where {
|
||||
column: "name".to_string(),
|
||||
value: crate::dsl::Value::Text("Bob".to_string()),
|
||||
},
|
||||
};
|
||||
let err = crate::db::DbError::Sqlite {
|
||||
message: "UNIQUE constraint failed: Customers.id".to_string(),
|
||||
kind: crate::db::SqliteErrorKind::UniqueViolation,
|
||||
};
|
||||
app.update(AppEvent::DslFailed { command: cmd, error: err });
|
||||
let body = app
|
||||
.output
|
||||
.iter()
|
||||
.map(|l| l.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(body.contains("`7`"), "expected attempted id `7`:\n{body}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn messages_short_drops_the_hint_in_dsl_failure_render() {
|
||||
// Verbose mode → headline + hint. Short mode → headline only.
|
||||
// Use a UNIQUE-style violation since it has a meaty hint
|
||||
// worth measuring against.
|
||||
let mut app = App::new();
|
||||
let cmd = Command::Insert {
|
||||
table: "Customers".to_string(),
|
||||
columns: Some(vec!["id".to_string()]),
|
||||
values: vec![],
|
||||
};
|
||||
let err = || crate::db::DbError::Sqlite {
|
||||
message: "UNIQUE constraint failed: Customers.id".to_string(),
|
||||
kind: crate::db::SqliteErrorKind::UniqueViolation,
|
||||
};
|
||||
|
||||
app.messages_verbosity = crate::friendly::Verbosity::Verbose;
|
||||
app.update(AppEvent::DslFailed {
|
||||
command: cmd.clone(),
|
||||
error: err(),
|
||||
});
|
||||
let verbose_text = app
|
||||
.output
|
||||
.iter()
|
||||
.map(|l| l.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
verbose_text.contains("pick a different value"),
|
||||
"verbose mode missing hint: {verbose_text}"
|
||||
);
|
||||
|
||||
// Reset and try short.
|
||||
let mut app = App::new();
|
||||
app.messages_verbosity = crate::friendly::Verbosity::Short;
|
||||
app.update(AppEvent::DslFailed {
|
||||
command: cmd,
|
||||
error: err(),
|
||||
});
|
||||
let short_text = app
|
||||
.output
|
||||
.iter()
|
||||
.map(|l| l.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
short_text.contains("already has the value"),
|
||||
"short still has the headline: {short_text}"
|
||||
);
|
||||
assert!(
|
||||
!short_text.contains("pick a different value"),
|
||||
"short mode should not include the hint: {short_text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user