diff --git a/linux/changelog.d/3555.added.md b/linux/changelog.d/3555.added.md new file mode 100644 index 0000000000..ac4e30ad34 --- /dev/null +++ b/linux/changelog.d/3555.added.md @@ -0,0 +1 @@ +Translatable error details for CSV import, cell edits, column types and defaults, the keyring and driver refusals. diff --git a/linux/crates/app/src/services/secret_save_report.rs b/linux/crates/app/src/services/secret_save_report.rs index 683fc9aeb2..7f65adabcd 100644 --- a/linux/crates/app/src/services/secret_save_report.rs +++ b/linux/crates/app/src/services/secret_save_report.rs @@ -13,10 +13,16 @@ pub(crate) fn not_saved_message(error: &SecretError) -> String { SecretError::Locked | SecretError::UnlockDismissed => crate::i18n::gettext( "The keyring stayed locked, so the password was not saved. It will be asked for next time.", ), - SecretError::Backend { detail } => { - crate::i18n::gettext_f("The keyring did not save the password: {reason}", &[("reason", detail)]) + SecretError::Backend { + service_message: Some(message), + .. + } => crate::i18n::gettext_f( + "The keyring did not save the password: {reason}", + &[("reason", message)], + ), + SecretError::Backend { .. } | SecretError::InvalidEncoding => { + crate::i18n::gettext("The keyring did not save the password.") } - SecretError::InvalidEncoding => crate::i18n::gettext("The keyring did not save the password."), } } @@ -28,12 +34,17 @@ mod tests { fn the_message_says_what_happened_in_words_not_the_error_text() { let locked = not_saved_message(&SecretError::UnlockDismissed); let backend = not_saved_message(&SecretError::Backend { - detail: "org.freedesktop.Secret.Error.NoSuchObject".to_owned(), + detail: "DBus error service error org.freedesktop.Secret.Error.NoSuchObject".to_owned(), + service_message: Some("No such collection".to_owned()), + }); + let silent = not_saved_message(&SecretError::Backend { + detail: "DBus error Item/Collection was deleted, can no longer be used".to_owned(), + service_message: None, }); assert!(locked.contains("password was not saved"), "{locked}"); assert!(!locked.contains(&SecretError::UnlockDismissed.to_string()), "{locked}"); - assert!(backend.contains("NoSuchObject"), "{backend}"); - assert!(!backend.contains("the secret service reported"), "{backend}"); + assert_eq!(backend, "The keyring did not save the password: No such collection"); + assert_eq!(silent, "The keyring did not save the password."); } } diff --git a/linux/crates/app/src/test_support/log_lines.rs b/linux/crates/app/src/test_support/log_lines.rs new file mode 100644 index 0000000000..9985374c77 --- /dev/null +++ b/linux/crates/app/src/test_support/log_lines.rs @@ -0,0 +1,36 @@ +use std::sync::{Arc, Mutex, PoisonError}; + +/// What this thread logs, as text. +#[derive(Clone, Default)] +pub(crate) struct LogLines(Arc>>); + +impl LogLines { + /// A subscriber that writes here, for `tracing::subscriber::set_default`. + pub(crate) fn subscriber(&self, level: tracing::Level) -> impl tracing::Subscriber + Send + Sync + 'static { + let lines = self.clone(); + tracing_subscriber::fmt() + .with_ansi(false) + .with_max_level(level) + .with_writer(move || lines.clone()) + .finish() + } + + pub(crate) fn text(&self) -> String { + let bytes = self.0.lock().unwrap_or_else(PoisonError::into_inner); + String::from_utf8_lossy(&bytes).into_owned() + } +} + +impl std::io::Write for LogLines { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0 + .lock() + .unwrap_or_else(PoisonError::into_inner) + .extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} diff --git a/linux/crates/app/src/test_support/mod.rs b/linux/crates/app/src/test_support/mod.rs index c780e8ac2d..0c4c1bcd04 100644 --- a/linux/crates/app/src/test_support/mod.rs +++ b/linux/crates/app/src/test_support/mod.rs @@ -8,6 +8,7 @@ mod focus_thief; mod icon_buttons; mod key_injection_error; mod key_injector; +mod log_lines; mod main_loop; mod memory_settings; mod owned_window; @@ -32,6 +33,7 @@ pub(crate) use icon_buttons::{ }; pub(crate) use key_injection_error::KeyInjectionError; pub(crate) use key_injector::KeyInjector; +pub(crate) use log_lines::LogLines; pub(crate) use main_loop::{after_next_frame, drain_main_context, finish_on_main, wait_until}; pub(crate) use memory_settings::MemorySettings; pub(crate) use owned_window::{OwnedWindow, drop_focus}; diff --git a/linux/crates/app/src/ui/browse_tab/tests/mod.rs b/linux/crates/app/src/ui/browse_tab/tests/mod.rs index 432871a5e4..f2eb09c7e3 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/mod.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/mod.rs @@ -24,7 +24,7 @@ use crate::services::change_tracker::{RowKey, StatementSource}; use crate::services::tab_tracker_handle::TabTrackerHandle; use crate::services::tracker_delta::TrackerDelta; use crate::services::tracker_edit::TrackerEdit; -use crate::test_support::{FakeSession, MemorySettings, after_next_frame, drain_main_context, wait_until}; +use crate::test_support::{FakeSession, LogLines, MemorySettings, after_next_frame, drain_main_context, wait_until}; use crate::ui::clipboard::InsertRow; use crate::ui::grid::{GridCell, GridRowIndex, RowObject}; @@ -2842,43 +2842,10 @@ fn a_bulk_delete_waiting_while_a_column_is_added_marks_its_rows_under_the_new_co ); } -/// What this thread logs, as text. -#[derive(Clone, Default)] -pub(super) struct LogLines(Arc>>); - -impl LogLines { - pub(super) fn text(&self) -> String { - let bytes = self.0.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - String::from_utf8_lossy(&bytes).into_owned() - } -} - -impl std::io::Write for LogLines { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.0 - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .extend_from_slice(bytes); - Ok(bytes.len()) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) - } -} - #[gtk4::test] fn a_failed_row_count_logs_its_category_and_not_the_server_message() { let lines = LogLines::default(); - let subscriber = tracing_subscriber::fmt() - .with_ansi(false) - .with_max_level(tracing::Level::DEBUG) - .with_writer({ - let lines = lines.clone(); - move || lines.clone() - }) - .finish(); - let _logging = tracing::subscriber::set_default(subscriber); + let _logging = tracing::subscriber::set_default(lines.subscriber(tracing::Level::DEBUG)); let mut script = serving_table(); script.query_bound.always(FakeReply::ok(rows(&["Ada"]))); script.count_rows.always(FakeReply::err(DriverError::reported( diff --git a/linux/crates/app/src/ui/browse_tab/tests/total.rs b/linux/crates/app/src/ui/browse_tab/tests/total.rs index 2266369ea5..90fc76daff 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/total.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/total.rs @@ -9,10 +9,10 @@ use tablepro_test_support::{CallCounters, ConnectionScript, FakeReply}; use super::super::{PagingInput, SaveInput}; use super::{ - Harness, LogLines, WAIT, estimate, full_page, not_null, not_null_name, paging, rows, rows_from, serving_table, text, + Harness, WAIT, estimate, full_page, not_null, not_null_name, paging, rows, rows_from, serving_table, text, }; use crate::services::change_origins::ChangeOrigins; -use crate::test_support::{drain_main_context, wait_until}; +use crate::test_support::{LogLines, drain_main_context, wait_until}; fn paginator(harness: &Harness) -> String { harness.tab().imp().pager_bar.imp().paginator_label.label().to_string() @@ -75,14 +75,7 @@ fn unknown_estimate_offers_count_without_counting() { #[gtk4::test] fn estimate_error_offers_count_and_logs() { let lines = LogLines::default(); - let subscriber = tracing_subscriber::fmt() - .with_ansi(false) - .with_writer({ - let lines = lines.clone(); - move || lines.clone() - }) - .finish(); - let _logging = tracing::subscriber::set_default(subscriber); + let _logging = tracing::subscriber::set_default(lines.subscriber(tracing::Level::INFO)); let mut script = estimated(RowCountEstimate::Unknown); script.table_estimate = Default::default(); script diff --git a/linux/crates/app/src/ui/connect_dialog/mod.rs b/linux/crates/app/src/ui/connect_dialog/mod.rs index 1f80c2fa41..34061dbcfc 100644 --- a/linux/crates/app/src/ui/connect_dialog/mod.rs +++ b/linux/crates/app/src/ui/connect_dialog/mod.rs @@ -506,10 +506,11 @@ impl ConnectDialog { let weak = self.downgrade(); glib::spawn_future_local(async move { let result = check.await.unwrap_or_else(|failure| { + tracing::warn!(%failure, "the certificate check did not finish"); Err(ConfigError::CaFile { path: fallback_path, problem: tablepro_core::CertificateFileProblem::Unreadable { - detail: failure.to_string(), + kind: std::io::ErrorKind::Other, }, }) }); diff --git a/linux/crates/app/src/ui/error_text/ddl.rs b/linux/crates/app/src/ui/error_text/ddl.rs index 15091002c8..6bd6475393 100644 --- a/linux/crates/app/src/ui/error_text/ddl.rs +++ b/linux/crates/app/src/ui/error_text/ddl.rs @@ -3,6 +3,7 @@ use tablepro_core::ddl::{BuildDdlError, DdlOperation, DraftProblem, RefusalReason}; +use super::syntax::{literal_message, type_syntax_message}; use crate::services::pending_structure::SaveBlocker; /// Why a control is off, for its tooltip. @@ -101,12 +102,12 @@ pub fn build_error_text(error: &BuildDdlError) -> String { ), BuildDdlError::Unsupported(_) => crate::i18n::gettext("This database can't make this change."), BuildDdlError::InvalidType { column, error } => crate::i18n::gettext_f( - "Column {column} has a type this database doesn't accept: {error}", - &[("column", column), ("error", &error.to_string())], + "The type of column {column} can't be used. {reason}", + &[("column", column), ("reason", &type_syntax_message(error))], ), BuildDdlError::Literal { column, error } => crate::i18n::gettext_f( - "The default of column {column} can't be written as SQL: {error}", - &[("column", column), ("error", &error.to_string())], + "The default of column {column} can't be written as SQL. {reason}", + &[("column", column), ("reason", &literal_message(error))], ), BuildDdlError::MissingCatalogDeclaration(column) => crate::i18n::gettext_f( "The database didn't report the type of column {column}.", diff --git a/linux/crates/app/src/ui/error_text/driver.rs b/linux/crates/app/src/ui/error_text/driver.rs index 30b150275d..cc99066468 100644 --- a/linux/crates/app/src/ui/error_text/driver.rs +++ b/linux/crates/app/src/ui/error_text/driver.rs @@ -3,8 +3,10 @@ use tablepro_core::{ TransportError, }; +use super::files::{certificate_problem_text, file_io_message}; use super::ssh::ssh_failure_message; use super::tls::tls_message; +use super::unsupported::unsupported_message; pub fn driver_message(error: &DriverError) -> String { // An SSH failure carries its own category and its own next step. @@ -16,13 +18,13 @@ pub fn driver_message(error: &DriverError) -> String { ErrorCategory::Network => network_message(error), ErrorCategory::Tls => match error { DriverError::Tls { failure, detail } => tls_message(*failure, detail), - other => detail_sentence(crate::i18n::gettext("The secure connection failed."), other), + _ => crate::i18n::gettext("The secure connection failed."), }, ErrorCategory::Authentication => authentication_message(error), ErrorCategory::ReadOnly => read_only_message(error), ErrorCategory::Server => match error { DriverError::Server(diagnostics) => server_message(diagnostics), - other => detail_sentence(crate::i18n::gettext("The server refused the request."), other), + _ => crate::i18n::gettext("The server refused the request."), }, ErrorCategory::Interrupted => crate::i18n::gettext("Stopped."), ErrorCategory::Timeout => timeout_message(error), @@ -39,19 +41,11 @@ pub fn driver_message(error: &DriverError) -> String { } } -/// A category sentence followed by what the error itself says, for a -/// variant this build has no sentence of its own for. -fn detail_sentence(sentence: String, error: &DriverError) -> String { - format!("{sentence} {error}") -} - fn configuration_message(error: &DriverError) -> String { match error { DriverError::Config(config) => config_message(config), - DriverError::Unsupported { feature } => { - crate::i18n::gettext_f("This database has no {feature}.", &[("feature", feature)]) - } - other => detail_sentence(crate::i18n::gettext("The connection settings cannot be used."), other), + DriverError::Unsupported { feature } => unsupported_message(*feature), + _ => crate::i18n::gettext("The connection settings cannot be used."), } } @@ -73,11 +67,17 @@ pub fn config_message(error: &ConfigError) -> String { } ConfigError::CaFile { path, problem } => crate::i18n::gettext_f( "The CA certificate {path} cannot be used: {problem}", - &[("path", &path.display().to_string()), ("problem", &problem.to_string())], + &[ + ("path", &path.display().to_string()), + ("problem", &certificate_problem_text(problem)), + ], ), ConfigError::ClientIdentityFile { path, problem } => crate::i18n::gettext_f( "The client certificate {path} cannot be used: {problem}", - &[("path", &path.display().to_string()), ("problem", &problem.to_string())], + &[ + ("path", &path.display().to_string()), + ("problem", &certificate_problem_text(problem)), + ], ), ConfigError::InvalidServerName => { crate::i18n::gettext("The TLS server name is not a valid host name or IP address.") @@ -105,7 +105,7 @@ fn network_message(error: &DriverError) -> String { "Could not reach {host}:{port}.", &[("host", endpoint.host()), ("port", &endpoint.port().to_string())], ), - other => detail_sentence(crate::i18n::gettext("The server could not be reached."), other), + _ => crate::i18n::gettext("The server could not be reached."), } } @@ -182,7 +182,7 @@ fn timeout_message(error: &DriverError) -> String { server_cancelled, } => (*phase, *server_cancelled), DriverError::Transport(TransportError::Timeout { phase }) => (*phase, false), - other => return detail_sentence(crate::i18n::gettext("The server took too long to answer."), other), + _ => return crate::i18n::gettext("The server took too long to answer."), }; match phase { TimeoutPhase::Connect => crate::i18n::gettext("The server did not answer the connection in time."), @@ -253,27 +253,25 @@ fn file_message(error: &DriverError) -> String { "{path} is not a database file.", &[("path", &path.display().to_string())], ), - DriverError::FileIo { path, kind } => crate::i18n::gettext_f( - "Could not write {path}: {reason}", - &[("path", &path.display().to_string()), ("reason", &kind.to_string())], - ), - other => detail_sentence(crate::i18n::gettext("The database file cannot be used."), other), + DriverError::FileIo { path, kind } => file_io_message(path, *kind), + _ => crate::i18n::gettext("The database file cannot be used."), } } +/// The detail of an internal error is the driver's or its client +/// library's English, about our side rather than anything the user can +/// change, so it goes to the log at the wire-detail level instead. fn internal_message(error: &DriverError) -> String { + tracing::trace!(%error, "shown as an internal driver error"); match error { DriverError::GuardUnsupported { .. } => crate::i18n::gettext( "This database does not report how many rows a statement changed, so the save cannot be checked. Use a table with a primary key.", ), - DriverError::Decode { column, detail } => crate::i18n::gettext_f( - "Could not read column {column}: {detail}", - &[("column", column), ("detail", detail)], - ), - DriverError::Protocol(detail) => crate::i18n::gettext_f( - "The driver could not read the server's reply: {detail}", - &[("detail", detail)], + DriverError::Decode { column, .. } => crate::i18n::gettext_f( + "The driver could not read a value in column {column}.", + &[("column", column)], ), - other => detail_sentence(crate::i18n::gettext("The driver hit an internal error."), other), + DriverError::Protocol(_) => crate::i18n::gettext("The driver could not read the server's reply."), + _ => crate::i18n::gettext("The driver hit an internal error."), } } diff --git a/linux/crates/app/src/ui/error_text/edit.rs b/linux/crates/app/src/ui/error_text/edit.rs index 542cebbc40..9437e0f216 100644 --- a/linux/crates/app/src/ui/error_text/edit.rs +++ b/linux/crates/app/src/ui/error_text/edit.rs @@ -1,5 +1,6 @@ use tablepro_core::column::IntegerKind; use tablepro_core::edit::{EditParseError, ReadOnlyReason}; +use tablepro_core::value::JsonTextError; use crate::i18n::{gettext, gettext_f, ngettext_f}; @@ -33,7 +34,11 @@ pub fn edit_parse_message(error: &EditParseError) -> String { ), EditParseError::InvalidBoolean => gettext("Enter true or false"), EditParseError::InvalidUuid => gettext("Enter a UUID"), - EditParseError::InvalidJson(detail) => gettext_f("Enter valid JSON: {detail}", &[("detail", detail)]), + EditParseError::InvalidJson(JsonTextError::Incomplete) => gettext("Enter valid JSON: it ends too early"), + EditParseError::InvalidJson(JsonTextError::Invalid { line, column }) => gettext_f( + "Enter valid JSON: check line {line}, column {column}", + &[("line", &line.to_string()), ("column", &column.to_string())], + ), EditParseError::InvalidDate => gettext("Enter a date as YYYY-MM-DD"), EditParseError::InvalidTime => gettext("Enter a time as HH:MM:SS"), EditParseError::TimeOutOfRange => gettext("This time is outside the range the column can hold"), @@ -84,7 +89,7 @@ mod tests { use tablepro_core::column::{IntegerKind, IntegerWidth}; use tablepro_core::edit::{EditParseError, ReadOnlyReason}; - use tablepro_core::value::DecimalParseError; + use tablepro_core::value::{DecimalParseError, JsonTextError}; use super::{edit_parse_message, integer_range}; @@ -104,7 +109,8 @@ mod tests { EditParseError::ScaleExceeded { scale: 2 }, EditParseError::InvalidBoolean, EditParseError::InvalidUuid, - EditParseError::InvalidJson("expected value at line 1 column 1".to_owned()), + EditParseError::InvalidJson(JsonTextError::Incomplete), + EditParseError::InvalidJson(JsonTextError::Invalid { line: 2, column: 7 }), EditParseError::InvalidDate, EditParseError::InvalidTime, EditParseError::TimeOutOfRange, @@ -123,6 +129,7 @@ mod tests { let distinct: HashSet<&String> = messages.iter().collect(); assert_eq!(distinct.len(), messages.len(), "{messages:?}"); assert_eq!(messages[2], "Enter a whole number from -32768 to 32767"); + assert_eq!(messages[10], "Enter valid JSON: check line 2, column 7"); } #[test] diff --git a/linux/crates/app/src/ui/error_text/files.rs b/linux/crates/app/src/ui/error_text/files.rs new file mode 100644 index 0000000000..74bd45831a --- /dev/null +++ b/linux/crates/app/src/ui/error_text/files.rs @@ -0,0 +1,110 @@ +use std::io::ErrorKind; +use std::path::Path; + +use tablepro_core::CertificateFileProblem; + +use crate::i18n::{gettext, gettext_f, ngettext_f}; + +/// A database file the file system refused, for a reason other than +/// permission or a missing file, which have their own variants. +pub(super) fn file_io_message(path: &Path, kind: ErrorKind) -> String { + let path = path.display().to_string(); + let args = [("path", path.as_str())]; + match kind { + ErrorKind::StorageFull | ErrorKind::QuotaExceeded => { + gettext_f("Could not write {path}: the disk is full.", &args) + } + ErrorKind::ReadOnlyFilesystem => gettext_f("Could not write {path}: the disk is read-only.", &args), + ErrorKind::FileTooLarge => gettext_f("Could not write {path}: the file is too large.", &args), + _ => gettext_f("Could not write {path}.", &args), + } +} + +/// What is wrong with a CA, certificate or key file, as the part after +/// "cannot be used:". +pub(super) fn certificate_problem_text(problem: &CertificateFileProblem) -> String { + match problem { + CertificateFileProblem::Unreadable { + kind: ErrorKind::NotFound, + } => gettext("the file does not exist"), + CertificateFileProblem::Unreadable { + kind: ErrorKind::PermissionDenied, + } => gettext("TablePro is not allowed to read it"), + CertificateFileProblem::Unreadable { .. } => gettext("the file could not be read"), + CertificateFileProblem::NotRegularFile => gettext("it is not a plain file"), + CertificateFileProblem::TooLarge { limit_bytes } => { + gettext_f("it is larger than {size}", &[("size", &size_text(*limit_bytes))]) + } + CertificateFileProblem::NoCertificates => gettext("it holds no certificates"), + CertificateFileProblem::InvalidCertificate => gettext("a certificate in it is not valid"), + CertificateFileProblem::MultipleCertificates { count } => ngettext_f( + "it holds {count} certificate, and only one is allowed", + "it holds {count} certificates, and only one is allowed", + u32::try_from(*count).unwrap_or(u32::MAX), + &[("count", &count.to_string())], + ), + CertificateFileProblem::NoPrivateKey => gettext("it holds no private key"), + CertificateFileProblem::InvalidPrivateKey => gettext("its private key is not valid"), + } +} + +fn size_text(bytes: usize) -> String { + gtk4::glib::format_size(u64::try_from(bytes).unwrap_or(u64::MAX)).to_string() +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn a_refused_write_names_the_file_and_the_reason_in_the_apps_words() { + let path = Path::new("/srv/app.db"); + + let full = file_io_message(path, ErrorKind::StorageFull); + let read_only = file_io_message(path, ErrorKind::ReadOnlyFilesystem); + let other = file_io_message(path, ErrorKind::Interrupted); + + assert_eq!(full, "Could not write /srv/app.db: the disk is full."); + assert_eq!(read_only, "Could not write /srv/app.db: the disk is read-only."); + assert_eq!(other, "Could not write /srv/app.db."); + assert!(!full.contains(&ErrorKind::StorageFull.to_string()), "{full}"); + } + + #[test] + fn every_certificate_problem_has_its_own_words() { + let problems = [ + CertificateFileProblem::Unreadable { + kind: ErrorKind::NotFound, + }, + CertificateFileProblem::Unreadable { + kind: ErrorKind::PermissionDenied, + }, + CertificateFileProblem::Unreadable { + kind: ErrorKind::Interrupted, + }, + CertificateFileProblem::NotRegularFile, + CertificateFileProblem::TooLarge { + limit_bytes: 1024 * 1024, + }, + CertificateFileProblem::NoCertificates, + CertificateFileProblem::InvalidCertificate, + CertificateFileProblem::MultipleCertificates { count: 2 }, + CertificateFileProblem::NoPrivateKey, + CertificateFileProblem::InvalidPrivateKey, + ]; + + let texts: Vec = problems.iter().map(certificate_problem_text).collect(); + + for (problem, text) in problems.iter().zip(&texts) { + assert_ne!(text, &problem.to_string(), "{problem:?} shows its log text"); + } + assert_eq!(texts.iter().collect::>().len(), problems.len(), "{texts:?}"); + assert_eq!( + texts[4], + format!("it is larger than {}", gtk4::glib::format_size(1024 * 1024)) + ); + assert_eq!(texts[7], "it holds 2 certificates, and only one is allowed"); + } +} diff --git a/linux/crates/app/src/ui/error_text/mod.rs b/linux/crates/app/src/ui/error_text/mod.rs index 5fa70446de..55d0d65d22 100644 --- a/linux/crates/app/src/ui/error_text/mod.rs +++ b/linux/crates/app/src/ui/error_text/mod.rs @@ -8,15 +8,19 @@ mod ddl; mod driver; mod edit; +mod files; mod operation; mod saved_connection; mod script; mod secret; mod ssh; +mod statement; +mod syntax; #[cfg(test)] mod tests; mod tls; mod tls_remedy; +mod unsupported; pub use ddl::{build_error_text, refusal_text, save_blocker_text}; pub use driver::{config_message, driver_message}; @@ -24,4 +28,5 @@ pub use edit::edit_parse_message; pub use operation::{connect_error_message, operation_error_message, task_failure_message}; pub use saved_connection::saved_connection_error_message; pub use script::script_diagnostic_message; +pub use syntax::{expression_syntax_message, type_syntax_message}; pub use tls_remedy::{TlsRemedy, connect_remedy}; diff --git a/linux/crates/app/src/ui/error_text/operation.rs b/linux/crates/app/src/ui/error_text/operation.rs index 906e5bad70..18d29f3f29 100644 --- a/linux/crates/app/src/ui/error_text/operation.rs +++ b/linux/crates/app/src/ui/error_text/operation.rs @@ -5,8 +5,10 @@ use tablepro_session::runtime::TaskFailure; use tablepro_session::{ConnectError, OperationError}; use tablepro_storage::KeyringError; +use super::ddl::build_error_text; use super::driver::driver_message; use super::secret::secret_message; +use super::statement::{bind_reason, browse_reason, build_reason, filter_message}; /// What to tell the user about an operation that produced no value. pub fn operation_error_message(error: &OperationError) -> String { @@ -58,29 +60,23 @@ fn sql_build_message(error: &SqlBuildError) -> String { match error { SqlBuildError::Dml(bind) => crate::i18n::gettext_f( "This change cannot be saved: {reason}", - &[("reason", &bind.to_string())], + &[("reason", &bind_reason(bind))], ), SqlBuildError::Browse(bind) => crate::i18n::gettext_f( "This filter cannot be applied: {reason}", - &[("reason", &bind.to_string())], - ), - SqlBuildError::Ddl(reason) => crate::i18n::gettext_f( - "This structure change cannot be written: {reason}", - &[("reason", reason)], + &[("reason", &bind_reason(bind))], ), + SqlBuildError::Ddl(error) => build_error_text(error), } } fn browse_message(error: &BrowseSqlError) -> String { match error { - BrowseSqlError::Filter(filter) => crate::i18n::gettext_f( - "This filter cannot be applied: {reason}", - &[("reason", &filter.to_string())], - ), + BrowseSqlError::Filter(filter) => filter_message(filter), BrowseSqlError::Build(build) => build_message(build), other => crate::i18n::gettext_f( "This page of the table cannot be read: {reason}", - &[("reason", &other.to_string())], + &[("reason", &browse_reason(other))], ), } } @@ -96,7 +92,7 @@ fn build_message(error: &BuildSqlError) -> String { ), other => crate::i18n::gettext_f( "This change cannot be saved: {reason}", - &[("reason", &other.to_string())], + &[("reason", &build_reason(other))], ), } } diff --git a/linux/crates/app/src/ui/error_text/secret.rs b/linux/crates/app/src/ui/error_text/secret.rs index 416528a433..be75b0b9f9 100644 --- a/linux/crates/app/src/ui/error_text/secret.rs +++ b/linux/crates/app/src/ui/error_text/secret.rs @@ -17,8 +17,16 @@ pub(super) fn secret_message(error: &tablepro_core::credentials::SecretError) -> SecretError::InvalidEncoding => { crate::i18n::gettext("The stored password is not readable text. Enter it again to replace it.") } - SecretError::Backend { detail } => { - crate::i18n::gettext_f("The keyring reported: {detail}", &[("detail", detail)]) + SecretError::Backend { + detail, + service_message, + } => { + // oo7's text is never shown, so the log is the only place it goes. + tracing::warn!(%detail, "the keyring failed"); + match service_message { + Some(message) => crate::i18n::gettext_f("The keyring reported: {detail}", &[("detail", message)]), + None => crate::i18n::gettext("The keyring failed without giving a reason."), + } } } } diff --git a/linux/crates/app/src/ui/error_text/statement.rs b/linux/crates/app/src/ui/error_text/statement.rs new file mode 100644 index 0000000000..696d8b9c06 --- /dev/null +++ b/linux/crates/app/src/ui/error_text/statement.rs @@ -0,0 +1,166 @@ +//! Why a row change, a filter or a page could not be written as SQL. +//! The variants carry the dialect's own names for types and values, +//! which are for the log, so none of them reaches the sentence. + +use tablepro_core::BuildFilterError; +use tablepro_core::browse::BrowseSqlError; +use tablepro_core::dialect::BindError; +use tablepro_core::dml::BuildSqlError; + +use super::edit::edit_parse_message; +use super::syntax::expression_syntax_message; +use crate::i18n::{gettext, gettext_f}; + +/// Why a value could not be sent, as the part after "cannot be saved:" +/// or "cannot be applied:". +pub(super) fn bind_reason(error: &BindError) -> String { + match error { + BindError::ValueTypeMismatch { .. } => gettext("a value does not match its column's type"), + BindError::MissingCatalogType { column } => gettext_f( + "the database did not report the type of column {column}", + &[("column", column)], + ), + BindError::Undecodable { .. } => { + gettext("a value could not be read from the server, so it cannot be written back") + } + BindError::Unrepresentable { .. } => gettext("this database cannot take one of the values"), + } +} + +/// Why the tab's filter cannot be written, as a whole sentence. +pub(super) fn filter_message(error: &BuildFilterError) -> String { + let refused = |reason: String| gettext_f("This filter cannot be applied: {reason}", &[("reason", &reason)]); + match error { + BuildFilterError::UnknownColumn(column) => refused(gettext_f( + "it names column {column}, which the table does not have", + &[("column", column)], + )), + BuildFilterError::MissingValue(_) => refused(gettext("a rule has no value")), + BuildFilterError::BetweenMissingBound => refused(gettext("a between rule needs both of its values")), + BuildFilterError::EmptyInList => refused(gettext("a one-of rule needs at least one value")), + BuildFilterError::WrongValueShape { .. } => refused(gettext("a rule's value does not fit its operator")), + BuildFilterError::Parse { column, source } => gettext_f( + "This filter cannot be applied. Value for {column}: {reason}", + &[("column", column), ("reason", &edit_parse_message(source))], + ), + BuildFilterError::Bind(bind) => refused(bind_reason(bind)), + BuildFilterError::OperatorNotAllowed { column, .. } => refused(gettext_f( + "column {column} does not allow one of its operators", + &[("column", column)], + )), + BuildFilterError::RawPredicate(syntax) => gettext_f( + "This SQL filter cannot be applied. {reason}", + &[("reason", &expression_syntax_message(syntax))], + ), + } +} + +/// Why a page of the table could not be read, as the part after +/// "cannot be read:". +pub(super) fn browse_reason(error: &BrowseSqlError) -> String { + match error { + BrowseSqlError::NoColumns => gettext("the table has no columns to select"), + BrowseSqlError::UnknownSortColumn(_) => gettext("the sort names a column the table does not have"), + BrowseSqlError::UnsupportedPosition { .. } => gettext("this position cannot be reached by this way of paging"), + BrowseSqlError::NullAnchor => gettext("the row to page from has an empty key"), + BrowseSqlError::UnreadableAnchor => gettext("the row to page from has no address that can be read"), + BrowseSqlError::ShapeMismatch { .. } => gettext("the page came back with other columns than were asked for"), + BrowseSqlError::Bind(bind) => bind_reason(bind), + BrowseSqlError::Filter(_) | BrowseSqlError::Build(_) => gettext("the query could not be built"), + } +} + +/// Why a row change could not be written, as the part after "cannot be +/// saved:". +pub(super) fn build_reason(error: &BuildSqlError) -> String { + match error { + BuildSqlError::UnknownKeyColumn { name } => gettext_f( + "the row key names column {column}, which the table does not have", + &[("column", name)], + ), + BuildSqlError::LengthMismatch { .. } => gettext("the row key does not have one value for each key column"), + BuildSqlError::Bind(bind) => bind_reason(bind), + BuildSqlError::NoRowIdentity + | BuildSqlError::ReadOnlyRows + | BuildSqlError::NothingToUpdate + | BuildSqlError::NoInsertValues + | BuildSqlError::UndecodableKey => gettext("the change could not be written as SQL"), + } +} + +#[cfg(test)] +mod tests { + use tablepro_core::FilterOp; + use tablepro_core::edit::EditParseError; + use tablepro_core::sql_syntax::ExpressionSyntaxError; + + use super::*; + + #[test] + fn a_bind_refusal_leaves_out_the_dialects_type_names() { + let mismatch = bind_reason(&BindError::ValueTypeMismatch { + expected: "Uuid", + found: "Text", + }); + let unrepresentable = bind_reason(&BindError::Unrepresentable { + expected: "a SQLite rowid", + found: "another engine's row address", + }); + + assert!(!mismatch.contains("Uuid") && !mismatch.contains("Text"), "{mismatch}"); + assert!(!unrepresentable.contains("rowid"), "{unrepresentable}"); + } + + #[test] + fn a_filter_value_the_column_cannot_take_says_which_column_and_why() { + let message = filter_message(&BuildFilterError::Parse { + column: "price".to_owned(), + source: EditParseError::InvalidFloat, + }); + + assert_eq!( + message, + "This filter cannot be applied. Value for price: Enter a number" + ); + } + + #[test] + fn an_sql_filter_refusal_is_the_apps_sentence() { + let message = filter_message(&BuildFilterError::RawPredicate(ExpressionSyntaxError::Invalid)); + + assert_eq!( + message, + "This SQL filter cannot be applied. This database does not read this as one expression." + ); + } + + #[test] + fn every_filter_refusal_has_its_own_sentence() { + let errors = [ + BuildFilterError::UnknownColumn("gone".to_owned()), + BuildFilterError::MissingValue(FilterOp::Eq), + BuildFilterError::BetweenMissingBound, + BuildFilterError::EmptyInList, + BuildFilterError::WrongValueShape { op: FilterOp::In }, + BuildFilterError::Parse { + column: "id".to_owned(), + source: EditParseError::InvalidInteger, + }, + BuildFilterError::Bind(BindError::MissingCatalogType { + column: "id".to_owned(), + }), + BuildFilterError::OperatorNotAllowed { + column: "id".to_owned(), + op: FilterOp::Like, + }, + BuildFilterError::RawPredicate(ExpressionSyntaxError::Comment), + ]; + + let messages: std::collections::HashSet = errors.iter().map(filter_message).collect(); + + assert_eq!(messages.len(), errors.len(), "{messages:?}"); + for error in &errors { + assert!(!messages.contains(&error.to_string()), "{error:?} shows its log text"); + } + } +} diff --git a/linux/crates/app/src/ui/error_text/syntax.rs b/linux/crates/app/src/ui/error_text/syntax.rs new file mode 100644 index 0000000000..3962192a58 --- /dev/null +++ b/linux/crates/app/src/ui/error_text/syntax.rs @@ -0,0 +1,113 @@ +use tablepro_core::dialect::LiteralError; +use tablepro_core::sql_syntax::{ExpressionSyntaxError, TypeSyntaxError}; + +use crate::i18n::{gettext, ngettext_f}; + +/// Why the Type field's text is refused, beside the field. +pub fn type_syntax_message(error: &TypeSyntaxError) -> String { + match error { + TypeSyntaxError::Empty => gettext("Enter a type."), + TypeSyntaxError::TooLong { max } => too_long(*max), + TypeSyntaxError::Comment => gettext("Remove the comment. It would hide the rest of the statement."), + TypeSyntaxError::Invalid => gettext("This database does not read this as one type."), + TypeSyntaxError::SessionDependentBackslash => backslash(), + } +} + +/// Why a default or an SQL filter is refused. +pub fn expression_syntax_message(error: &ExpressionSyntaxError) -> String { + match error { + ExpressionSyntaxError::Empty => gettext("Enter an expression."), + ExpressionSyntaxError::TooLong { max } => too_long(*max), + ExpressionSyntaxError::Comment => gettext("Remove the comment. It would hide the rest of the statement."), + ExpressionSyntaxError::Invalid => gettext("This database does not read this as one expression."), + ExpressionSyntaxError::SessionDependentBackslash => backslash(), + } +} + +/// Why a value has no SQL spelling, for an engine without parameters. +pub fn literal_message(error: &LiteralError) -> String { + match error { + LiteralError::Unrepresentable { .. } => gettext("This database has no way to write this value in SQL."), + LiteralError::Undecodable { .. } => { + gettext("The value could not be read from the server, so it cannot be written back.") + } + } +} + +fn too_long(max: usize) -> String { + ngettext_f( + "Use at most {max} character.", + "Use at most {max} characters.", + u32::try_from(max).unwrap_or(u32::MAX), + &[("max", &max.to_string())], + ) +} + +fn backslash() -> String { + gettext("Remove the backslash. This database reads it differently from one session to another.") +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn each_type_refusal_says_its_own_thing_in_the_apps_words() { + let errors = [ + TypeSyntaxError::Empty, + TypeSyntaxError::TooLong { max: 512 }, + TypeSyntaxError::Comment, + TypeSyntaxError::Invalid, + TypeSyntaxError::SessionDependentBackslash, + ]; + + let messages: Vec = errors.iter().map(type_syntax_message).collect(); + + for (error, message) in errors.iter().zip(&messages) { + assert_ne!(message, &error.to_string(), "{error:?} shows its log text"); + } + assert_eq!( + messages.iter().collect::>().len(), + errors.len(), + "{messages:?}" + ); + assert_eq!(messages[1], "Use at most 512 characters."); + } + + #[test] + fn each_expression_refusal_says_its_own_thing_in_the_apps_words() { + let errors = [ + ExpressionSyntaxError::Empty, + ExpressionSyntaxError::TooLong { max: 4096 }, + ExpressionSyntaxError::Comment, + ExpressionSyntaxError::Invalid, + ExpressionSyntaxError::SessionDependentBackslash, + ]; + + let messages: Vec = errors.iter().map(expression_syntax_message).collect(); + + for (error, message) in errors.iter().zip(&messages) { + assert_ne!(message, &error.to_string(), "{error:?} shows its log text"); + } + assert_eq!( + messages.iter().collect::>().len(), + errors.len(), + "{messages:?}" + ); + } + + #[test] + fn a_literal_refusal_leaves_out_the_value_type_and_the_reason() { + let unrepresentable = literal_message(&LiteralError::Unrepresentable { found: "Bytes" }); + let undecodable = literal_message(&LiteralError::Undecodable { + reason: "InvalidEncoding is not decodable here".to_owned(), + }); + + assert!(!unrepresentable.contains("Bytes"), "{unrepresentable}"); + assert!(!undecodable.contains("InvalidEncoding"), "{undecodable}"); + assert_ne!(unrepresentable, undecodable); + } +} diff --git a/linux/crates/app/src/ui/error_text/tests.rs b/linux/crates/app/src/ui/error_text/tests.rs index dc504c4503..90c19f2898 100644 --- a/linux/crates/app/src/ui/error_text/tests.rs +++ b/linux/crates/app/src/ui/error_text/tests.rs @@ -383,6 +383,11 @@ fn every_secret_error_has_its_own_message() { SecretError::InvalidEncoding, SecretError::Backend { detail: "boom".to_owned(), + service_message: Some("No such item".to_owned()), + }, + SecretError::Backend { + detail: "boom".to_owned(), + service_message: None, }, ]; @@ -401,12 +406,32 @@ fn the_portal_message_says_what_to_check() { } #[test] -fn a_backend_error_keeps_its_detail() { - let message = secret_message(&SecretError::Backend { - detail: "org.freedesktop.Secret failed".to_owned(), +fn a_backend_error_shows_the_services_words_and_never_oo7s() { + let answered = secret_message(&SecretError::Backend { + detail: "DBus error service error org.freedesktop.Secret.Error.NoSuchObject".to_owned(), + service_message: Some("No such item".to_owned()), + }); + let silent = secret_message(&SecretError::Backend { + detail: "DBus error Item/Collection was deleted, can no longer be used".to_owned(), + service_message: None, + }); + + assert_eq!(answered, "The keyring reported: No such item"); + assert!(!answered.contains("DBus error"), "{answered}"); + assert!(!silent.contains("DBus error"), "{silent}"); +} + +#[test] +fn oo7s_words_for_a_backend_error_go_to_the_log() { + let lines = crate::test_support::LogLines::default(); + let _logging = tracing::subscriber::set_default(lines.subscriber(tracing::Level::WARN)); + + secret_message(&SecretError::Backend { + detail: "DBus error Item/Collection was deleted, can no longer be used".to_owned(), + service_message: None, }); - assert!(message.contains("org.freedesktop.Secret failed"), "{message}"); + assert!(lines.text().contains("Item/Collection was deleted"), "{}", lines.text()); } #[test] diff --git a/linux/crates/app/src/ui/error_text/unsupported.rs b/linux/crates/app/src/ui/error_text/unsupported.rs new file mode 100644 index 0000000000..2995dc8469 --- /dev/null +++ b/linux/crates/app/src/ui/error_text/unsupported.rs @@ -0,0 +1,83 @@ +use tablepro_core::UnsupportedFeature; + +use crate::i18n::{gettext, gettext_f}; + +/// What the engine cannot do. Every feature has its own branch, so a +/// new one fails to compile here rather than reaching the user in +/// English. +pub(super) fn unsupported_message(feature: UnsupportedFeature) -> String { + match feature { + UnsupportedFeature::DatabaseFile => { + gettext("This database is reached over the network. It cannot open a database file.") + } + UnsupportedFeature::NetworkConnection => { + gettext("This database opens a file. It cannot connect to a server over the network.") + } + UnsupportedFeature::ForeignKeySuspension => { + gettext("This database cannot turn foreign key checks off for a save.") + } + UnsupportedFeature::MinimumVersion { required } => { + gettext_f("This needs {version} or later.", &[("version", required)]) + } + UnsupportedFeature::CopyFromStdinInEditor => gettext("COPY FROM STDIN cannot run in the editor."), + UnsupportedFeature::CopyToStdoutInEditor => gettext("COPY TO STDOUT cannot run in the editor."), + UnsupportedFeature::TimestampInRepeatedHour => gettext( + "This time falls in the hour the session's time zone repeats, so the database cannot store it as one instant.", + ), + UnsupportedFeature::TimeOutsideOneDay => gettext("This database has no times outside one day."), + UnsupportedFeature::TimeWithOffset => gettext("This database has no times with an offset."), + UnsupportedFeature::Interval => gettext("This database has no intervals."), + UnsupportedFeature::BitString => gettext("This database has no bit strings."), + UnsupportedFeature::NumberWiderThan { digits } => gettext_f( + "This database has no numbers wider than {digits} digits.", + &[("digits", &digits.to_string())], + ), + UnsupportedFeature::DecimalOutOfRange { digits } => gettext_f( + "This database has no NaN, no infinity and no decimals wider than {digits} digits.", + &[("digits", &digits.to_string())], + ), + UnsupportedFeature::DateOutsideYears { first, last } => gettext_f( + "This database has no dates outside the years {first} to {last}.", + &[("first", &first.to_string()), ("last", &last.to_string())], + ), + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use super::*; + + #[test] + fn every_feature_has_its_own_sentence() { + let features = [ + UnsupportedFeature::DatabaseFile, + UnsupportedFeature::NetworkConnection, + UnsupportedFeature::ForeignKeySuspension, + UnsupportedFeature::MinimumVersion { + required: "SQLite 3.37.0", + }, + UnsupportedFeature::CopyFromStdinInEditor, + UnsupportedFeature::CopyToStdoutInEditor, + UnsupportedFeature::TimestampInRepeatedHour, + UnsupportedFeature::TimeOutsideOneDay, + UnsupportedFeature::TimeWithOffset, + UnsupportedFeature::Interval, + UnsupportedFeature::BitString, + UnsupportedFeature::NumberWiderThan { digits: 38 }, + UnsupportedFeature::DecimalOutOfRange { digits: 38 }, + UnsupportedFeature::DateOutsideYears { first: 1, last: 9999 }, + ]; + + let messages: Vec = features.iter().map(|feature| unsupported_message(*feature)).collect(); + + assert_eq!( + messages.iter().collect::>().len(), + features.len(), + "{messages:?}" + ); + assert_eq!(messages[3], "This needs SQLite 3.37.0 or later."); + assert_eq!(messages[13], "This database has no dates outside the years 1 to 9999."); + } +} diff --git a/linux/crates/app/src/ui/import_dialog.rs b/linux/crates/app/src/ui/import_dialog.rs index c8f7ca31b2..171a383557 100644 --- a/linux/crates/app/src/ui/import_dialog.rs +++ b/linux/crates/app/src/ui/import_dialog.rs @@ -265,7 +265,13 @@ impl ImportCsvDialog { } let choices: Vec = std::iter::once(crate::i18n::gettext("Skip")) - .chain(self.sheet.headers.iter().cloned()) + .chain( + self.sheet + .headers + .iter() + .enumerate() + .map(|(index, header)| field_label(index, header)), + ) .collect(); let choice_refs: Vec<&str> = choices.iter().map(String::as_str).collect(); // One list for every row: each row keeps its own selection, and @@ -369,9 +375,11 @@ pub(crate) fn import_error_text(error: &ImportError) -> String { "The file isn't UTF-8 text from byte {offset} on. Save it as UTF-8 and import it again.", &[("offset", &offset.to_string())], ), - ImportError::Malformed(detail) => { - crate::i18n::gettext_f("The file couldn't be read as CSV: {error}", &[("error", detail)]) - } + ImportError::Malformed { line: Some(line) } => crate::i18n::gettext_f( + "The file couldn't be read as CSV at line {line}.", + &[("line", &line.to_string())], + ), + ImportError::Malformed { line: None } => crate::i18n::gettext("The file couldn't be read as CSV."), ImportError::NoColumns => crate::i18n::gettext("The file has no columns."), ImportError::TooManyFields { line, limit } => crate::i18n::gettext_f( "Line {line} has more than {limit} fields, more than any table holds. Check the delimiter.", @@ -380,6 +388,15 @@ pub(crate) fn import_error_text(error: &ImportError) -> String { } } +/// A field as the mapping offers it: its header, or its position when +/// the file names none. +fn field_label(index: usize, header: &str) -> String { + match header.trim().is_empty() { + true => crate::i18n::gettext_f("Column {n}", &[("n", &(index + 1).to_string())]), + false => header.to_owned(), + } +} + /// The first few values of one field, as one line. fn sample_text(sheet: &CsvSheet, field: usize, options: &CsvImportOptions) -> String { let values: Vec = sheet @@ -452,6 +469,25 @@ mod tests { assert_eq!(qualified(Some("public"), "orders"), "public.orders"); assert_eq!(qualified(None, "orders"), "orders"); } + + #[test] + fn a_file_the_csv_reader_refused_says_where_in_the_apps_words() { + assert_eq!( + import_error_text(&ImportError::Malformed { line: Some(12) }), + "The file couldn't be read as CSV at line 12." + ); + assert_eq!( + import_error_text(&ImportError::Malformed { line: None }), + "The file couldn't be read as CSV." + ); + } + + #[test] + fn a_field_with_no_name_is_offered_by_its_position() { + assert_eq!(field_label(0, ""), "Column 1"); + assert_eq!(field_label(2, " "), "Column 3"); + assert_eq!(field_label(1, "city"), "city"); + } } #[cfg(test)] @@ -525,9 +561,20 @@ mod dialog_tests { dialog.model().header_row.set_active(false); crate::test_support::drain_main_context(); - // "Column 1" and "Column 2" match no table column, so every row - // falls back to Skip and the header line reads as data. + // Fields with no name match no table column, so every row falls + // back to Skip and the header line reads as data. assert_eq!(dialog.model().mapping(), vec![None, None, None]); + let offered: Vec = (0..3) + .map(|index| { + dialog.model().mapping_rows.borrow()[0] + .model() + .and_downcast::() + .and_then(|list| list.string(index)) + .map(|text| text.to_string()) + .unwrap_or_default() + }) + .collect(); + assert_eq!(offered, vec!["Skip", "Column 1", "Column 2"]); } #[gtk4::test] diff --git a/linux/crates/app/src/ui/session_window/connect_flow.rs b/linux/crates/app/src/ui/session_window/connect_flow.rs index 7bd80d56b5..98347038aa 100644 --- a/linux/crates/app/src/ui/session_window/connect_flow.rs +++ b/linux/crates/app/src/ui/session_window/connect_flow.rs @@ -110,6 +110,9 @@ pub(super) async fn connect_saved( let pending = unless_cancelled(pending, &cancel).await?; let kept: Vec = remember.iter().map(|save| save.kind).collect(); let refused = credentials.remember(remember).await; + for (kind, error) in &refused { + tracing::warn!(?kind, %error, "the keyring did not keep a typed secret"); + } let session_only = typed .into_iter() .filter(|kind| !kept.contains(kind) || refused.iter().any(|(failed, _)| failed == kind)) @@ -516,6 +519,8 @@ mod tests { async fn a_password_the_keyring_refused_to_keep_is_the_sessions_own() { use tablepro_core::credentials::PromptReply; + let lines = crate::test_support::LogLines::default(); + let _logging = tracing::subscriber::set_default(lines.subscriber(tracing::Level::WARN)); let saved = connection(Uuid::new_v4()); let vault = Arc::new(FakeSecretVault::new()); vault.fail_store(SecretKind::DatabasePassword, SecretError::Locked); @@ -541,6 +546,11 @@ mod tests { assert_eq!(login.refused.len(), 1); assert_eq!(login.session_only, [SecretKind::DatabasePassword]); + let logged = lines.text(); + assert!( + logged.contains("the keyring did not keep a typed secret") && logged.contains("DatabasePassword"), + "{logged}" + ); let _closed = login.pending.discard().await; } diff --git a/linux/crates/app/src/ui/session_window/secret_retry.rs b/linux/crates/app/src/ui/session_window/secret_retry.rs index 549f29b4ce..3035bbb5a9 100644 --- a/linux/crates/app/src/ui/session_window/secret_retry.rs +++ b/linux/crates/app/src/ui/session_window/secret_retry.rs @@ -179,6 +179,7 @@ mod tests { kind: SecretKind::SshPassword, error: tablepro_core::credentials::SecretError::Backend { detail: "broken".to_owned(), + service_message: None, }, }); diff --git a/linux/crates/app/src/ui/structure_tab/columns.rs b/linux/crates/app/src/ui/structure_tab/columns.rs index 23b8e09c2c..8d7b0c3140 100644 --- a/linux/crates/app/src/ui/structure_tab/columns.rs +++ b/linux/crates/app/src/ui/structure_tab/columns.rs @@ -86,7 +86,10 @@ fn type_error(dialect: Option<&dyn SqlDialect>, column: &DraftColumn, text: &str return None; } let dialect = dialect?; - dialect.parse_type(text).err().map(|error| error.to_string()) + dialect + .parse_type(text) + .err() + .map(|error| crate::ui::error_text::type_syntax_message(&error)) } /// Mark the Default field with the error the column holds, now and each diff --git a/linux/crates/app/src/ui/structure_tab/context.rs b/linux/crates/app/src/ui/structure_tab/context.rs index fc311ff0d0..6fefa96435 100644 --- a/linux/crates/app/src/ui/structure_tab/context.rs +++ b/linux/crates/app/src/ui/structure_tab/context.rs @@ -143,6 +143,7 @@ impl StructureContext { let column_type = self .default_column_type(column) .unwrap_or_else(|| ColumnType::untyped_text(column.column_type.text())); - parse_default(dialect, text, &column_type).map_err(|error| error.to_string()) + parse_default(dialect, text, &column_type) + .map_err(|error| crate::ui::error_text::expression_syntax_message(&error)) } } diff --git a/linux/crates/app/src/ui/workspace/import.rs b/linux/crates/app/src/ui/workspace/import.rs index fae483fca4..b63c5d4190 100644 --- a/linux/crates/app/src/ui/workspace/import.rs +++ b/linux/crates/app/src/ui/workspace/import.rs @@ -447,7 +447,7 @@ fn import_rows(import: &CsvImport) -> Result>, String> { &[ ("line", &error.line.to_string()), ("column", &error.column), - ("reason", &error.reason), + ("reason", &crate::ui::error_text::edit_parse_message(&error.reason)), ("text", &error.text), ], ) @@ -667,8 +667,10 @@ mod tests { let message = import_rows(&bad).unwrap_err(); - assert!(message.contains('2'), "{message}"); - assert!(message.contains("London"), "{message}"); + assert_eq!( + message, + "Line 2, column city: Enter a whole number (the value was “London”)" + ); } fn read(path: &std::path::Path) -> Result, String> { diff --git a/linux/crates/app/src/workspace/save_outcome.rs b/linux/crates/app/src/workspace/save_outcome.rs index f2b4c71743..6c7375b66d 100644 --- a/linux/crates/app/src/workspace/save_outcome.rs +++ b/linux/crates/app/src/workspace/save_outcome.rs @@ -55,8 +55,8 @@ impl SaveOutcome { #[cfg(test)] mod tests { + use tablepro_core::ddl::BuildDdlError; use tablepro_core::dml::BuildSqlError; - use tablepro_core::statement::SqlBuildError; use tablepro_core::{CommitToken, LossPhase, ReadOnlyRefusal, StatementOutcome, TimeoutPhase, TransportError}; use tablepro_session::runtime::TaskPanic; @@ -157,8 +157,8 @@ mod tests { assert!(rolled_back(&driver(refused.clone())), "{refused:?}"); } assert_eq!( - SaveOutcome::classify(Err(OperationError::Sql(SqlBuildError::Ddl("no".to_owned())))), - SaveOutcome::RolledBack(RollbackCause::Sql(SqlBuildError::Ddl("no".to_owned()))) + SaveOutcome::classify(Err(OperationError::Sql(BuildDdlError::NotAsReviewed.into()))), + SaveOutcome::RolledBack(RollbackCause::Sql(BuildDdlError::NotAsReviewed.into())) ); assert_eq!( SaveOutcome::classify(Err(OperationError::Build(BuildSqlError::NoRowIdentity))), diff --git a/linux/crates/core/src/certificate_file_problem.rs b/linux/crates/core/src/certificate_file_problem.rs index 417968ee8d..94cff92159 100644 --- a/linux/crates/core/src/certificate_file_problem.rs +++ b/linux/crates/core/src/certificate_file_problem.rs @@ -1,9 +1,12 @@ use thiserror::Error; +/// What is wrong with a CA, certificate or key file. The PEM and +/// certificate parsers report in English only, so their messages are not +/// kept; the I/O error's kind is. #[derive(Debug, Clone, PartialEq, Eq, Error)] pub enum CertificateFileProblem { - #[error("the file could not be read: {detail}")] - Unreadable { detail: String }, + #[error("the file could not be read: {kind}")] + Unreadable { kind: std::io::ErrorKind }, #[error("it is not a regular file")] NotRegularFile, @@ -14,8 +17,8 @@ pub enum CertificateFileProblem { #[error("the file contains no certificates")] NoCertificates, - #[error("a certificate is invalid: {detail}")] - InvalidCertificate { detail: String }, + #[error("a certificate is invalid")] + InvalidCertificate, #[error("the file contains {count} certificates, but only one is allowed")] MultipleCertificates { count: usize }, @@ -23,6 +26,6 @@ pub enum CertificateFileProblem { #[error("the file contains no private key")] NoPrivateKey, - #[error("the private key is invalid: {detail}")] - InvalidPrivateKey { detail: String }, + #[error("the private key is invalid")] + InvalidPrivateKey, } diff --git a/linux/crates/core/src/credentials/secret_error.rs b/linux/crates/core/src/credentials/secret_error.rs index f3eb8f3d9e..c230b54a37 100644 --- a/linux/crates/core/src/credentials/secret_error.rs +++ b/linux/crates/core/src/credentials/secret_error.rs @@ -17,8 +17,14 @@ pub enum SecretError { UnlockDismissed, #[error("the stored secret is not valid UTF-8")] InvalidEncoding, + /// A failure the app has no step for. `detail` is oo7's English text + /// for the log; `service_message` is what the Secret Service itself + /// answered, when the failure is its reply. #[error("the secret service reported: {detail}")] - Backend { detail: String }, + Backend { + detail: String, + service_message: Option, + }, } impl SecretError { diff --git a/linux/crates/core/src/edit/edit_parse_error.rs b/linux/crates/core/src/edit/edit_parse_error.rs index f7059cf36a..ea1c45a6b3 100644 --- a/linux/crates/core/src/edit/edit_parse_error.rs +++ b/linux/crates/core/src/edit/edit_parse_error.rs @@ -1,7 +1,7 @@ use thiserror::Error; use crate::column::IntegerKind; -use crate::value::DecimalParseError; +use crate::value::{DecimalParseError, JsonTextError}; /// Why the text in a cell could not become a value of the column's /// type. @@ -30,7 +30,7 @@ pub enum EditParseError { #[error("enter a UUID")] InvalidUuid, #[error("enter valid JSON: {0}")] - InvalidJson(String), + InvalidJson(#[source] JsonTextError), #[error("enter a date as YYYY-MM-DD")] InvalidDate, #[error("enter a time as HH:MM:SS")] diff --git a/linux/crates/core/src/edit/parse_literal_text.rs b/linux/crates/core/src/edit/parse_literal_text.rs index 7f0270e820..750a2674b6 100644 --- a/linux/crates/core/src/edit/parse_literal_text.rs +++ b/linux/crates/core/src/edit/parse_literal_text.rs @@ -95,7 +95,7 @@ fn strict_value(input: &str, column: &ColumnType, purpose: Purpose) -> Result Ok(Value::Text(input.to_owned())), ColumnKind::Json { .. } => JsonText::parse(input.to_owned()) .map(Value::Json) - .map_err(|error| EditParseError::InvalidJson(error.to_string())), + .map_err(EditParseError::InvalidJson), ColumnKind::Uuid => trimmed .parse() .map(Value::Uuid) diff --git a/linux/crates/core/src/error.rs b/linux/crates/core/src/error.rs index 53242dd4f6..4422296175 100644 --- a/linux/crates/core/src/error.rs +++ b/linux/crates/core/src/error.rs @@ -11,6 +11,7 @@ use crate::server_diagnostics::ServerDiagnostics; use crate::timeout_phase::TimeoutPhase; use crate::tls_failure::TlsFailure; use crate::transport_error::TransportError; +use crate::unsupported_feature::UnsupportedFeature; use crate::write_report::StatementOutcome; /// What a driver could not do, in enough detail for the app to say @@ -134,7 +135,7 @@ pub enum DriverError { Protocol(String), #[error("this engine has no {feature}")] - Unsupported { feature: &'static str }, + Unsupported { feature: UnsupportedFeature }, } impl DriverError { diff --git a/linux/crates/core/src/filter/validate_raw_predicate.rs b/linux/crates/core/src/filter/validate_raw_predicate.rs index 9b929698d0..54fb745d99 100644 --- a/linux/crates/core/src/filter/validate_raw_predicate.rs +++ b/linux/crates/core/src/filter/validate_raw_predicate.rs @@ -27,20 +27,20 @@ mod tests { #[test] fn a_predicate_that_closes_the_where_is_refused() { - assert!(matches!( + assert_eq!( validate("1=1); DELETE FROM t; SELECT (1"), - Err(ExpressionSyntaxError::Invalid(_)) - )); + Err(ExpressionSyntaxError::Invalid) + ); } #[test] fn a_trailing_semicolon_is_refused() { - assert!(matches!(validate("a = 1;"), Err(ExpressionSyntaxError::Invalid(_)))); + assert_eq!(validate("a = 1;"), Err(ExpressionSyntaxError::Invalid)); } #[test] fn a_batch_separator_is_refused() { - assert!(matches!(validate("1=1 GO"), Err(ExpressionSyntaxError::Invalid(_)))); + assert_eq!(validate("1=1 GO"), Err(ExpressionSyntaxError::Invalid)); } #[test] diff --git a/linux/crates/core/src/foreign_key_mode.rs b/linux/crates/core/src/foreign_key_mode.rs index dfb47d0f89..7d7e9cab9c 100644 --- a/linux/crates/core/src/foreign_key_mode.rs +++ b/linux/crates/core/src/foreign_key_mode.rs @@ -1,5 +1,6 @@ use crate::error::DriverError; use crate::foreign_key_check::ForeignKeyCheck; +use crate::unsupported_feature::UnsupportedFeature; /// Whether foreign keys stay on while a batch runs. /// @@ -30,7 +31,7 @@ impl ForeignKeyMode { match self { Self::Enforced => Ok(()), Self::SuspendedThenVerified { .. } => Err(DriverError::Unsupported { - feature: "foreign key suspension", + feature: UnsupportedFeature::ForeignKeySuspension, }), } } @@ -53,7 +54,7 @@ mod tests { matches!( refusal, Err(DriverError::Unsupported { - feature: "foreign key suspension" + feature: UnsupportedFeature::ForeignKeySuspension }) ), "got {refusal:?}" diff --git a/linux/crates/core/src/import/csv_import.rs b/linux/crates/core/src/import/csv_import.rs index 1ae1fa7101..cb20dff450 100644 --- a/linux/crates/core/src/import/csv_import.rs +++ b/linux/crates/core/src/import/csv_import.rs @@ -1,7 +1,7 @@ use thiserror::Error; use crate::column::ColumnInfo; -use crate::edit::{CellInput, parse_literal_text}; +use crate::edit::{CellInput, EditParseError, parse_literal_text}; use crate::export::CsvDelimiter; use super::ImportColumn; @@ -22,8 +22,10 @@ pub enum ImportError { #[error("the file is not valid UTF-8 at byte {offset}")] NotUtf8 { offset: usize }, - #[error("the file could not be read as CSV: {0}")] - Malformed(String), + /// The csv crate's message is English only, so only where it + /// stopped is kept. + #[error("the file could not be read as CSV")] + Malformed { line: Option }, #[error("the file has no columns in it")] NoColumns, @@ -41,7 +43,7 @@ pub struct CsvRowError { pub line: usize, pub column: String, pub text: String, - pub reason: String, + pub reason: EditParseError, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -68,8 +70,8 @@ impl Default for CsvImportOptions { /// A CSV file as the import reads it. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct CsvSheet { - /// One name per field, from the header record or made up from the - /// position when there is none. + /// One name per field, from the header record. Empty when the file + /// has no header, for the app to name the field by its position. pub headers: Vec, pub rows: Vec>, /// Rows past `MAX_PREVIEW_ROWS` when the read was a preview, so the @@ -98,9 +100,7 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) let headers: Vec = match options.has_header { true => { - let header = reader - .headers() - .map_err(|error| ImportError::Malformed(error.to_string()))?; + let header = reader.headers().map_err(malformed)?; within_width(header)?; header.iter().map(str::to_owned).collect() } @@ -110,7 +110,7 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) let mut rows: Vec> = Vec::new(); let mut truncated = false; for record in reader.records() { - let record = record.map_err(|error| ImportError::Malformed(error.to_string()))?; + let record = record.map_err(malformed)?; if let Some(limit) = limit && rows.len() >= limit { @@ -126,7 +126,7 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) return Err(ImportError::NoColumns); } let headers = match headers.is_empty() { - true => (1..=width).map(|index| format!("Column {index}")).collect(), + true => vec![String::new(); width], false => headers, }; // A short row is padded so every row can be indexed by field @@ -142,6 +142,12 @@ pub fn read_csv(bytes: &[u8], options: &CsvImportOptions, limit: Option) }) } +fn malformed(error: csv::Error) -> ImportError { + ImportError::Malformed { + line: error.position().map(csv::Position::line), + } +} + /// Refuse a record wider than `MAX_FIELDS` before its fields are copied. fn within_width(record: &csv::StringRecord) -> Result<(), ImportError> { match record.len() > MAX_FIELDS { @@ -203,7 +209,7 @@ pub fn row_to_cells( .collect() } -fn cell_for(text: &str, column: &ColumnInfo, options: &CsvImportOptions) -> Result { +fn cell_for(text: &str, column: &ColumnInfo, options: &CsvImportOptions) -> Result { if text == options.null_marker { // An empty string is a value in its own right for a text // column, and the marker being empty is the common case. The @@ -218,9 +224,7 @@ fn cell_for(text: &str, column: &ColumnInfo, options: &CsvImportOptions) -> Resu } return Ok(CellInput::Value(crate::value::Value::Null)); } - parse_literal_text(text, &column.column_type) - .map(CellInput::Value) - .map_err(|error| error.to_string()) + parse_literal_text(text, &column.column_type).map(CellInput::Value) } fn delimiter_byte(delimiter: CsvDelimiter) -> u8 { @@ -301,13 +305,13 @@ mod tests { } #[test] - fn a_file_with_no_header_gets_names_from_the_positions() { + fn a_file_with_no_header_leaves_its_field_names_empty() { let mut options = options(); options.has_header = false; let sheet = read_csv(b"1,ada\n2,grace\n", &options, None).expect("read"); - assert_eq!(sheet.headers, vec!["Column 1", "Column 2"]); + assert_eq!(sheet.headers, vec![String::new(), String::new()]); assert_eq!(sheet.rows.len(), 2); } @@ -533,6 +537,6 @@ mod tests { assert_eq!(error.line, 4); assert_eq!(error.column, "id"); assert_eq!(error.text, "not a number"); - assert!(!error.reason.is_empty()); + assert_eq!(error.reason, EditParseError::InvalidInteger); } } diff --git a/linux/crates/core/src/lib.rs b/linux/crates/core/src/lib.rs index f6d721f09d..e88cd26574 100644 --- a/linux/crates/core/src/lib.rs +++ b/linux/crates/core/src/lib.rs @@ -66,6 +66,7 @@ mod transport_class; mod transport_error; mod transport_kind; mod transport_route; +mod unsupported_feature; mod user_sql; pub mod value; mod workspace_settings; @@ -135,6 +136,7 @@ pub use transport_class::TransportClass; pub use transport_error::TransportError; pub use transport_kind::TransportKind; pub use transport_route::TransportRoute; +pub use unsupported_feature::UnsupportedFeature; pub use user_sql::UserSql; pub use value::Value; pub use workspace_settings::WorkspaceSettings; diff --git a/linux/crates/core/src/sql_syntax/errors.rs b/linux/crates/core/src/sql_syntax/errors.rs index 942979427f..b01456eb87 100644 --- a/linux/crates/core/src/sql_syntax/errors.rs +++ b/linux/crates/core/src/sql_syntax/errors.rs @@ -9,8 +9,10 @@ pub enum TypeSyntaxError { TooLong { max: usize }, #[error("the type contains a comment")] Comment, - #[error("the type is not valid for this engine: {0}")] - Invalid(String), + /// The engine's parser does not read the whole text as one type. + /// sqlparser's own message is English only, so none is kept. + #[error("the type is not valid for this engine")] + Invalid, /// MySQL and PostgreSQL read a backslash differently depending on a /// session setting, so text containing one cannot be validated here. #[error("the value contains a backslash, which this engine reads differently per session")] @@ -26,8 +28,10 @@ pub enum ExpressionSyntaxError { TooLong { max: usize }, #[error("the expression contains a comment")] Comment, - #[error("the expression is not valid for this engine: {0}")] - Invalid(String), + /// The engine's parser does not read the whole text as one + /// expression. + #[error("the expression is not valid for this engine")] + Invalid, #[error("the value contains a backslash, which this engine reads differently per session")] SessionDependentBackslash, } diff --git a/linux/crates/core/src/sql_syntax/parse_expression.rs b/linux/crates/core/src/sql_syntax/parse_expression.rs index d5e292c5d3..216646989c 100644 --- a/linux/crates/core/src/sql_syntax/parse_expression.rs +++ b/linux/crates/core/src/sql_syntax/parse_expression.rs @@ -40,14 +40,10 @@ pub fn parse_expression( let mut parser = Parser::new(grammar.parser_dialect()) .with_recursion_limit(RECURSION_LIMIT) .try_with_sql(text) - .map_err(|error| ExpressionSyntaxError::Invalid(error.to_string()))?; - let parsed = parser - .parse_expr() - .map_err(|error| ExpressionSyntaxError::Invalid(error.to_string()))?; + .map_err(|_| ExpressionSyntaxError::Invalid)?; + let parsed = parser.parse_expr().map_err(|_| ExpressionSyntaxError::Invalid)?; if parser.peek_token_ref().token != Token::EOF { - return Err(ExpressionSyntaxError::Invalid( - "unexpected text after the expression".to_owned(), - )); + return Err(ExpressionSyntaxError::Invalid); } Ok(ValidatedExpression { text: text.to_owned(), @@ -139,7 +135,7 @@ mod tests { fn parse_expression_rejects_trailing_tokens() { let error = parse("1; DROP TABLE x").expect_err("trailing statement"); - assert!(matches!(error, ExpressionSyntaxError::Invalid(_)), "{error:?}"); + assert_eq!(error, ExpressionSyntaxError::Invalid); } #[test] diff --git a/linux/crates/core/src/sql_syntax/parse_type.rs b/linux/crates/core/src/sql_syntax/parse_type.rs index b08c2d4728..97a1655d8a 100644 --- a/linux/crates/core/src/sql_syntax/parse_type.rs +++ b/linux/crates/core/src/sql_syntax/parse_type.rs @@ -56,14 +56,12 @@ pub fn parse_type(grammar: SqlGrammar, text: &str, max_len: usize) -> Result for SqlBuildError { - fn from(error: LiteralError) -> Self { - Self::Ddl(error.to_string()) - } + Ddl(#[source] Box), } impl From for SqlBuildError { fn from(error: BuildDdlError) -> Self { - Self::Ddl(error.to_string()) + Self::Ddl(Box::new(error)) } } diff --git a/linux/crates/core/src/unsupported_feature.rs b/linux/crates/core/src/unsupported_feature.rs new file mode 100644 index 0000000000..893e872be1 --- /dev/null +++ b/linux/crates/core/src/unsupported_feature.rs @@ -0,0 +1,63 @@ +use std::fmt; + +/// What an engine cannot do, as a value the app turns into its own +/// sentence. A driver names one of these rather than writing the +/// feature in English. Closed, so a new one needs its sentence in the +/// app before it builds. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum UnsupportedFeature { + /// A file path, for an engine reached over the network. + DatabaseFile, + /// A host and port, for an engine that opens a file. + NetworkConnection, + /// A save that turns foreign key checks off and verifies them after. + ForeignKeySuspension, + /// The engine's client library is older than the driver needs, such + /// as `SQLite 3.37.0`. + MinimumVersion { + required: &'static str, + }, + CopyFromStdinInEditor, + CopyToStdoutInEditor, + /// A TIMESTAMP in the hour the session time zone repeats, which the + /// server cannot store as the one instant it was given. + TimestampInRepeatedHour, + TimeOutsideOneDay, + TimeWithOffset, + Interval, + BitString, + NumberWiderThan { + digits: u32, + }, + /// A decimal that is NaN or infinite, or wider than `digits`. + DecimalOutOfRange { + digits: u32, + }, + DateOutsideYears { + first: i32, + last: i32, + }, +} + +impl fmt::Display for UnsupportedFeature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DatabaseFile => f.write_str("a database file"), + Self::NetworkConnection => f.write_str("network connections"), + Self::ForeignKeySuspension => f.write_str("foreign key suspension"), + Self::MinimumVersion { required } => write!(f, "{required} or later"), + Self::CopyFromStdinInEditor => f.write_str("COPY FROM STDIN in the editor"), + Self::CopyToStdoutInEditor => f.write_str("COPY TO STDOUT in the editor"), + Self::TimestampInRepeatedHour => { + f.write_str("single instant for a TIMESTAMP in the hour its time zone repeats") + } + Self::TimeOutsideOneDay => f.write_str("a time outside one day"), + Self::TimeWithOffset => f.write_str("a time with an offset"), + Self::Interval => f.write_str("an interval"), + Self::BitString => f.write_str("a bit string"), + Self::NumberWiderThan { digits } => write!(f, "a number wider than {digits} digits"), + Self::DecimalOutOfRange { digits } => write!(f, "a decimal NaN, infinity or wider than {digits} digits"), + Self::DateOutsideYears { first, last } => write!(f, "a date outside years {first} to {last}"), + } + } +} diff --git a/linux/crates/core/src/value/json_text.rs b/linux/crates/core/src/value/json_text.rs index b796231a80..21cbf7a48d 100644 --- a/linux/crates/core/src/value/json_text.rs +++ b/linux/crates/core/src/value/json_text.rs @@ -5,16 +5,31 @@ pub struct JsonText { text: Box, } -#[derive(Debug, Clone, PartialEq, Eq, Error)] +/// Where serde_json stopped reading. Its message is English only, so +/// the position is kept instead, for the app to point at. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] pub enum JsonTextError { - #[error("invalid JSON: {0}")] - Invalid(String), + #[error("the JSON ends before its value does")] + Incomplete, + #[error("invalid JSON at line {line}, column {column}")] + Invalid { line: usize, column: usize }, +} + +impl JsonTextError { + fn from_serde(error: &serde_json::Error) -> Self { + match error.classify() { + serde_json::error::Category::Eof => Self::Incomplete, + _ => Self::Invalid { + line: error.line(), + column: error.column(), + }, + } + } } impl JsonText { pub fn parse(text: String) -> Result { - serde_json::from_str::(&text) - .map_err(|error| JsonTextError::Invalid(error.to_string()))?; + serde_json::from_str::(&text).map_err(|error| JsonTextError::from_serde(&error))?; Ok(Self { text: text.into_boxed_str(), }) @@ -36,4 +51,17 @@ mod tests { let document = r#"{"price": 12.3400000000000000001, "tags": ["a"]}"#; assert_eq!(JsonText::parse(document.to_owned()).unwrap().as_str(), document); } + + #[test] + fn a_refusal_says_where_the_json_went_wrong() { + assert_eq!(JsonText::parse("{\"a\": 1".to_owned()), Err(JsonTextError::Incomplete)); + assert_eq!(JsonText::parse(String::new()), Err(JsonTextError::Incomplete)); + assert!( + matches!( + JsonText::parse("[1,\n2,\n3 4]".to_owned()), + Err(JsonTextError::Invalid { line: 3, .. }) + ), + "a missing comma on the third line" + ); + } } diff --git a/linux/crates/drivers/clickhouse/src/driver.rs b/linux/crates/drivers/clickhouse/src/driver.rs index 4c70c903f8..ebb46b51b4 100644 --- a/linux/crates/drivers/clickhouse/src/driver.rs +++ b/linux/crates/drivers/clickhouse/src/driver.rs @@ -1,7 +1,9 @@ use std::sync::Arc; use async_trait::async_trait; -use tablepro_core::{ConnectOptions, ConnectTarget, Connection, DatabaseDriver, DriverCapabilities, DriverError}; +use tablepro_core::{ + ConnectOptions, ConnectTarget, Connection, DatabaseDriver, DriverCapabilities, DriverError, UnsupportedFeature, +}; use tokio_util::sync::CancellationToken; use crate::capabilities::CAPABILITIES; @@ -34,7 +36,7 @@ impl DatabaseDriver for ClickhouseDriver { ) -> Result, DriverError> { let ConnectTarget::Network(target) = &options.target else { return Err(DriverError::Unsupported { - feature: "a database file", + feature: UnsupportedFeature::DatabaseFile, }); }; if let Some(tls) = CAPABILITIES.tls.as_ref() { diff --git a/linux/crates/drivers/clickhouse/src/write_batch.rs b/linux/crates/drivers/clickhouse/src/write_batch.rs index 93a5c2ef5e..16d6c062e3 100644 --- a/linux/crates/drivers/clickhouse/src/write_batch.rs +++ b/linux/crates/drivers/clickhouse/src/write_batch.rs @@ -172,7 +172,7 @@ async fn count(calls: &CallClient, probe: &Statement) -> Result Result, DriverError> { let ConnectTarget::Network(target) = &options.target else { return Err(DriverError::Unsupported { - feature: "a database file", + feature: UnsupportedFeature::DatabaseFile, }); }; if let Some(tls) = CAPABILITIES.tls.as_ref() { diff --git a/linux/crates/drivers/mssql/src/encode.rs b/linux/crates/drivers/mssql/src/encode.rs index fb1a798d4c..901fccdc9a 100644 --- a/linux/crates/drivers/mssql/src/encode.rs +++ b/linux/crates/drivers/mssql/src/encode.rs @@ -1,9 +1,9 @@ use std::borrow::Cow; use chrono::{Datelike, NaiveDate}; -use tablepro_core::DriverError; use tablepro_core::statement::{BoundParam, ParamType}; use tablepro_core::value::{Temporal, Value}; +use tablepro_core::{DriverError, UnsupportedFeature}; use tiberius::numeric::Numeric; use tiberius::{ColumnData, IntoSql, ToSql}; @@ -87,7 +87,7 @@ pub(crate) fn encode(param: &BoundParam) -> Result, DriverEr .to_time_of_day() .map(IntoSql::into_sql) .ok_or(DriverError::Unsupported { - feature: "a time outside one day", + feature: UnsupportedFeature::TimeOutsideOneDay, }), other => Err(refused(param_type, other)), }, @@ -106,11 +106,13 @@ pub(crate) fn encode(param: &BoundParam) -> Result, DriverEr other => Err(refused(param_type, other)), }, ParamType::TimeTz => Err(DriverError::Unsupported { - feature: "a time with an offset", + feature: UnsupportedFeature::TimeWithOffset, + }), + ParamType::Interval => Err(DriverError::Unsupported { + feature: UnsupportedFeature::Interval, }), - ParamType::Interval => Err(DriverError::Unsupported { feature: "an interval" }), ParamType::Bits => Err(DriverError::Unsupported { - feature: "a bit string", + feature: UnsupportedFeature::BitString, }), } } @@ -161,12 +163,16 @@ fn numeric(param_type: &ParamType, value: &Value) -> Result (i128::from(*integer), 0), Value::WideInt(integer) => ( integer.to_i128().ok_or(DriverError::Unsupported { - feature: "an integer wider than 38 digits", + feature: UnsupportedFeature::NumberWiderThan { + digits: MAX_NUMERIC_DIGITS, + }, })?, 0, ), Value::Decimal(decimal) => decimal.to_scaled_i128().ok_or(DriverError::Unsupported { - feature: "a decimal NaN, infinity or wider than 38 digits", + feature: UnsupportedFeature::DecimalOutOfRange { + digits: MAX_NUMERIC_DIGITS, + }, })?, other => return Err(refused(param_type, other)), }; @@ -179,7 +185,9 @@ fn numeric(param_type: &ParamType, value: &Value) -> Result Ok(Numeric::new_with_scale(mantissa, scale)), _ => Err(DriverError::Unsupported { - feature: "a decimal wider than 38 digits", + feature: UnsupportedFeature::NumberWiderThan { + digits: MAX_NUMERIC_DIGITS, + }, }), } } @@ -189,7 +197,7 @@ fn in_range(date: NaiveDate) -> Result { match (1..=9999).contains(&date.year()) { true => Ok(date), false => Err(DriverError::Unsupported { - feature: "a date outside years 1 to 9999", + feature: UnsupportedFeature::DateOutsideYears { first: 1, last: 9999 }, }), } } diff --git a/linux/crates/drivers/mssql/src/write_batch.rs b/linux/crates/drivers/mssql/src/write_batch.rs index a499b936e1..9157f434b4 100644 --- a/linux/crates/drivers/mssql/src/write_batch.rs +++ b/linux/crates/drivers/mssql/src/write_batch.rs @@ -235,7 +235,7 @@ impl Write<'_> { #[cfg(test)] mod tests { use tablepro_core::meta::TableRef; - use tablepro_core::{ForeignKeyCheck, ForeignKeyMode, WriteKind}; + use tablepro_core::{ForeignKeyCheck, ForeignKeyMode, UnsupportedFeature, WriteKind}; use super::*; @@ -257,7 +257,7 @@ mod tests { assert!(matches!( precheck(&batch(suspended), false), Err(DriverError::Unsupported { - feature: "foreign key suspension" + feature: UnsupportedFeature::ForeignKeySuspension }) )); assert!(precheck(&batch(ForeignKeyMode::Enforced), false).is_ok()); diff --git a/linux/crates/drivers/mysql/src/driver.rs b/linux/crates/drivers/mysql/src/driver.rs index 4978f65223..65882bcdff 100644 --- a/linux/crates/drivers/mysql/src/driver.rs +++ b/linux/crates/drivers/mysql/src/driver.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use async_trait::async_trait; use tablepro_core::{ ConfigError, ConnectOptions, ConnectTarget, Connection, DatabaseDriver, DriverCapabilities, DriverError, + UnsupportedFeature, }; use tokio_util::sync::CancellationToken; @@ -39,7 +40,7 @@ impl DatabaseDriver for MysqlDriver { ) -> Result, DriverError> { let ConnectTarget::Network(target) = &options.target else { return Err(DriverError::Unsupported { - feature: "a database file", + feature: UnsupportedFeature::DatabaseFile, }); }; if target.database.as_deref().is_none_or(str::is_empty) { diff --git a/linux/crates/drivers/mysql/src/server_zone.rs b/linux/crates/drivers/mysql/src/server_zone.rs index be4240ed97..2f69423f54 100644 --- a/linux/crates/drivers/mysql/src/server_zone.rs +++ b/linux/crates/drivers/mysql/src/server_zone.rs @@ -36,9 +36,9 @@ use chrono::NaiveDateTime; use sqlx::mysql::MySqlConnection; use sqlx::{AssertSqlSafe, Executor as _, Row as _}; -use tablepro_core::DriverError; use tablepro_core::statement::{BoundParam, GuardedStatement}; use tablepro_core::value::{Temporal, Value}; +use tablepro_core::{DriverError, UnsupportedFeature}; use crate::decode::decode_row; use crate::errors::map_error; @@ -55,9 +55,6 @@ pub(crate) const READ_ZONE_STATEMENT: &str = "SET SESSION time_zone = '+00:00'"; /// A TIMESTAMP parameter, bound as UTC wall time. pub(crate) const TIMESTAMP_PLACEHOLDER: &str = "CONVERT_TZ(?, '+00:00', @@session.time_zone)"; -/// How the refusal reads: "This database has no {feature}." -const REPEATED_HOUR: &str = "single instant for a TIMESTAMP in the hour its time zone repeats"; - /// Instants checked per round trip. Each is bound twice, far inside /// the server's 65,535 placeholders. const CHECK_CHUNK: usize = 100; @@ -105,7 +102,9 @@ pub(crate) async fn repeated_instant( /// The refusal for a batch [`repeated_instant`] found a step in. pub(crate) fn repeated_hour() -> DriverError { - DriverError::Unsupported { feature: REPEATED_HOUR } + DriverError::Unsupported { + feature: UnsupportedFeature::TimestampInRepeatedHour, + } } /// Every finite instant a step binds, its probe's included. diff --git a/linux/crates/drivers/mysql/src/write_batch.rs b/linux/crates/drivers/mysql/src/write_batch.rs index d13e98b66e..b14e9bed1c 100644 --- a/linux/crates/drivers/mysql/src/write_batch.rs +++ b/linux/crates/drivers/mysql/src/write_batch.rs @@ -324,7 +324,7 @@ fn acknowledged(request: &StopRequest) -> DriverError { #[cfg(test)] mod tests { use tablepro_core::meta::TableRef; - use tablepro_core::{ForeignKeyCheck, ForeignKeyMode, StopReason, TimeoutPhase}; + use tablepro_core::{ForeignKeyCheck, ForeignKeyMode, StopReason, TimeoutPhase, UnsupportedFeature}; use super::*; @@ -346,7 +346,7 @@ mod tests { assert_eq!( precheck(&batch(suspended), false), Err(DriverError::Unsupported { - feature: "foreign key suspension" + feature: UnsupportedFeature::ForeignKeySuspension }) ); assert_eq!( diff --git a/linux/crates/drivers/postgres/src/driver.rs b/linux/crates/drivers/postgres/src/driver.rs index 85694d16be..31fac50936 100644 --- a/linux/crates/drivers/postgres/src/driver.rs +++ b/linux/crates/drivers/postgres/src/driver.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use async_trait::async_trait; use tablepro_core::{ ConnectOptions, ConnectTarget, Connection, DatabaseDriver, DriverCapabilities, DriverError, TlsCapabilities, + UnsupportedFeature, }; use tokio_util::sync::CancellationToken; use tokio_util::task::TaskTracker; @@ -39,7 +40,7 @@ impl DatabaseDriver for PgDriver { ) -> Result, DriverError> { let ConnectTarget::Network(target) = &options.target else { return Err(DriverError::Unsupported { - feature: "a database file", + feature: UnsupportedFeature::DatabaseFile, }); }; let tls: Option<&TlsCapabilities> = CAPABILITIES.tls.as_ref(); diff --git a/linux/crates/drivers/postgres/src/editor/editor_dispatch.rs b/linux/crates/drivers/postgres/src/editor/editor_dispatch.rs index f1f335412c..47f87298af 100644 --- a/linux/crates/drivers/postgres/src/editor/editor_dispatch.rs +++ b/linux/crates/drivers/postgres/src/editor/editor_dispatch.rs @@ -1,4 +1,4 @@ -use tablepro_core::{DriverError, ReadOnlyRefusal}; +use tablepro_core::{DriverError, ReadOnlyRefusal, UnsupportedFeature}; use super::statement_class::StatementClass; use super::transaction_tracker::KnownState; @@ -44,12 +44,12 @@ pub(crate) fn plan(class: StatementClass, state: KnownState, read_only: bool) -> match class { StatementClass::CopyFromStdin => { return Flight::Refuse(DriverError::Unsupported { - feature: "COPY FROM STDIN in the editor", + feature: UnsupportedFeature::CopyFromStdinInEditor, }); } StatementClass::CopyToStdout => { return Flight::Refuse(DriverError::Unsupported { - feature: "COPY TO STDOUT in the editor", + feature: UnsupportedFeature::CopyToStdoutInEditor, }); } StatementClass::TwoPhase if read_only => { diff --git a/linux/crates/drivers/postgres/src/write_batch.rs b/linux/crates/drivers/postgres/src/write_batch.rs index 591c7f07d8..2a2cb832cf 100644 --- a/linux/crates/drivers/postgres/src/write_batch.rs +++ b/linux/crates/drivers/postgres/src/write_batch.rs @@ -209,7 +209,7 @@ async fn transaction_id(client: &Client, legacy: bool) -> Result Result<(), DriverError> { if *version < SQLITE_MIN_VERSION { return Err(DriverError::Unsupported { - feature: "SQLite 3.37.0 or later", + feature: UnsupportedFeature::MinimumVersion { + required: "SQLite 3.37.0", + }, }); } Ok(()) @@ -62,7 +64,9 @@ mod tests { assert_eq!( refused, Err(DriverError::Unsupported { - feature: "SQLite 3.37.0 or later" + feature: UnsupportedFeature::MinimumVersion { + required: "SQLite 3.37.0" + } }) ); assert_eq!(check_runtime_version(&SqliteVersion::new(3, 37, 0)), Ok(())); diff --git a/linux/crates/drivers/sqlite/src/driver.rs b/linux/crates/drivers/sqlite/src/driver.rs index dab3da34d7..4d426f243d 100644 --- a/linux/crates/drivers/sqlite/src/driver.rs +++ b/linux/crates/drivers/sqlite/src/driver.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use async_trait::async_trait; use tablepro_core::{ ConnectOptions, ConnectTarget, Connection, DatabaseDriver, DriverCapabilities, DriverError, TimeoutPhase, + UnsupportedFeature, }; use tokio_util::sync::CancellationToken; @@ -39,7 +40,7 @@ impl DatabaseDriver for SqliteDriver { ) -> Result, DriverError> { let ConnectTarget::File(endpoint) = &options.target else { return Err(DriverError::Unsupported { - feature: "network connections", + feature: UnsupportedFeature::NetworkConnection, }); }; let opening = SqliteConnection::open(endpoint, options.workspace_settings, options.liveness); diff --git a/linux/crates/drivers/sqlite/tests/csv_import.rs b/linux/crates/drivers/sqlite/tests/csv_import.rs index e3b93a075e..f51ffe6580 100644 --- a/linux/crates/drivers/sqlite/tests/csv_import.rs +++ b/linux/crates/drivers/sqlite/tests/csv_import.rs @@ -310,7 +310,7 @@ async fn a_semicolon_file_without_a_header_imports_by_position() { // mapping is empty and every column keeps its default. That is the // honest outcome: the dialog is where the user maps by hand. let sheet = read_csv(b"one;two\n", &options, None).expect("the file"); - assert_eq!(sheet.headers, vec!["Column 1", "Column 2"]); + assert_eq!(sheet.headers, vec![String::new(), String::new()]); let table = TableRef::unqualified("pairs"); let definition = connection diff --git a/linux/crates/net/src/tls/pem_check.rs b/linux/crates/net/src/tls/pem_check.rs index e944bdb4b2..dfca3fde0c 100644 --- a/linux/crates/net/src/tls/pem_check.rs +++ b/linux/crates/net/src/tls/pem_check.rs @@ -52,9 +52,10 @@ pub(crate) fn read_identity(identity: &ClientIdentity) -> Result CertificateFileProblem::NoPrivateKey, - other => CertificateFileProblem::InvalidPrivateKey { - detail: other.to_string(), - }, + other => { + tracing::debug!(error = %other, "the private key file is not valid"); + CertificateFileProblem::InvalidPrivateKey + } }; identity_problem(&identity.key, problem) })?; @@ -71,9 +72,7 @@ pub(crate) fn read_identity(identity: &ClientIdentity) -> Result Result, CertificateFileProblem> { - let unreadable = |error: std::io::Error| CertificateFileProblem::Unreadable { - detail: error.to_string(), - }; + let unreadable = |error: std::io::Error| CertificateFileProblem::Unreadable { kind: error.kind() }; if !std::fs::metadata(path).map_err(unreadable)?.is_file() { return Err(CertificateFileProblem::NotRegularFile); } @@ -103,11 +102,10 @@ fn parse_certificates(bytes: &[u8]) -> Result>, Cert } let mut scratch = RootCertStore::empty(); for certificate in &certificates { - scratch - .add(certificate.clone()) - .map_err(|error| CertificateFileProblem::InvalidCertificate { - detail: error.to_string(), - })?; + scratch.add(certificate.clone()).map_err(|error| { + tracing::debug!(%error, "a certificate in the file is not valid"); + CertificateFileProblem::InvalidCertificate + })?; } Ok(certificates) } @@ -115,8 +113,9 @@ fn parse_certificates(bytes: &[u8]) -> Result>, Cert fn certificate_problem(error: pem::Error) -> CertificateFileProblem { match error { pem::Error::NoItemsFound => CertificateFileProblem::NoCertificates, - other => CertificateFileProblem::InvalidCertificate { - detail: other.to_string(), - }, + other => { + tracing::debug!(error = %other, "the certificate file is not valid PEM"); + CertificateFileProblem::InvalidCertificate + } } } diff --git a/linux/crates/net/tests/tls_handshake.rs b/linux/crates/net/tests/tls_handshake.rs index 3d81963a05..ae7a062915 100644 --- a/linux/crates/net/tests/tls_handshake.rs +++ b/linux/crates/net/tests/tls_handshake.rs @@ -185,7 +185,7 @@ fn malformed_ca_file_is_invalid_certificate() { assert!(matches!( check_pem_files(&verify_full(Some(&ca_file))), Err(ConfigError::CaFile { - problem: CertificateFileProblem::InvalidCertificate { .. }, + problem: CertificateFileProblem::InvalidCertificate, .. }) )); diff --git a/linux/crates/session/src/connect/driver_connector.rs b/linux/crates/session/src/connect/driver_connector.rs index 939abce8f5..129c106d83 100644 --- a/linux/crates/session/src/connect/driver_connector.rs +++ b/linux/crates/session/src/connect/driver_connector.rs @@ -5,7 +5,8 @@ use futures::future::BoxFuture; use tablepro_core::credentials::CredentialInteraction; use tablepro_core::meta::{ObjectInfo, ObjectScope, SchemaInfo}; use tablepro_core::{ - CallOptions, ConfigError, ConnectOptions, ConnectTarget, Connection, DriverError, NetworkTarget, TlsMode, Transport, + CallOptions, ConfigError, ConnectOptions, ConnectTarget, Connection, DriverError, NetworkTarget, TlsMode, + Transport, UnsupportedFeature, }; use tablepro_net::DirectTcpTransport; use tablepro_ssh::SshSession; @@ -159,11 +160,11 @@ fn validate(request: &ConnectRequest) -> Result<(), ConnectError> { let refuse = |error: DriverError| Err(ConnectError::Driver(error)); match &request.target { RequestTarget::File(_) if !capabilities.is_file_based() => refuse(DriverError::Unsupported { - feature: "database files", + feature: UnsupportedFeature::DatabaseFile, }), RequestTarget::File(_) => Ok(()), RequestTarget::Network { .. } if capabilities.is_file_based() => refuse(DriverError::Unsupported { - feature: "network connections", + feature: UnsupportedFeature::NetworkConnection, }), RequestTarget::Network { credentials, diff --git a/linux/crates/storage/src/credentials/keyring_credentials.rs b/linux/crates/storage/src/credentials/keyring_credentials.rs index 409f06dddc..4e956ffc09 100644 --- a/linux/crates/storage/src/credentials/keyring_credentials.rs +++ b/linux/crates/storage/src/credentials/keyring_credentials.rs @@ -518,6 +518,7 @@ mod tests { let prompter = Prompter::answering([typed("typed", true)]); let backend = SecretError::Backend { detail: "broken".to_owned(), + service_message: None, }; let (credentials, _) = credentials(Vault::failing(SecretKind::DatabasePassword, backend.clone()), None); diff --git a/linux/crates/storage/src/secrets/keyring_error.rs b/linux/crates/storage/src/secrets/keyring_error.rs index f9c61df3ac..94848e2450 100644 --- a/linux/crates/storage/src/secrets/keyring_error.rs +++ b/linux/crates/storage/src/secrets/keyring_error.rs @@ -28,11 +28,37 @@ pub(super) fn map_error(error: oo7::Error, stage: Stage) -> SecretError { detail: error.to_string(), }, other => SecretError::Backend { + service_message: service_message(&other), detail: other.to_string(), }, } } +/// The Secret Service's own words, for a failure that is its reply +/// rather than oo7's or the bus connection's. +fn service_message(error: &oo7::Error) -> Option { + use oo7::dbus::{Error, ServiceError}; + use oo7::zbus::DBusError as _; + + let oo7::Error::DBus(error) = error else { + return None; + }; + let message = match error { + Error::Service( + ServiceError::IsLocked(message) | ServiceError::NoSession(message) | ServiceError::NoSuchObject(message), + ) => Some(message.clone()), + Error::Service(ServiceError::ZBus(error)) | Error::ZBus(error) => match error { + oo7::zbus::Error::MethodError(name, description, _) => { + Some(description.clone().unwrap_or_else(|| name.to_string())) + } + oo7::zbus::Error::FDO(error) => error.description().map(str::to_owned), + _ => None, + }, + _ => None, + }; + message.filter(|message| !message.trim().is_empty()) +} + /// Whether the cached handle is worth keeping. A wire error means the /// connection is gone, so the next call reopens instead of retrying on a /// dead socket. @@ -81,7 +107,51 @@ mod tests { fn map_not_found_while_using_is_a_backend_error() { let mapped = map_error(oo7::dbus::Error::NotFound("collection".to_owned()).into(), Stage::Use); - assert!(matches!(mapped, SecretError::Backend { .. }), "{mapped:?}"); + assert!( + matches!( + mapped, + SecretError::Backend { + service_message: None, + .. + } + ), + "{mapped:?}" + ); + } + + #[test] + fn a_reply_from_the_service_keeps_its_own_words() { + let mapped = map_error( + oo7::dbus::Error::Service(oo7::dbus::ServiceError::NoSuchObject( + "No such item at /org/freedesktop/secrets/collection/login/7".to_owned(), + )) + .into(), + Stage::Use, + ); + + let SecretError::Backend { service_message, .. } = mapped else { + panic!("expected a backend error, got {mapped:?}"); + }; + assert_eq!( + service_message.as_deref(), + Some("No such item at /org/freedesktop/secrets/collection/login/7") + ); + } + + #[test] + fn oo7s_own_text_is_not_taken_for_the_services() { + let mapped = map_error(oo7::dbus::Error::Deleted.into(), Stage::Use); + + assert!( + matches!( + mapped, + SecretError::Backend { + service_message: None, + .. + } + ), + "{mapped:?}" + ); } #[test] diff --git a/linux/docs/error-handling.md b/linux/docs/error-handling.md index 6dfe2ea3a8..8ac17cb714 100644 --- a/linux/docs/error-handling.md +++ b/linux/docs/error-handling.md @@ -36,15 +36,16 @@ Errors are typed enums, built with `thiserror` wherever they cross a crate bound | `Busy` | The server has no room for another connection, or the pool had no free slot in time. | | `EditorSessionBusy` | The tab's session is already running a statement. | | `Config(ConfigError)` | The settings cannot be used: a bad host, port, path, TLS option, auth mode or missing database name. | -| `Decode { column, detail }` | The driver could not read a value. A bug on our side, not the server's. | -| `Protocol(String)` | The server or the client library said something the driver did not expect. | -| `Unsupported { feature }` | The engine has no such feature. | +| `Decode { column, detail }` | The driver could not read a value. A bug on our side, not the server's. The detail is for the log. | +| `Protocol(String)` | The server or the client library said something the driver did not expect. The text is for the log. | +| `Unsupported { feature }` | The engine has no such feature. `UnsupportedFeature` names it, and the app writes the sentence. | Rules: - Add a variant only when the app has to act on it differently from every existing one. A new message is not a reason. - Never wrap a `Box` or the client library's error. Keep what the app needs: a code, a phase, a path. - The `#[error]` text is for logs. The UI writes its own sentence from the variant. +- A variant carries no English meant for the user. What the app needs to say goes in as a value it can render: an `UnsupportedFeature`, an `io::ErrorKind`, a line and column. A library message that is English only, such as sqlparser's or serde_json's, is not kept. ## `ErrorCategory` @@ -125,7 +126,9 @@ The buttons are `TlsRemedy` in `crates/app/src/ui/error_text/tls_remedy.rs`: the ## User-facing text -`crates/app/src/ui/error_text/` turns errors into sentences: `driver_message` for a `DriverError`, `connect_error_message` and `operation_error_message` for the session's errors, and an SSH module for `SshFailure`. Every string goes through `crate::i18n`. Never show `Debug` or the `#[error]` text of an error on its own. +`crates/app/src/ui/error_text/` turns errors into sentences: `driver_message` for a `DriverError`, `connect_error_message` and `operation_error_message` for the session's errors, and an SSH module for `SshFailure`. Every string goes through `crate::i18n`. Never show `Debug` or the `#[error]` text of an error, on its own or inside a sentence. + +The only text shown as it came is what another party wrote, inside a sentence that says whose it is: a server's `ServerDiagnostics`, the Secret Service's reply (the `service_message` of `SecretError::Backend`), OpenSSH's stderr, and the Kerberos and TLS library reports. The detail of an `Internal` error is logged at `trace` when it is shown, since it can quote what the server sent. ## Logging diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index f67f49ef8d..243abb11dc 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -106,6 +106,7 @@ crates/app/src/test_support/focus_thief.rs crates/app/src/test_support/icon_buttons.rs crates/app/src/test_support/key_injection_error.rs crates/app/src/test_support/key_injector.rs +crates/app/src/test_support/log_lines.rs crates/app/src/test_support/main_loop.rs crates/app/src/test_support/memory_settings.rs crates/app/src/test_support/mod.rs @@ -229,15 +230,19 @@ crates/app/src/ui/editor_font_provider.rs crates/app/src/ui/error_text/ddl.rs crates/app/src/ui/error_text/driver.rs crates/app/src/ui/error_text/edit.rs +crates/app/src/ui/error_text/files.rs crates/app/src/ui/error_text/mod.rs crates/app/src/ui/error_text/operation.rs crates/app/src/ui/error_text/saved_connection.rs crates/app/src/ui/error_text/script.rs crates/app/src/ui/error_text/secret.rs crates/app/src/ui/error_text/ssh.rs +crates/app/src/ui/error_text/statement.rs +crates/app/src/ui/error_text/syntax.rs crates/app/src/ui/error_text/tests.rs crates/app/src/ui/error_text/tls.rs crates/app/src/ui/error_text/tls_remedy.rs +crates/app/src/ui/error_text/unsupported.rs crates/app/src/ui/export/dialog.rs crates/app/src/ui/export/document_sink.rs crates/app/src/ui/export/encoding_sink.rs diff --git a/linux/po/tablepro.pot b/linux/po/tablepro.pot index 5eac8d702e..8dece2d49f 100644 --- a/linux/po/tablepro.pot +++ b/linux/po/tablepro.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: tablepro 0.1.0\n" "Report-Msgid-Bugs-To: https://github.com/TableProApp/TablePro/issues\n" -"POT-Creation-Date: 2026-10-18 18:05+0000\n" +"POT-Creation-Date: 2026-10-19 00:18+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -1236,12 +1236,12 @@ msgstr "" #: crates/app/src/ui/error_text/ddl.rs #, rust-format -msgid "Column {column} has a type this database doesn't accept: {error}" +msgid "The type of column {column} can't be used. {reason}" msgstr "" #: crates/app/src/ui/error_text/ddl.rs #, rust-format -msgid "The default of column {column} can't be written as SQL: {error}" +msgid "The default of column {column} can't be written as SQL. {reason}" msgstr "" #: crates/app/src/ui/error_text/ddl.rs @@ -1298,11 +1298,6 @@ msgid "" "The server has no room for another connection right now. Try again shortly." msgstr "" -#: crates/app/src/ui/error_text/driver.rs -#, rust-format -msgid "This database has no {feature}." -msgstr "" - #: crates/app/src/ui/error_text/driver.rs msgid "The connection settings cannot be used." msgstr "" @@ -1540,11 +1535,6 @@ msgstr "" msgid "{path} is not a database file." msgstr "" -#: crates/app/src/ui/error_text/driver.rs -#, rust-format -msgid "Could not write {path}: {reason}" -msgstr "" - #: crates/app/src/ui/error_text/driver.rs msgid "The database file cannot be used." msgstr "" @@ -1557,12 +1547,11 @@ msgstr "" #: crates/app/src/ui/error_text/driver.rs #, rust-format -msgid "Could not read column {column}: {detail}" +msgid "The driver could not read a value in column {column}." msgstr "" #: crates/app/src/ui/error_text/driver.rs -#, rust-format -msgid "The driver could not read the server's reply: {detail}" +msgid "The driver could not read the server's reply." msgstr "" #: crates/app/src/ui/error_text/driver.rs @@ -1612,9 +1601,13 @@ msgstr "" msgid "Enter a UUID" msgstr "" +#: crates/app/src/ui/error_text/edit.rs +msgid "Enter valid JSON: it ends too early" +msgstr "" + #: crates/app/src/ui/error_text/edit.rs #, rust-format -msgid "Enter valid JSON: {detail}" +msgid "Enter valid JSON: check line {line}, column {column}" msgstr "" #: crates/app/src/ui/error_text/edit.rs @@ -1676,6 +1669,70 @@ msgstr "" msgid "Values whose type changes by row can't be edited here" msgstr "" +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "Could not write {path}: the disk is full." +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "Could not write {path}: the disk is read-only." +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "Could not write {path}: the file is too large." +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "Could not write {path}." +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "the file does not exist" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "TablePro is not allowed to read it" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "the file could not be read" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "it is not a plain file" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "it is larger than {size}" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "it holds no certificates" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "a certificate in it is not valid" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +#, rust-format +msgid "it holds {count} certificate, and only one is allowed" +msgid_plural "it holds {count} certificates, and only one is allowed" +msgstr[0] "" +msgstr[1] "" + +#: crates/app/src/ui/error_text/files.rs +msgid "it holds no private key" +msgstr "" + +#: crates/app/src/ui/error_text/files.rs +msgid "its private key is not valid" +msgstr "" + #: crates/app/src/ui/error_text/operation.rs #, rust-format msgid "TablePro hit an internal error: {detail}" @@ -1706,15 +1763,11 @@ msgid "This change cannot be saved: {reason}" msgstr "" #: crates/app/src/ui/error_text/operation.rs +#: crates/app/src/ui/error_text/statement.rs #, rust-format msgid "This filter cannot be applied: {reason}" msgstr "" -#: crates/app/src/ui/error_text/operation.rs -#, rust-format -msgid "This structure change cannot be written: {reason}" -msgstr "" - #: crates/app/src/ui/error_text/operation.rs #, rust-format msgid "This page of the table cannot be read: {reason}" @@ -1809,6 +1862,10 @@ msgstr "" msgid "The keyring reported: {detail}" msgstr "" +#: crates/app/src/ui/error_text/secret.rs +msgid "The keyring failed without giving a reason." +msgstr "" + #: crates/app/src/ui/error_text/ssh.rs #, rust-format msgid "" @@ -1918,6 +1975,142 @@ msgstr "" msgid "SSH failed: {detail}" msgstr "" +#: crates/app/src/ui/error_text/statement.rs +msgid "a value does not match its column's type" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "the database did not report the type of column {column}" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "a value could not be read from the server, so it cannot be written back" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "this database cannot take one of the values" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "it names column {column}, which the table does not have" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "a rule has no value" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "a between rule needs both of its values" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "a one-of rule needs at least one value" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "a rule's value does not fit its operator" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "This filter cannot be applied. Value for {column}: {reason}" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "column {column} does not allow one of its operators" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "This SQL filter cannot be applied. {reason}" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the table has no columns to select" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the sort names a column the table does not have" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "this position cannot be reached by this way of paging" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the row to page from has an empty key" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the row to page from has no address that can be read" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the page came back with other columns than were asked for" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the query could not be built" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +#, rust-format +msgid "the row key names column {column}, which the table does not have" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the row key does not have one value for each key column" +msgstr "" + +#: crates/app/src/ui/error_text/statement.rs +msgid "the change could not be written as SQL" +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "Enter a type." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "Remove the comment. It would hide the rest of the statement." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "This database does not read this as one type." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "Enter an expression." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "This database does not read this as one expression." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "This database has no way to write this value in SQL." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "" +"The value could not be read from the server, so it cannot be written back." +msgstr "" + +#: crates/app/src/ui/error_text/syntax.rs +#, rust-format +msgid "Use at most {max} character." +msgid_plural "Use at most {max} characters." +msgstr[0] "" +msgstr[1] "" + +#: crates/app/src/ui/error_text/syntax.rs +msgid "" +"Remove the backslash. This database reads it differently from one session to " +"another." +msgstr "" + #: crates/app/src/ui/error_text/tls.rs msgid "" "The server's certificate was signed by an authority this machine does not " @@ -1976,6 +2169,72 @@ msgstr "" msgid "Set TLS Server Name" msgstr "" +#: crates/app/src/ui/error_text/unsupported.rs +msgid "" +"This database is reached over the network. It cannot open a database file." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "" +"This database opens a file. It cannot connect to a server over the network." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "This database cannot turn foreign key checks off for a save." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +#, rust-format +msgid "This needs {version} or later." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "COPY FROM STDIN cannot run in the editor." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "COPY TO STDOUT cannot run in the editor." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "" +"This time falls in the hour the session's time zone repeats, so the database " +"cannot store it as one instant." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "This database has no times outside one day." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "This database has no times with an offset." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "This database has no intervals." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +msgid "This database has no bit strings." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +#, rust-format +msgid "This database has no numbers wider than {digits} digits." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +#, rust-format +msgid "" +"This database has no NaN, no infinity and no decimals wider than {digits} " +"digits." +msgstr "" + +#: crates/app/src/ui/error_text/unsupported.rs +#, rust-format +msgid "This database has no dates outside the years {first} to {last}." +msgstr "" + #: crates/app/src/ui/export/dialog.rs #, rust-format msgid "" @@ -2916,7 +3175,11 @@ msgstr "" #: crates/app/src/ui/import_dialog.rs #, rust-format -msgid "The file couldn't be read as CSV: {error}" +msgid "The file couldn't be read as CSV at line {line}." +msgstr "" + +#: crates/app/src/ui/import_dialog.rs +msgid "The file couldn't be read as CSV." msgstr "" #: crates/app/src/ui/import_dialog.rs @@ -2930,6 +3193,11 @@ msgid "" "the delimiter." msgstr "" +#: crates/app/src/ui/import_dialog.rs +#, rust-format +msgid "Column {n}" +msgstr "" + #: crates/app/src/ui/import_dialog.rs msgid "NULL" msgstr ""