fix(linux-sqlite): tell a locked file apart from a full server (#3585)

Author: datlechin

linux/ROADMAP.md

@@ -182,7 +182,7 @@ What has to hold before the first release, for a user on GNOME 50 with Ubuntu 26
 | 2 | Connects to their everyday PostgreSQL, MySQL, MariaDB, SQL Server, ClickHouse or SQLite database | The supported releases and why others are out, in the README's [supported servers](README.md#supported-servers). CI runs every driver's Docker suite on each tested release, MySQL 8.0 and SQL Server 2017 and 2019 among them | Nothing |
 | 3 | Browses tables of real-world data (dates, decimals, JSON, UUIDs, NULLs) and sees correct values | Per-engine decoding with the type fallback and unsupported-type conformance cases ([ADR 0014](docs/decisions/0014-lossless-values-and-dialects.md)). A value corpus on every engine, MariaDB too: dates and times at their extremes and in other zones, decimals, JSON, UUIDs, Unicode and binary values, read back through a table tab, Export All and an editor run | Nothing |
 | 4 | Runs typical queries against tables of a million rows or more without running out of memory or freezing | The 10,000-row editor limit with Export All, and driver tests over 2,000,000 rows. Pages seek past the edge row's key, or walk PostgreSQL heap block windows for a table with no key; OFFSET only under a user sort, a restored deep offset or a partitioned keyless table. Docker tests page 60k and 25k-row tables both ways and check the plan. A 1,000,000-row table is paged to its end and exported on PostgreSQL, MySQL, MariaDB, SQL Server, ClickHouse and SQLite, with bounds on time and on resident peak growth. SQLite's case runs in the unit suite in a test binary of its own, so the peak is its alone | Nothing |
-| 5 | Edits data in place, and the changes commit correctly even when the connection or the app misbehaves | Write, row guard and commit resolution conformance on every engine, with the fault fakes and the grid save Docker test. `kill_during_save` kills TablePro halfway through a SQLite save: the file keeps none of it, and the next launch restores the session with its history, settings and saved tabs | A save into a SQLite file another program holds locked fails as `DriverError::Busy`, whose message says the server has no room for another connection |
+| 5 | Edits data in place, and the changes commit correctly even when the connection or the app misbehaves | Write, row guard and commit resolution conformance on every engine, with the fault fakes and the grid save Docker test. `kill_during_save` kills TablePro halfway through a SQLite save: the file keeps none of it, and the next launch restores the session with its history, settings and saved tabs. A save into a SQLite file another program holds locked fails as `Busy(FileLocked)`, which says another program or an editor tab holds the file's lock, and leaves the file as it was whether the lock shows at `BEGIN IMMEDIATE` or at the `COMMIT` | Nothing |
 | 6 | Trusts the app with credentials: TLS, SSH tunnels, no password in plain text | Verify full by default, the TLS conformance suite, the Secret Service, and statement logging silenced | An outside security review |
 | 7 | Recovers when the network drops or the database restarts | Health monitoring, backoff and replay rules with their tests, a stopped container in the liveness tests, and a session test that restarts PostgreSQL: the read in flight runs again, the save in flight does not and leaves no row | Nothing |
 | 8 | Works with a screen reader, the keyboard alone and larger text | Label audits of the picker, the connect pages, the workspace, the connect dialog and preferences, labelled cells and real-key tests. Sorting from the cell menu, named filter fields and cell editors, focus handed on when a control hides, and announced query results and errors. Columns resize from the keyboard | A column resized from the keyboard comes back with expand on (`crates/app/src/ui/grid/column_factory.rs`), so it can draw wider than its saved width; the manual pass in the backlog |

linux/crates/app/src/ui/error_text/driver.rs

@@ -1,5 +1,5 @@
 use tablepro_core::{
-    ConfigError, DriverError, ErrorCategory, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase,
+    BusyReason, ConfigError, DriverError, ErrorCategory, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase,
     TransportError,
 };
 
@@ -32,10 +32,15 @@ pub fn driver_message(error: &DriverError) -> String {
         ErrorCategory::WriteOutcome => write_outcome_message(error),
         ErrorCategory::File => file_message(error),
         ErrorCategory::Busy => match error {
-            DriverError::EditorSessionBusy => {
-                crate::i18n::gettext("This tab is still running a statement. Stop it or wait for it to finish.")
-            }
-            _ => crate::i18n::gettext("The server has no room for another connection right now. Try again shortly."),
+            DriverError::Busy(reason) => match reason {
+                BusyReason::NoFreeConnection => {
+                    crate::i18n::gettext("The server has no room for another connection right now. Try again shortly.")
+                }
+                BusyReason::FileLocked => crate::i18n::gettext(
+                    "Another program, or an editor tab with an open transaction, holds a lock on this database file. Try again when it has finished.",
+                ),
+            },
+            _ => crate::i18n::gettext("This tab is still running a statement. Stop it or wait for it to finish."),
         },
         ErrorCategory::Internal => internal_message(error),
     }

linux/crates/app/src/ui/error_text/tests.rs

@@ -3,7 +3,7 @@ use std::path::PathBuf;
 use tablepro_core::credentials::{SecretError, SecretKind};
 use tablepro_core::sql_syntax::script::{OpenConstruct, ScriptDiagnostic};
 use tablepro_core::{
-    ConfigError, DriverError, ErrorCategory, LossPhase, NetworkEndpoint, ReadOnlyRefusal, ServerCode,
+    BusyReason, ConfigError, DriverError, ErrorCategory, LossPhase, NetworkEndpoint, ReadOnlyRefusal, ServerCode,
     ServerDiagnostics, SshDestinationPart, SshDestinationProblem, SshFailure, StatementOutcome, TimeoutPhase,
     TlsFailure, TransportError,
 };
@@ -69,7 +69,7 @@ fn one_per_category() -> Vec<(ErrorCategory, DriverError)> {
                 path: PathBuf::from("/srv/app.db"),
             },
         ),
-        (ErrorCategory::Busy, DriverError::Busy),
+        (ErrorCategory::Busy, DriverError::Busy(BusyReason::NoFreeConnection)),
         (ErrorCategory::Internal, DriverError::Protocol("bad frame".to_owned())),
     ]
 }
@@ -195,11 +195,37 @@ fn a_refused_login_is_not_called_a_wrong_password() {
 #[test]
 fn busy_and_editor_busy_messages_differ() {
     assert_ne!(
-        driver_message(&DriverError::Busy),
+        driver_message(&DriverError::Busy(BusyReason::NoFreeConnection)),
         driver_message(&DriverError::EditorSessionBusy)
     );
 }
 
+#[test]
+fn a_locked_file_names_the_lock_not_a_full_server() {
+    let locked = driver_message(&DriverError::Busy(BusyReason::FileLocked));
+    let full = driver_message(&DriverError::Busy(BusyReason::NoFreeConnection));
+
+    assert_eq!(
+        full,
+        "The server has no room for another connection right now. Try again shortly."
+    );
+    assert!(locked.contains("lock on this database file"), "{locked}");
+    assert!(!locked.contains("server"), "{locked}");
+    assert_ne!(locked, driver_message(&DriverError::EditorSessionBusy));
+}
+
+#[test]
+fn a_save_whose_commit_met_a_lock_says_why_and_that_nothing_changed() {
+    let locked = DriverError::Busy(BusyReason::FileLocked);
+    let message = driver_message(&DriverError::RolledBack {
+        statement_index: 1,
+        source: Box::new(locked.clone()),
+    });
+
+    assert!(message.contains(&driver_message(&locked)), "{message}");
+    assert!(message.contains("no rows were changed"), "{message}");
+}
+
 #[test]
 fn file_not_found_names_path() {
     let message = driver_message(&DriverError::FileNotFound {
@@ -447,8 +473,8 @@ fn operation_errors_cancelled_and_closed_differ() {
         operation_error_message(&OperationError::Closed)
     );
     assert_eq!(
-        operation_error_message(&OperationError::Driver(DriverError::Busy)),
-        driver_message(&DriverError::Busy)
+        operation_error_message(&OperationError::Driver(DriverError::Busy(BusyReason::NoFreeConnection))),
+        driver_message(&DriverError::Busy(BusyReason::NoFreeConnection))
     );
 }
 

linux/crates/app/src/workspace/save_outcome.rs

@@ -57,7 +57,9 @@ impl SaveOutcome {
 mod tests {
     use tablepro_core::ddl::BuildDdlError;
     use tablepro_core::dml::BuildSqlError;
-    use tablepro_core::{CommitToken, LossPhase, ReadOnlyRefusal, StatementOutcome, TimeoutPhase, TransportError};
+    use tablepro_core::{
+        BusyReason, CommitToken, LossPhase, ReadOnlyRefusal, StatementOutcome, TimeoutPhase, TransportError,
+    };
     use tablepro_session::runtime::TaskPanic;
 
     use super::*;
@@ -145,7 +147,8 @@ mod tests {
             SaveOutcome::Unknown(UnknownCause::Interrupted)
         );
         for refused in [
-            DriverError::Busy,
+            DriverError::Busy(BusyReason::NoFreeConnection),
+            DriverError::Busy(BusyReason::FileLocked),
             DriverError::server("syntax error"),
             DriverError::auth(None),
             DriverError::Transport(TransportError::NameResolution {

linux/crates/app/src/workspace/workspace_handle.rs

@@ -629,7 +629,7 @@ fn failing_source(plan: &SavePlan, changes: &TrackedChanges, error: &DriverError
 mod tests {
     use tablepro_core::edit::CellInput;
     use tablepro_core::meta::TableRef;
-    use tablepro_core::{CommitToken, LossPhase, Value};
+    use tablepro_core::{BusyReason, CommitToken, LossPhase, Value};
     use tablepro_test_support::{CallCounters, ConnectionScript, FakeReply};
 
     use super::*;
@@ -850,7 +850,7 @@ mod tests {
     fn an_error_that_names_no_statement_flashes_nothing() {
         let (plan, changes, _) = planned();
 
-        assert!(failing_source(&plan, &changes, &DriverError::Busy).is_none());
+        assert!(failing_source(&plan, &changes, &DriverError::Busy(BusyReason::NoFreeConnection)).is_none());
         let past_the_end = DriverError::RowGuardFailed {
             statement_index: 7,
             expected: 1,

linux/crates/core/src/busy_reason.rs

@@ -0,0 +1,22 @@
+use std::fmt;
+
+/// What was busy. The app acts on both the same way, so this is a
+/// value it words rather than a variant of its own.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub enum BusyReason {
+    /// The server has no room for another connection, or the pool had
+    /// no free slot in time.
+    NoFreeConnection,
+    /// Another connection to the database file held the lock a
+    /// statement needed until the lock timeout ran out.
+    FileLocked,
+}
+
+impl fmt::Display for BusyReason {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(match self {
+            Self::NoFreeConnection => "the server has no room for another connection",
+            Self::FileLocked => "another connection holds a lock on the database file",
+        })
+    }
+}

linux/crates/core/src/error.rs

@@ -2,6 +2,7 @@ use std::path::PathBuf;
 
 use thiserror::Error;
 
+use crate::busy_reason::BusyReason;
 use crate::commit::CommitToken;
 use crate::config_error::ConfigError;
 use crate::error_category::ErrorCategory;
@@ -119,8 +120,8 @@ pub enum DriverError {
     #[error("could not write {}: {kind}", .path.display())]
     FileIo { path: PathBuf, kind: std::io::ErrorKind },
 
-    #[error("the server has no room for another connection")]
-    Busy,
+    #[error("{0}")]
+    Busy(BusyReason),
 
     #[error("the editor session is already running a statement")]
     EditorSessionBusy,
@@ -160,7 +161,7 @@ impl DriverError {
             | Self::FileAccessDenied { .. }
             | Self::NotADatabase { .. }
             | Self::FileIo { .. } => ErrorCategory::File,
-            Self::Busy | Self::EditorSessionBusy => ErrorCategory::Busy,
+            Self::Busy(_) | Self::EditorSessionBusy => ErrorCategory::Busy,
             Self::Config(_) | Self::Unsupported { .. } => ErrorCategory::Configuration,
             Self::GuardUnsupported { .. } | Self::Decode { .. } | Self::Protocol(_) => ErrorCategory::Internal,
         }
@@ -266,6 +267,17 @@ mod tests {
         assert_ne!(refused, DriverError::auth(None));
     }
 
+    #[test]
+    fn a_locked_file_is_busy_but_not_a_full_server() {
+        let locked = DriverError::Busy(BusyReason::FileLocked);
+        let full = DriverError::Busy(BusyReason::NoFreeConnection);
+
+        assert_eq!(locked.category(), ErrorCategory::Busy);
+        assert_eq!(full.category(), ErrorCategory::Busy);
+        assert!(!locked.to_string().contains("server"), "{locked}");
+        assert_ne!(locked.to_string(), full.to_string());
+    }
+
     #[test]
     fn an_unreadable_column_is_ours_to_fix_not_the_servers() {
         let error = DriverError::Decode {

linux/crates/core/src/lib.rs

@@ -1,5 +1,6 @@
 mod auth_mode;
 pub mod browse;
+mod busy_reason;
 mod call_options;
 mod capabilities;
 mod certificate_file_problem;
@@ -77,6 +78,7 @@ mod write_ledger;
 mod write_report;
 
 pub use auth_mode::AuthMode;
+pub use busy_reason::BusyReason;
 pub use call_options::CallOptions;
 pub use capabilities::{
     CommitResolutionSupport, DriverCapabilities, EndpointKind, EstimateQuality, ReadOnlyEnforcement, ScriptExecution,

linux/crates/drivers/clickhouse/src/errors.rs

@@ -3,7 +3,8 @@ use std::io;
 
 use clickhouse::error::Error as ClickHouseError;
 use tablepro_core::{
-    DriverError, LossPhase, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase, TlsFailure, TransportError,
+    BusyReason, DriverError, LossPhase, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase, TlsFailure,
+    TransportError,
 };
 
 /// Codes the app acts on. Everything else is the server's own answer,
@@ -106,7 +107,9 @@ fn server_error(message: &str, context: ErrorContext) -> DriverError {
             phase: TimeoutPhase::Statement,
             server_cancelled: true,
         },
-        Some(code::TOO_MANY_SIMULTANEOUS_QUERIES | code::NO_FREE_CONNECTION) => DriverError::Busy,
+        Some(code::TOO_MANY_SIMULTANEOUS_QUERIES | code::NO_FREE_CONNECTION) => {
+            DriverError::Busy(BusyReason::NoFreeConnection)
+        }
         _ => DriverError::reported(diagnostics),
     }
 }

linux/crates/drivers/mssql/src/errors.rs

@@ -1,5 +1,5 @@
 use deadpool::managed::{PoolError, TimeoutType};
-use tablepro_core::{DriverError, LossPhase, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase};
+use tablepro_core::{BusyReason, DriverError, LossPhase, ReadOnlyRefusal, ServerCode, ServerDiagnostics, TimeoutPhase};
 use tiberius::error::Error;
 
 use crate::tds_message::TdsMessage;
@@ -47,7 +47,7 @@ pub(crate) fn server_error(message: &TdsMessage) -> DriverError {
             server_cancelled: true,
         },
         // The server's user connection limit.
-        17809 => DriverError::Busy,
+        17809 => DriverError::Busy(BusyReason::NoFreeConnection),
         _ => DriverError::reported(diagnostics),
     }
 }
@@ -81,7 +81,7 @@ pub(crate) fn ends_connection(error: &DriverError) -> bool {
 /// busy, which is the server having no room rather than a dead link.
 pub(crate) fn map_pool_error(error: PoolError<DriverError>) -> DriverError {
     match error {
-        PoolError::Timeout(TimeoutType::Wait) => DriverError::Busy,
+        PoolError::Timeout(TimeoutType::Wait) => DriverError::Busy(BusyReason::NoFreeConnection),
         PoolError::Timeout(TimeoutType::Create) => DriverError::Timeout {
             phase: TimeoutPhase::Login,
             server_cancelled: false,
@@ -142,7 +142,7 @@ mod tests {
             (
                 17809,
                 "Could not connect because the maximum number of user connections",
-                |e| matches!(e, DriverError::Busy),
+                |e| matches!(e, DriverError::Busy(BusyReason::NoFreeConnection)),
             ),
             (208, "Invalid object name 't'.", |e| matches!(e, DriverError::Server(_))),
             (1205, "Transaction was deadlocked", |e| {
@@ -157,7 +157,10 @@ mod tests {
         for (number, text, expected) in cases {
             let mapped = server_error(&message(number, text));
             assert!(expected(&mapped), "{number}: {mapped:?}");
-            if !matches!(mapped, DriverError::Timeout { .. } | DriverError::Busy) {
+            if !matches!(
+                mapped,
+                DriverError::Timeout { .. } | DriverError::Busy(BusyReason::NoFreeConnection)
+            ) {
                 assert_eq!(tds_number(&mapped), Some(number), "{number} kept its number");
             }
         }
@@ -225,7 +228,10 @@ mod tests {
 
     #[test]
     fn a_pool_wait_that_runs_out_is_busy() {
-        assert_eq!(map_pool_error(PoolError::Timeout(TimeoutType::Wait)), DriverError::Busy);
+        assert_eq!(
+            map_pool_error(PoolError::Timeout(TimeoutType::Wait)),
+            DriverError::Busy(BusyReason::NoFreeConnection)
+        );
         assert!(matches!(
             map_pool_error(PoolError::Closed),
             DriverError::ConnectionLost { .. }