improve error messages

Author: lovasoa

CHANGELOG.md

@@ -32,6 +32,10 @@
 - Add support for `change_percent` without `description` in the big_number component to display the percentage change of a value.
 - Add support for `freeze_columns` and `freeze_headers` in the table component to freeze columns and headers.
 - Fix an error that occured when the site_prefix configuration option was set to a value containing special characters like `-` and a request was made directly to the server without url-encoding the site prefix.
+- Improve error messages:
+  - The error message now always includes the name of the file where the error occurred, which is useful when embedding SQLPage pages using `sqlpage.run_sql`, or the card component.
+  - When an error occurs while executing a SQL statement, the error message now always includes the (potentially transformed) SQL statement that was sent to the database.
+  - Fixed a problem where database errors would be displayed twice in the error message.
 
 ## 0.30.1 (2024-10-31)
 - fix a bug where table sorting would break if table search was not also enabled.

src/app_config.rs

@@ -561,7 +561,7 @@ mod test {
         assert_eq!(normalize_site_prefix("*-+/:;,?%\"'{"), "/*-+/:;,%3F%%22'{/");
         assert_eq!(
             normalize_site_prefix(
-                &(0..=0x7F).map(|b| char::from(b)).collect::<String>()
+                &(0..=0x7F).map(char::from).collect::<String>()
             ),
             "/%00%01%02%03%04%05%06%07%08%0B%0C%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20!%22%23$%&'()*+,-./0123456789:;%3C=%3E%3F@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~%7F/"
         );

src/file_cache.rs

@@ -139,7 +139,7 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
 
         let parsed = match file_contents {
             Ok(contents) => {
-                let value = T::from_str_with_state(app_state, &contents).await?;
+                let value = T::from_str_with_state(app_state, &contents, path).await?;
                 Ok(Cached::new(value))
             }
             // If a file is not found, we try to load it from the static files
@@ -184,5 +184,6 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
 
 #[async_trait(? Send)]
 pub trait AsyncFromStrWithState: Sized {
-    async fn from_str_with_state(app_state: &AppState, source: &str) -> anyhow::Result<Self>;
-}
+    /// Parses the string into an object.
+    async fn from_str_with_state(app_state: &AppState, source: &str, source_path: &Path) -> anyhow::Result<Self>;
+}

src/lib.rs

@@ -17,7 +17,7 @@ use crate::app_config::AppConfig;
 use crate::filesystem::FileSystem;
 use crate::webserver::database::ParsedSqlFile;
 use file_cache::FileCache;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use templates::AllTemplates;
 use webserver::Database;
 
@@ -47,7 +47,7 @@ impl AppState {
         let file_system = FileSystem::init(&config.web_root, &db).await;
         sql_file_cache.add_static(
             PathBuf::from("index.sql"),
-            ParsedSqlFile::new(&db, include_str!("../index.sql")),
+            ParsedSqlFile::new(&db, include_str!("../index.sql"), Path::new("index.sql")),
         );
         Ok(AppState {
             db,

src/templates.rs

@@ -5,7 +5,7 @@ use crate::{AppState, FileCache, TEMPLATES_DIR};
 use async_trait::async_trait;
 use handlebars::{template::TemplateElement, Handlebars, Template};
 use include_dir::{include_dir, Dir};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
 pub struct SplitTemplate {
@@ -52,7 +52,8 @@ pub fn split_template(mut original: Template) -> SplitTemplate {
 
 #[async_trait(? Send)]
 impl AsyncFromStrWithState for SplitTemplate {
-    async fn from_str_with_state(_app_state: &AppState, source: &str) -> anyhow::Result<Self> {
+    async fn from_str_with_state(_app_state: &AppState, source: &str, source_path: &Path) -> anyhow::Result<Self> {
+        log::debug!("Compiling template {:?}", source_path);
         let tpl = Template::compile_with_name(source, "SQLPage component".to_string())?;
         Ok(split_template(tpl))
     }

src/webserver/database/error_highlighting.rs

@@ -1,19 +1,27 @@
-use std::fmt::Write;
+use std::{
+    fmt::Write,
+    path::{Path, PathBuf},
+};
 
 #[derive(Debug)]
 struct NiceDatabaseError {
+    source_file: PathBuf,
     db_err: sqlx::error::Error,
     query: String,
 }
 
 impl std::fmt::Display for NiceDatabaseError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.db_err.fmt(f)?;
+        write!(
+            f,
+            "In \"{}\": The following error occurred while executing an SQL statement:\n{}\n\nThe SQL statement sent by SQLPage was:\n",
+            self.source_file.display(),
+            self.db_err
+        )?;
         if let sqlx::error::Error::Database(db_err) = &self.db_err {
             let Some(mut offset) = db_err.offset() else {
-                return Ok(());
+                return write!(f, "{}", self.query);
             };
-            write!(f, "\n\nError in query:\n")?;
             for (line_no, line) in self.query.lines().enumerate() {
                 if offset > line.len() {
                     offset -= line.len() + 1;
@@ -23,23 +31,24 @@ impl std::fmt::Display for NiceDatabaseError {
                     break;
                 }
             }
+            Ok(())
         } else {
-            write!(f, "{}", self.query.lines().next().unwrap_or_default())?;
+            write!(f, "{}", self.query)
         }
-        Ok(())
     }
 }
 
-impl std::error::Error for NiceDatabaseError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        self.db_err.source()
-    }
-}
+impl std::error::Error for NiceDatabaseError {}
 
 /// Display a database error with a highlighted line and character offset.
 #[must_use]
-pub fn display_db_error(query: &str, db_err: sqlx::error::Error) -> anyhow::Error {
+pub fn display_db_error(
+    source_file: &Path,
+    query: &str,
+    db_err: sqlx::error::Error,
+) -> anyhow::Error {
     anyhow::Error::new(NiceDatabaseError {
+        source_file: source_file.to_path_buf(),
         db_err,
         query: query.to_string(),
     })

src/webserver/database/execute_queries.rs

@@ -4,6 +4,7 @@ use futures_util::StreamExt;
 use serde_json::Value;
 use std::borrow::Cow;
 use std::collections::HashMap;
+use std::path::Path;
 use std::pin::Pin;
 
 use super::csv_import::run_csv_import;
@@ -34,7 +35,7 @@ impl Database {
             .prepare_with(query, param_types)
             .await
             .map(|s| s.to_owned())
-            .map_err(|e| display_db_error(query, e))
+            .map_err(|e| display_db_error(Path::new("autogenerated sqlpage query"), query, e))
     }
 }
 
@@ -43,6 +44,7 @@ pub fn stream_query_results_with_conn<'a>(
     request: &'a mut RequestInfo,
     db_connection: &'a mut DbConn,
 ) -> impl Stream<Item = DbItem> + 'a {
+    let source_file = &sql_file.source_path;
     async_stream::try_stream! {
         for res in &sql_file.statements {
             match res {
@@ -58,7 +60,7 @@ pub fn stream_query_results_with_conn<'a>(
                     let mut stream = connection.fetch_many(query);
                     while let Some(elem) = stream.next().await {
                         let is_err = elem.is_err();
-                        let mut query_result = parse_single_sql_result(&stmt.query, elem);
+                        let mut query_result = parse_single_sql_result(source_file, &stmt.query, elem);
                         apply_json_columns(&mut query_result, &stmt.json_columns);
                         apply_delayed_functions(request, &stmt.delayed_functions, &mut query_result).await?;
                         for i in parse_dynamic_rows(query_result) {
@@ -80,7 +82,7 @@ pub fn stream_query_results_with_conn<'a>(
                         yield i;
                     }
                 }
-                ParsedStatement::Error(e) => yield DbItem::Error(clone_anyhow_err(e)),
+                ParsedStatement::Error(e) => yield DbItem::Error(clone_anyhow_err(source_file, e)),
             }
         }
     }
@@ -216,7 +218,11 @@ async fn take_connection<'a, 'b>(
 }
 
 #[inline]
-fn parse_single_sql_result(sql: &str, res: sqlx::Result<Either<AnyQueryResult, AnyRow>>) -> DbItem {
+fn parse_single_sql_result(
+    source_file: &Path,
+    sql: &str,
+    res: sqlx::Result<Either<AnyQueryResult, AnyRow>>,
+) -> DbItem {
     match res {
         Ok(Either::Right(r)) => {
             if log::log_enabled!(log::Level::Trace) {
@@ -228,7 +234,7 @@ fn parse_single_sql_result(sql: &str, res: sqlx::Result<Either<AnyQueryResult, A
             log::debug!("Finished query with result: {:?}", res);
             DbItem::FinishedQuery
         }
-        Err(err) => DbItem::Error(display_db_error(sql, err)),
+        Err(err) => DbItem::Error(display_db_error(source_file, sql, err)),
     }
 }
 
@@ -252,8 +258,8 @@ fn debug_row(r: &AnyRow) {
     log::trace!("Received db row: {}", row_str);
 }
 
-fn clone_anyhow_err(err: &anyhow::Error) -> anyhow::Error {
-    let mut e = anyhow!("SQLPage could not parse and prepare this SQL statement");
+fn clone_anyhow_err(source_file: &Path, err: &anyhow::Error) -> anyhow::Error {
+    let mut e = anyhow!("{source_file:?} contains a syntax error preventing SQLPage from parsing and preparing its SQL statements.");
     for c in err.chain().rev() {
         e = e.context(c.to_string());
     }

src/webserver/database/migrations.rs

@@ -35,7 +35,9 @@ pub async fn apply(config: &crate::app_config::AppConfig, db: &Database) -> anyh
         match err {
             MigrateError::Execute(n, source) => {
                 let migration = migrator.iter().find(|&m| m.version == n).unwrap();
-                display_db_error(&migration.sql, source).context(format!(
+                let source_file =
+                    migrations_dir.join(format!("{:04}_{}.sql", n, migration.description));
+                display_db_error(&source_file, &migration.sql, source).context(format!(
                     "Failed to apply migration {}",
                     DisplayMigration(migration)
                 ))

src/webserver/database/sql.rs

@@ -17,38 +17,43 @@ use sqlparser::tokenizer::Token::{SemiColon, EOF};
 use sqlparser::tokenizer::Tokenizer;
 use sqlx::any::AnyKind;
 use std::ops::ControlFlow;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
 #[derive(Default)]
 pub struct ParsedSqlFile {
     pub(super) statements: Vec<ParsedStatement>,
+    pub(super) source_path: PathBuf,
 }
 
 impl ParsedSqlFile {
     #[must_use]
-    pub fn new(db: &Database, sql: &str) -> ParsedSqlFile {
+    pub fn new(db: &Database, sql: &str, source_path: &Path) -> ParsedSqlFile {
         let dialect = dialect_for_db(db.connection.any_kind());
+        log::debug!("Parsing SQL file {:?}", source_path);
         let parsed_statements = match parse_sql(dialect.as_ref(), sql) {
             Ok(parsed) => parsed,
-            Err(err) => return Self::from_err(err),
+            Err(err) => return Self::from_err(err, source_path),
         };
         let statements = parsed_statements.collect();
-        ParsedSqlFile { statements }
+        ParsedSqlFile {
+            statements,
+            source_path: source_path.to_path_buf(),
+        }
     }
 
-    fn from_err(e: impl Into<anyhow::Error>) -> Self {
+    fn from_err(e: impl Into<anyhow::Error>, source_path: &Path) -> Self {
         Self {
-            statements: vec![ParsedStatement::Error(
-                e.into().context("SQLPage could not parse the SQL file"),
-            )],
+            statements: vec![ParsedStatement::Error(e.into().context(format!("While parsing file {source_path:?}")))],
+            source_path: source_path.to_path_buf(),
         }
     }
 }
 
 #[async_trait(? Send)]
 impl AsyncFromStrWithState for ParsedSqlFile {
-    async fn from_str_with_state(app_state: &AppState, source: &str) -> anyhow::Result<Self> {
-        Ok(ParsedSqlFile::new(&app_state.db, source))
+    async fn from_str_with_state(app_state: &AppState, source: &str, source_path: &Path) -> anyhow::Result<Self> {
+        Ok(ParsedSqlFile::new(&app_state.db, source, source_path))
     }
 }
 
@@ -96,7 +101,7 @@ fn parse_sql<'a>(
         .tokenize_with_location()
         .map_err(|err| {
             let location = err.location;
-            anyhow::Error::new(err).context(format!("The SQLPage parser couldn't understand the SQL file. Tokenization failed. Please check for syntax errors:\n{}", quote_source_with_highlight(sql, location.line, location.column)))
+            anyhow::Error::new(err).context(format!("The SQLPage parser could not understand the SQL file. Tokenization failed. Please check for syntax errors:\n{}", quote_source_with_highlight(sql, location.line, location.column)))
         })?;
     let mut parser = Parser::new(dialect).with_tokens_with_locations(tokens);
     let db_kind = kind_of_dialect(dialect);

src/webserver/database/sql_to_json.rs

@@ -408,15 +408,13 @@ mod tests {
             }
             writeln!(
                 &mut comparison_string,
-                "{key:<width$}  actual  : {actual_value:?}\n{key:width$}  expected: {expected_value:?}\n",
-                width = max_key_len
+                "{key:<max_key_len$}  actual  : {actual_value:?}\n{key:max_key_len$}  expected: {expected_value:?}\n"
             )
             .unwrap();
         }
         assert_eq!(
             actual, expected,
-            "JSON objects are not equal:\n\n{}",
-            comparison_string
+            "JSON objects are not equal:\n\n{comparison_string}"
         );
     }
 }