From 27e59d1dd4cec7551f09ded028cde0bea98e7162 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Thu, 10 Sep 2026 10:06:26 -0400 Subject: [PATCH 1/4] fix: surface real PostgreSQL error messages instead of generic "db error" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tokio_postgres::Error's own Display impl prints the literal string "db error" for any server-side error (its Kind::Db arm) — the real message lives in the wrapped DbError, reachable via as_db_error(). Every query/execute call site was stringifying the outer error directly, so every query failure (syntax error, missing column, constraint violation) surfaced as the unhelpful "db error" instead of PostgreSQL's actual message. Ports format_pg_error from the built-in driver (src-tauri/src/drivers/postgres/client.rs), which checks as_db_error() first and falls back to e.to_string() otherwise, and applies it at every tokio_postgres::Error call site in client.rs and handlers/query.rs. Fixes #66 --- src/client.rs | 28 +++++++++++++++++++++------- src/handlers/query.rs | 4 ++-- tests/live_db.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/client.rs b/src/client.rs index 9fe7e9c..b964650 100644 --- a/src/client.rs +++ b/src/client.rs @@ -31,6 +31,20 @@ use crate::models::ConnectionParams; static POOLS: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); +/// Format a `tokio_postgres::Error` the way the built-in driver does: for a +/// server-side `DbError` (syntax errors, constraint violations, etc.) surface +/// the real severity/message instead of the generic `Kind::Db` "db error" +/// string that `tokio_postgres::Error`'s own `Display` impl produces. +pub(crate) fn format_pg_error(e: &tokio_postgres::Error) -> String { + if let Some(db) = e.as_db_error() { + let brief = format!("{}: {}", db.severity(), db.message()); + let detail = format!("{e:#?}"); + format!("{brief}\n\n{detail}") + } else { + e.to_string() + } +} + /// Build a connection pool from the given params and verify connectivity /// by acquiring one client and running `SELECT 1`. pub async fn test_connection(params: &ConnectionParams) -> Result<(), String> { @@ -42,7 +56,7 @@ pub async fn test_connection(params: &ConnectionParams) -> Result<(), String> { client .query_one("SELECT 1", &[]) .await - .map_err(|e| format!("Query failed: {e}"))?; + .map_err(|e| format_pg_error(&e))?; Ok(()) } @@ -62,7 +76,7 @@ pub async fn query_strings( let rows = client .query(query, query_params) .await - .map_err(|e| format!("Query failed: {e}"))?; + .map_err(|e| format_pg_error(&e))?; let results = rows .iter() @@ -85,7 +99,7 @@ pub async fn query_rows( client .query(query, query_params) .await - .map_err(|e| format!("Query failed: {e}")) + .map_err(|e| format_pg_error(&e)) } /// Execute a statement with explicit per-placeholder wire types, pinned via @@ -106,12 +120,12 @@ pub async fn execute_typed( let stmt = client .prepare_typed(query, &types) .await - .map_err(|e| format!("Prepare failed: {e}"))?; + .map_err(|e| format_pg_error(&e))?; let values: Vec<&(dyn ToSql + Sync)> = typed_params.iter().map(|(v, _)| *v).collect(); client .execute(&stmt, &values) .await - .map_err(|e| format!("Execute failed: {e}")) + .map_err(|e| format_pg_error(&e)) } /// Run a SELECT with explicit per-placeholder wire types (same rationale as @@ -130,12 +144,12 @@ pub async fn query_typed( let stmt = client .prepare_typed(query, &types) .await - .map_err(|e| format!("Prepare failed: {e}"))?; + .map_err(|e| format_pg_error(&e))?; let values: Vec<&(dyn ToSql + Sync)> = typed_params.iter().map(|(v, _)| *v).collect(); client .query(&stmt, &values) .await - .map_err(|e| format!("Query failed: {e}")) + .map_err(|e| format_pg_error(&e)) } /// Fetch data types for every column in a table as a name -> type map. diff --git a/src/handlers/query.rs b/src/handlers/query.rs index 5eeacc9..4f78aee 100644 --- a/src/handlers/query.rs +++ b/src/handlers/query.rs @@ -154,7 +154,7 @@ async fn exec_query_on_client( let affected = pg_client .execute(query, &[]) .await - .map_err(|e| format!("{e}"))?; + .map_err(|e| client::format_pg_error(&e))?; return Ok(json!({ "columns": [], "rows": [], @@ -177,7 +177,7 @@ async fn exec_query_on_client( let rows = pg_client .query(&final_query, &[]) .await - .map_err(|e| format!("{e}"))?; + .map_err(|e| client::format_pg_error(&e))?; if rows.is_empty() { // Get columns from the statement if possible diff --git a/tests/live_db.rs b/tests/live_db.rs index ba0249b..77b9628 100644 --- a/tests/live_db.rs +++ b/tests/live_db.rs @@ -147,6 +147,36 @@ fn execute_query_returns_rows_from_live_database() { assert_eq!(rows[0][0], json!(1)); } +// Coverage for #66: `tokio_postgres::Error`'s own `Display` impl prints the +// generic "db error" string for any server-side error (its `Kind::Db` arm), +// throwing away the real message in the wrapped `DbError`. `exec_query_on_client` +// previously stringified the error with `format!("{e}")` directly instead of +// checking `as_db_error()` first, so every query error (a syntax error, a +// missing column, a constraint violation) surfaced as the unhelpful literal +// "db error" — see issue #66. +#[test] +fn query_syntax_error_surfaces_the_real_postgres_message_not_generic_db_error() { + let mut plugin = Plugin::spawn(); + let response = plugin.call( + "execute_query", + json!({ "params": conn_params(), "query": "select foo" }), + ); + let error = response + .get("error") + .and_then(|e| e.get("message")) + .and_then(Value::as_str) + .expect("an invalid query must produce a JSON-RPC error"); + assert_ne!( + error, "db error", + "error message must surface the real PostgreSQL error, not the generic \ + tokio_postgres::Error::Display fallback" + ); + assert!( + error.contains("foo"), + "error message should mention the offending identifier, got: {error}" + ); +} + #[test] fn insert_record_persists_a_row() { let mut plugin = Plugin::spawn(); From cd4388b17a6d6bef6201a43758ab988c4a8ac608 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Thu, 10 Sep 2026 11:16:54 -0400 Subject: [PATCH 2/4] fix: apply the same real-error fix to startup script and blob fetch paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The initial fix only covered the query-execution path. Two more call sites had the identical bug — a raw tokio_postgres::Error stringified directly instead of going through format_pg_error: - startup_script_error wrapped the post_create hook's and preflight's error with format!("Startup script failed: {err}"), so a syntax error in a startup_script (the common failure) collapsed to "Startup script failed: db error". - fetch_blob_bytes's row.try_get(...).map_err(|e| e.to_string()) hits the same Display fallback, though in practice try_get produces a FromSql-kind error here, not a DbError. Verified against the live container: before the fix, a broken startup_script surfaced "Startup script failed: db error"; after, it surfaces the real "syntax error at or near ...". --- src/client.rs | 8 +++++--- src/handlers/blob.rs | 3 ++- tests/live_db.rs | 9 +++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/client.rs b/src/client.rs index b964650..f4ecf25 100644 --- a/src/client.rs +++ b/src/client.rs @@ -377,9 +377,11 @@ async fn build_pool(params: &ConnectionParams) -> Result { /// Format a startup-script execution failure so the surfaced error clearly /// names the startup script as the cause, instead of reading like a bad host -/// or wrong credentials. -fn startup_script_error(err: impl std::fmt::Display) -> String { - format!("Startup script failed: {err}") +/// or wrong credentials. Uses `format_pg_error` so a `DbError` (the common +/// case — a typo in the script) surfaces its real message instead of the +/// generic "db error" fallback. +fn startup_script_error(err: tokio_postgres::Error) -> String { + format!("Startup script failed: {}", format_pg_error(&err)) } /// Build the `post_create` hook that runs the startup script on every new diff --git a/src/handlers/blob.rs b/src/handlers/blob.rs index 25d355a..4583a86 100644 --- a/src/handlers/blob.rs +++ b/src/handlers/blob.rs @@ -96,7 +96,8 @@ async fn fetch_blob_bytes( let rows = client::query_typed(conn_params, &query, &typed_params).await?; let row = rows.first().ok_or_else(|| "Row not found".to_string())?; - row.try_get::<_, Vec>(0).map_err(|e| e.to_string()) + row.try_get::<_, Vec>(0) + .map_err(|e| client::format_pg_error(&e)) } /// Sanity-check `file_path` before spending a DB round-trip on a write that's diff --git a/tests/live_db.rs b/tests/live_db.rs index 77b9628..1b6a3f5 100644 --- a/tests/live_db.rs +++ b/tests/live_db.rs @@ -340,6 +340,15 @@ fn broken_startup_script_fails_fast_with_clear_attribution() { error.starts_with("Startup script failed:"), "error should be clearly attributed to the startup script, got: {error}" ); + // Coverage for #66: startup_script_error previously stringified the + // tokio_postgres::Error directly, so a DbError (a syntax error in the + // script, the common case) collapsed to the generic "db error" instead + // of the real PostgreSQL message. + assert!( + !error.contains("db error") && error.contains("syntax error"), + "error should surface the real PostgreSQL syntax error, not the generic \ + tokio_postgres::Error::Display fallback, got: {error}" + ); } // Coverage for #43: build_pool never called cfg.ssl_mode(...), so From e47396db66b7844c806a842a38546ae2338ea307 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Thu, 10 Sep 2026 12:17:00 -0400 Subject: [PATCH 3/4] fix: apply format_pg_error to the SET search_path call sites too Same bug, third location: both execute_query_batch and exec_query set search_path with a raw format!("Failed to set search_path: {e}") on the tokio_postgres::Error from batch_execute. In practice the identifier is always escaped/quoted before being sent, so a DbError here isn't reachable through normal use, but fix it for consistency since it's the identical pattern as the two already-fixed sites. --- src/handlers/query.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/handlers/query.rs b/src/handlers/query.rs index 4f78aee..3480402 100644 --- a/src/handlers/query.rs +++ b/src/handlers/query.rs @@ -56,7 +56,11 @@ pub async fn execute_query_batch(id: Value, params: &Value) -> Value { if let Some(s) = schema { let set_path = format!("SET search_path TO \"{}\"", s.replace('"', "\"\"")); if let Err(e) = pg_client.batch_execute(&set_path).await { - return error_response(id, -32603, &format!("Failed to set search_path: {e}")); + return error_response( + id, + -32603, + &format!("Failed to set search_path: {}", client::format_pg_error(&e)), + ); } } @@ -136,7 +140,7 @@ async fn exec_query( pg_client .batch_execute(&set_path) .await - .map_err(|e| format!("Failed to set search_path: {e}"))?; + .map_err(|e| format!("Failed to set search_path: {}", client::format_pg_error(&e)))?; } exec_query_on_client(&pg_client, query, limit, page).await From 4b656efc4d743bc6ad559f684c2d043fd71003d9 Mon Sep 17 00:00:00 2001 From: Adam J Esslinger Date: Thu, 10 Sep 2026 12:34:16 -0400 Subject: [PATCH 4/4] fix: apply the real-error fix to connection-establishment failures too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deepest and most impactful gap yet: pool.get()'s connection handshake itself constructs a Kind::Db tokio_postgres::Error for server-rejected connections (bad database name, bad password) — verified from tokio-postgres's own source, connect_raw.rs calls Error::db() on the server's ErrorResponse during startup, same as a query error. Every "Connection failed: {e}" call site was stringifying the outer deadpool_postgres::PoolError directly, which just forwards to the inner error's Display — so a bad database name or wrong password (probably the two most common real-world triggers of #66, more likely than a raw SQL syntax error) surfaced as "Connection failed: Error occurred while creating a new object: db error" instead of the real message. Adds format_pool_error, which unwraps PoolError::Backend and PoolError::PostCreateHook(HookError::Backend(_)) down to the inner tokio_postgres::Error and reuses format_pg_error. Applied at all 5 pool.get() sites in client.rs and both in handlers/query.rs. Verified against the live container: before the fix, both a nonexistent database and a wrong password produced "Connection failed: ... db error"; after, they produce the real "database ... does not exist" / "password authentication failed for user ...". --- src/client.rs | 28 +++++++++++++++++++++------ src/handlers/query.rs | 10 ++++++++-- tests/live_db.rs | 44 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/client.rs b/src/client.rs index f4ecf25..db7bbfd 100644 --- a/src/client.rs +++ b/src/client.rs @@ -45,6 +45,22 @@ pub(crate) fn format_pg_error(e: &tokio_postgres::Error) -> String { } } +/// Format a `pool.get()` failure. The pool's own connection-establishment +/// handshake (bad database name, bad password, ...) surfaces as a +/// `tokio_postgres::Error` wrapped in `PoolError::Backend` — the exact same +/// `Kind::Db`/"db error" pitfall `format_pg_error` exists to avoid, just one +/// layer deeper. `PostCreateHook` wraps the same shape via `HookError`. +pub(crate) fn format_pool_error(e: &deadpool_postgres::PoolError) -> String { + use deadpool_postgres::HookError; + match e { + deadpool_postgres::PoolError::Backend(pg_err) => format_pg_error(pg_err), + deadpool_postgres::PoolError::PostCreateHook(HookError::Backend(pg_err)) => { + format_pg_error(pg_err) + } + other => other.to_string(), + } +} + /// Build a connection pool from the given params and verify connectivity /// by acquiring one client and running `SELECT 1`. pub async fn test_connection(params: &ConnectionParams) -> Result<(), String> { @@ -52,7 +68,7 @@ pub async fn test_connection(params: &ConnectionParams) -> Result<(), String> { let client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pool_error(&e)))?; client .query_one("SELECT 1", &[]) .await @@ -72,7 +88,7 @@ pub async fn query_strings( let client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pool_error(&e)))?; let rows = client .query(query, query_params) .await @@ -95,7 +111,7 @@ pub async fn query_rows( let client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pool_error(&e)))?; client .query(query, query_params) .await @@ -115,7 +131,7 @@ pub async fn execute_typed( let client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pool_error(&e)))?; let types: Vec = typed_params.iter().map(|(_, t)| t.clone()).collect(); let stmt = client .prepare_typed(query, &types) @@ -139,7 +155,7 @@ pub async fn query_typed( let client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pool_error(&e)))?; let types: Vec = typed_params.iter().map(|(_, t)| t.clone()).collect(); let stmt = client .prepare_typed(query, &types) @@ -420,7 +436,7 @@ where let (mut client, connection) = pg_config .connect(tls) .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", format_pg_error(&e)))?; let driver = tokio::spawn(async move { let _ = connection.await; }); diff --git a/src/handlers/query.rs b/src/handlers/query.rs index 3480402..0a0228d 100644 --- a/src/handlers/query.rs +++ b/src/handlers/query.rs @@ -50,7 +50,13 @@ pub async fn execute_query_batch(id: Value, params: &Value) -> Value { }; let pg_client = match pool.get().await { Ok(c) => c, - Err(e) => return error_response(id, -32603, &format!("Connection failed: {e}")), + Err(e) => { + return error_response( + id, + -32603, + &format!("Connection failed: {}", client::format_pool_error(&e)), + ) + } }; if let Some(s) = schema { @@ -132,7 +138,7 @@ async fn exec_query( let pg_client = pool .get() .await - .map_err(|e| format!("Connection failed: {e}"))?; + .map_err(|e| format!("Connection failed: {}", client::format_pool_error(&e)))?; // Set search_path if schema is specified if let Some(s) = schema { diff --git a/tests/live_db.rs b/tests/live_db.rs index 1b6a3f5..c81c736 100644 --- a/tests/live_db.rs +++ b/tests/live_db.rs @@ -370,3 +370,47 @@ fn ssl_mode_require_fails_against_a_server_without_tls() { "ssl_mode=require must fail against a server with no TLS, not silently connect over plaintext" ); } + +// Coverage for #66: the connection-establishment handshake itself (bad +// database name, bad password) surfaces as a tokio_postgres::Error wrapped +// in deadpool_postgres::PoolError::Backend, from pool.get() — the same +// Kind::Db/"db error" pitfall as query execution, just one layer deeper. +// Every "Connection failed: {e}" call site previously stringified the +// PoolError directly instead of unwrapping to the inner DbError. +#[test] +fn connecting_to_a_nonexistent_database_surfaces_the_real_postgres_message() { + let mut plugin = Plugin::spawn(); + let mut params = conn_params(); + params["database"] = json!("this_database_does_not_exist_xyz"); + + let response = plugin.call("test_connection", json!({ "params": params })); + let error = response + .get("error") + .and_then(|e| e.get("message")) + .and_then(Value::as_str) + .expect("connecting to a nonexistent database must produce a JSON-RPC error"); + assert!( + !error.contains("db error") && error.contains("does not exist"), + "error should surface the real PostgreSQL message, not the generic \ + tokio_postgres::Error::Display fallback, got: {error}" + ); +} + +#[test] +fn connecting_with_a_wrong_password_surfaces_the_real_postgres_message() { + let mut plugin = Plugin::spawn(); + let mut params = conn_params(); + params["password"] = json!("definitely_wrong_password"); + + let response = plugin.call("test_connection", json!({ "params": params })); + let error = response + .get("error") + .and_then(|e| e.get("message")) + .and_then(Value::as_str) + .expect("a wrong password must produce a JSON-RPC error"); + assert!( + !error.contains("db error") && error.contains("password authentication failed"), + "error should surface the real PostgreSQL message, not the generic \ + tokio_postgres::Error::Display fallback, got: {error}" + ); +}