Better error messages (#156)

* chore: adding better error messages * fix * invalid regex pattern error * ignore file * wip: log level * fix * removing unwrap * chore: adding failed to retrieve a connection from the pool * chore: fix default value * wip * chore: adding better error message * add more errors * clippy fix * docs: document connection pool * provide default when no connections provided by file config * remove println
JasonShin · Jul 25, 2024 · 306355a · 306355a
1 parent 6abe29b
commit 306355a
Show file tree

Hide file tree

Showing 13 changed files with 95 additions and 37 deletions.
diff --git a/book/src/api/1.3.configs-file-based.md b/book/src/api/1.3.configs-file-based.md
@@ -38,7 +38,8 @@ Example `.sqlxrc.json`
       "DB_USER": "app_user",
       "DB_PASS": "password",
       "DB_HOST": "127.0.0.1",
-      "DB_PORT": 3307
+      "DB_PORT": 3307,
+      "POOL_SIZE": 20
     }
   }
 }
@@ -73,6 +74,7 @@ Supported fields of each connection include
 - `DB_HOST`: database host (e.g. 127.0.0.1)
 - `DB_PORT`: database port (e.g. 4321)
 - `PG_SEARCH_PATH`: PostgreSQL schema search path (default is "$user,public") [https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH](https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH)
+- `POOL_SIZE`: Size of the connection pool to establish per connection type
 
 ### generate_types
 

diff --git a/dprint.json b/dprint.json
diff --git a/src/common/cli.rs b/src/common/cli.rs
@@ -74,7 +74,7 @@ pub struct Cli {
   #[clap(long, short)]
   pub message_format: Option<String>,
 
-  /// log level to be used for the CLI info > warn > error
+  /// log level to be used for the CLI debug > info > warn > error
   #[clap(value_enum, long)]
   pub log_level: Option<LogLevel>,
 }
diff --git a/src/common/config.rs b/src/common/config.rs
@@ -52,8 +52,12 @@ pub struct DbConnectionConfig {
   pub db_name: Option<String>,
   #[serde(rename = "PG_SEARCH_PATH")]
   pub pg_search_path: Option<String>,
+  #[serde(rename = "POOL_SIZE", default = "default_pool_size")]
+  pub pool_size: u32,
 }
 
+fn default_pool_size() -> u32 { 10 }
+
 /// Config is used to determine connection configurations for primary target Database
 /// It uses 2 sources of config and they are used in following priorities
 /// 1. any configuration via CLI options
@@ -82,6 +86,7 @@ impl Config {
     let file_config_path = &CLI_ARGS.config.clone().unwrap_or(default_config_path);
     let connections = Self::build_configs(&dotenv, file_config_path);
     let generate_types_config = Self::generate_types_config(file_config_path);
+
     let generate_types_config =
       generate_types_config.and_then(|config| if config.enabled { Some(config) } else { None });
     let ignore_patterns = Self::get_ignore_patterns(&default_ignore_config_path);
@@ -103,7 +108,8 @@ impl Config {
       return base_ignore_patterns.to_vec();
     }
 
-    let file_based_ignore_config = &file_based_ignore_config.unwrap();
+    let file_based_ignore_config = &file_based_ignore_config
+        .expect("Failed to read the .sqlxignore file. Please check the contents of the ignore file and try again - https://jasonshin.github.io/sqlx-ts/api/2.ignore-patterns.html");
     let file_based_ignore_config = file_based_ignore_config.split('\n');
     let file_based_ignore_config: Vec<&str> = file_based_ignore_config.clone().collect();
 
@@ -258,6 +264,11 @@ impl Config {
       .or_else(|| dotenv.pg_search_path.clone())
       .or_else(|| default_config.map(|x| x.pg_search_path.clone()).flatten());
 
+    let pool_size = default_config
+      .map(|x| x.pool_size)
+      .or_else(|| Some(default_pool_size()))
+      .unwrap();
+
     DbConnectionConfig {
       db_type: db_type.to_owned(),
       db_host: db_host.to_owned(),
@@ -266,6 +277,7 @@ impl Config {
       db_pass: db_pass.to_owned(),
       db_name: db_name.to_owned(),
       pg_search_path: pg_search_path.to_owned(),
+      pool_size,
     }
   }
 
@@ -329,7 +341,6 @@ impl Config {
     )
   }
 
-  // TODO: update this to also factor in env variable
   pub fn get_log_level(file_config_path: &PathBuf) -> LogLevel {
     let file_based_config = fs::read_to_string(file_config_path);
     let file_based_config = &file_based_config.map(|f| serde_json::from_str::<SqlxConfig>(f.as_str()).unwrap());

diff --git a/src/common/errors.rs b/src/common/errors.rs
@@ -0,0 +1,6 @@
+
+// Common error messages
+pub static DB_CONN_POOL_RETRIEVE_ERROR: &str = "Failed to retrieve a connection from the pool. Increase the pool size and try again";
+pub static DB_SCHEME_READ_ERROR: &str = "Failed to read the database schema to retrieve details. Please raise the issue on https://github.com/JasonShin/sqlx-ts/issues";
+pub static DB_CONN_FROM_LOCAL_CACHE_ERROR: &str = "Failed to retrieve a connection from local cache, check the database name annotated in your query and connections config in your configuration file";
+
diff --git a/src/common/lazy.rs b/src/common/lazy.rs
@@ -19,29 +19,35 @@ lazy_static! {
     // This is a holder for shared DBSChema used to fetch information for information_schema table
     // By having a singleton, we can think about caching the result if we are fetching a query too many times
     pub static ref DB_SCHEMA: Arc<Mutex<DBSchema>> = Arc::new(Mutex::new(DBSchema::new()));
+    pub static ref ERR_DB_CONNECTION_ISSUE: String = "Unable to connect to the database, please check the connection configuration again https://jasonshin.github.io/sqlx-ts/api/1.connecting-to-db.html".to_string();
 
     // This variable holds database connections for each connection name that is defined in the config
     // We are using lazy_static to initialize the connections once and use them throughout the application
     static ref DB_CONN_CACHE: HashMap<String, Arc<Mutex<DBConn>>> = {
         let mut cache = HashMap::new();
         for connection in CONFIG.connections.keys() {
-            let connection_config = CONFIG.connections.get(connection).unwrap();
+            let connection_config = CONFIG.connections.get(connection)
+                .unwrap_or_else(|| panic!("Failed to find a correct connection from the configuration - {connection}"));
             let db_type = connection_config.db_type.to_owned();
             let conn = match db_type {
                 DatabaseType::Mysql => {
                     task::block_in_place(|| Handle::current().block_on(async {
                         let mysql_cred = CONFIG.get_mysql_cred_str(connection_config);
                         let mysql_cred = mysql_cred.as_str();
                         let manager = MySqlConnectionManager::new(mysql_cred.to_string());
-                        let pool = bb8::Pool::builder().max_size(10).build(manager).await.unwrap();
+                        let pool = bb8::Pool::builder().max_size(connection_config.pool_size).build(manager)
+                            .await
+                            .expect(&ERR_DB_CONNECTION_ISSUE);
                         DBConn::MySQLPooledConn(Mutex::new(pool))
                     }))
                 }
                 DatabaseType::Postgres => {
                     task::block_in_place(|| Handle::current().block_on(async {
                         let postgres_cred = CONFIG.get_postgres_cred(connection_config);
                         let manager = PostgresConnectionManager::new(postgres_cred);
-                        let pool = bb8::Pool::builder().max_size(10).build(manager).await.unwrap();
+                        let pool = bb8::Pool::builder().max_size(10).build(manager)
+                            .await
+                            .expect(&ERR_DB_CONNECTION_ISSUE);
                         let db_conn = DBConn::PostgresConn(Mutex::new(pool));
 
                         let conn = match &db_conn {
@@ -53,8 +59,15 @@ lazy_static! {
                             let search_path_query = format!("SET search_path TO {}", &connection_config.pg_search_path.clone().unwrap().as_str());
                             {
                                 let conn = conn.lock().await;
-                                let conn = conn.get().await.unwrap();
-                                conn.execute(&search_path_query, &[]).await.unwrap();
+                                let conn = conn
+                                    .get()
+                                    .await
+                                    .expect(&ERR_DB_CONNECTION_ISSUE);
+
+                                let ERR_SEARCH_PATH_QUERY = format!("Failed to execute the search_path query {:?}", search_path_query.as_str());
+                                let ERR_SEARCH_PATH_QUERY = ERR_SEARCH_PATH_QUERY.as_str();
+                                conn.execute(&search_path_query, &[]).await
+                                    .expect(ERR_SEARCH_PATH_QUERY);
                             }
                         }
                         db_conn

diff --git a/src/common/logger.rs b/src/common/logger.rs
@@ -1,6 +1,31 @@
 // TODO: Add documentation including examples
 // TODO: Use SQLX_TS_LOG env var to set log level
 
+macro_rules! debug {
+    ($arg:tt) => ({
+        use crate::common::lazy::CONFIG;
+        use crate::common::types::LogLevel;
+
+        if CONFIG.log_level.gte(&LogLevel::Debug) {
+            use colored::*;
+            let level = "[DEBUG]".white();
+            let message = $arg;
+            println!("{level} {message}")
+        }
+    });
+    ($arg:tt, $($arg2:tt)*) => ({
+        use crate::common::lazy::CONFIG;
+        use crate::common::types::LogLevel;
+        if CONFIG.log_level.gte(&LogLevel::Debug) {
+            use colored::*;
+            let level = "[DEBUG]".white();
+            let message = format!("{}", format!($arg, $($arg2)*));
+            println!("{level} {message}")
+        }
+    });
+}
+
+
 macro_rules! info {
     ($arg:tt) => ({
         use crate::common::lazy::CONFIG;
@@ -74,3 +99,4 @@ macro_rules! error {
         }
     });
 }
+
diff --git a/src/common/mod.rs b/src/common/mod.rs
@@ -9,6 +9,8 @@ pub mod dotenv;
 pub mod lazy;
 pub mod types;
 
+pub mod errors;
+
 // Source Parser
 #[allow(clippy::upper_case_acronyms)]
 #[derive(Debug, Clone)]

diff --git a/src/common/types.rs b/src/common/types.rs
@@ -44,6 +44,7 @@ impl NamingConvention {
 #[derive(ValueEnum, Debug, Clone, Serialize, Deserialize, Copy)]
 #[serde(rename_all = "lowercase")]
 pub enum LogLevel {
+  Debug = 4,
   Info = 3,
   Warning = 2,
   Error = 1,

diff --git a/src/core/connection.rs b/src/core/connection.rs
@@ -10,7 +10,7 @@ use tokio::sync::Mutex;
 
 use color_eyre::Result;
 use swc_common::errors::Handler;
-
+use crate::common::errors::DB_CONN_FROM_LOCAL_CACHE_ERROR;
 use super::mysql::pool::MySqlConnectionManager;
 use super::postgres::pool::PostgresConnectionManager;
 
@@ -51,7 +51,7 @@ impl<'a> DBConnections<'a> {
     let conn = self
       .cache
       .get(db_conn_name)
-      .expect("Failed to get the connection from cache");
+      .expect(DB_CONN_FROM_LOCAL_CACHE_ERROR);
     conn.to_owned()
   }
 }
diff --git a/src/core/mysql/prepare.rs b/src/core/mysql/prepare.rs
@@ -27,7 +27,8 @@ pub async fn prepare(
     let explain_query = format!("PREPARE stmt FROM \"{}\"", sql.query);
     let span = sql.span.to_owned();
     let conn = conn.lock().await;
-    let mut conn = conn.get().await.unwrap();
+    let mut conn = conn.get().await
+      .expect("Failed to retrieve a connection from the pool. Consider increasing the connection pool size");
     let result = conn.query::<Row, String>(explain_query).await;
 
     if let Err(err) = result {

diff --git a/src/scan_folder.rs b/src/scan_folder.rs
@@ -2,24 +2,31 @@ use std::path::{Path, PathBuf};
 
 use crate::common::lazy::CONFIG;
 use crate::common::types::JsExtension;
-use regex::Regex;
+use regex::{Regex, Error as RegexError};
 use walkdir::WalkDir;
 
-fn pattern_to_regex(pattern: &str) -> Regex {
+fn pattern_to_regex(pattern: &str) -> Result<Regex, RegexError> {
   let pattern = pattern.replace('.', "\\.");
   let pattern = pattern.replace('*', ".*");
   let pattern = format!("^{}$", pattern);
-  Regex::new(&pattern).unwrap()
+  Regex::new(&pattern)
 }
 
 fn is_match(pattern: &str, path: &Path) -> bool {
   let regex = pattern_to_regex(pattern);
 
-  if pattern.starts_with('!') {
-    !regex.is_match(path.to_str().unwrap())
-  } else {
-    regex.is_match(path.to_str().unwrap())
-  }
+  if regex.is_err() {
+      let invalid_pattern = format!("Invalid ignore path pattern found in the ignore file - pattern: ${:?}, path: ${:?}", pattern, path);
+      panic!("{}", invalid_pattern);
+    }
+
+    let regex = regex.unwrap();
+
+    if pattern.starts_with('!') {
+        !regex.is_match(path.to_str().unwrap())
+    } else {
+        regex.is_match(path.to_str().unwrap())
+    }
 }
 
 pub fn scan_folder<'a>(folder: &'a PathBuf, js_extension: &'a JsExtension) -> Vec<PathBuf> {

diff --git a/src/ts_generator/information_schema.rs b/src/ts_generator/information_schema.rs
@@ -3,7 +3,7 @@ use mysql_async::prelude::Queryable;
 use mysql_async::prelude::*;
 use std::collections::HashMap;
 use tokio::sync::Mutex;
-
+use crate::common::errors::{DB_CONN_POOL_RETRIEVE_ERROR, DB_SCHEME_READ_ERROR};
 use crate::core::connection::DBConn;
 use crate::core::mysql::pool::MySqlConnectionManager;
 use crate::core::postgres::pool::PostgresConnectionManager;
@@ -96,7 +96,7 @@ impl DBSchema {
     let mut fields: HashMap<String, Field> = HashMap::new();
 
     let conn = conn.lock().await;
-    let conn = conn.get().await.unwrap();
+    let conn = conn.get().await.expect(DB_CONN_POOL_RETRIEVE_ERROR);
     let result = conn.query(&query, &[]).await;
 
     if let Ok(result) = result {
@@ -142,14 +142,14 @@ impl DBSchema {
 
     let mut fields: HashMap<String, Field> = HashMap::new();
     let conn = conn.lock().await;
-    let mut conn = conn.get().await.unwrap();
+    let mut conn = conn.get().await.expect(DB_CONN_POOL_RETRIEVE_ERROR);
     let result = conn.query::<mysql_async::Row, String>(query).await;
 
     if let Ok(result) = result {
       for row in result {
-        let field_name: String = row.clone().take(0).unwrap();
-        let field_type: String = row.clone().take(1).unwrap();
-        let is_nullable: String = row.clone().take(2).unwrap();
+        let field_name: String = row.clone().take(0).expect(DB_SCHEME_READ_ERROR);
+        let field_type: String = row.clone().take(1).expect(DB_SCHEME_READ_ERROR);
+        let is_nullable: String = row.clone().take(2).expect(DB_SCHEME_READ_ERROR);
         let field = Field {
           field_type: TsFieldType::get_ts_field_type_from_mysql_field_type(field_type.to_owned()),
           is_nullable: is_nullable == "YES",