Better error messages & some clippy fixes (#197)

* better info

* better error messages and clippy fixes

* wip

* capturing error

* improving logging for panic and error

* wip: trying to figure out how to fail immediately if connection config is not right

* better error messages on various driver IO errors

* much better error messages

* fmt

* refactor

* postgres pool fix

* refactoring everything to lazyCell under sync

* lazy static to lazy cell, which is type safe

* clean up and remove lazy static

* fix

* postgres errors

* fix

* introduce connection timeout

* fix

* fix

* clippy fix

* hrmm

* hmm,

* remove postgres

* clippy

* remove wrong clippy config

* fix

* fix

* fix

* fixed

* clippy

* remove println

* fix loop

* remove println

* fmt

Author: JasonShin

Cargo.lock

@@ -26,7 +26,6 @@ serde_json = { version = "1.0.127" }
 sqlparser = { version = "0.45.0" }
 regex = { version = "1.10.6" }
 convert_case = "0.6.0"
-lazy_static = { version = "1.5.0" }
 openssl = { version = "0.10.66", features = ["vendored"] }
 colored = "2.1.0"
 mysql_async = { version = "0.34.2", features = ["minimal"] }

Cargo.toml

@@ -39,7 +39,8 @@ Example `.sqlxrc.json`
       "DB_PASS": "password",
       "DB_HOST": "127.0.0.1",
       "DB_PORT": 3307,
-      "POOL_SIZE": 20
+      "POOL_SIZE": 20,
+      "CONNECTION_TIMEOUT": 10
     }
   }
 }
@@ -75,6 +76,7 @@ Supported fields of each connection include
 - `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
+- `CONNECTION_TIMEOUT`: Timeout in second of Database connection attempt
 
 ### generate_types
 

book/src/api/1.3.configs-file-based.md

@@ -54,19 +54,24 @@ pub struct DbConnectionConfig {
   pub pg_search_path: Option<String>,
   #[serde(rename = "POOL_SIZE", default = "default_pool_size")]
   pub pool_size: u32,
+  #[serde(rename = "CONNECTION_TIMEOUT", default = "default_connection_timeout")]
+  pub connection_timeout: u64,
 }
 
 fn default_pool_size() -> u32 {
   10
 }
 
+fn default_connection_timeout() -> u64 {
+  5
+}
+
 /// 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
 /// 2. any dotenv configured options
 #[derive(Clone, Debug)]
 pub struct Config {
-  pub dotenv: Dotenv,
   pub generate_types_config: Option<GenerateTypesConfig>,
   pub connections: HashMap<String, DbConnectionConfig>,
   pub ignore_patterns: Vec<String>,
@@ -94,7 +99,6 @@ impl Config {
     let ignore_patterns = Self::get_ignore_patterns(&default_ignore_config_path);
     let log_level = Self::get_log_level(file_config_path);
     Config {
-      dotenv,
       connections,
       generate_types_config,
       ignore_patterns,
@@ -169,7 +173,7 @@ impl Config {
             "Empty or invalid JSON provided for file based configuration - config file: {:?}, error: {:?}",
             file_config_path, result,
           )
-        )
+        );
       }
 
       result.unwrap()
@@ -271,6 +275,11 @@ impl Config {
       .or_else(|| Some(default_pool_size()))
       .unwrap();
 
+    let connection_timeout = default_config
+      .map(|x| x.connection_timeout)
+      .or_else(|| Some(default_connection_timeout()))
+      .unwrap();
+
     DbConnectionConfig {
       db_type: db_type.to_owned(),
       db_host: db_host.to_owned(),
@@ -280,6 +289,7 @@ impl Config {
       db_name: db_name.to_owned(),
       pg_search_path: pg_search_path.to_owned(),
       pool_size,
+      connection_timeout,
     }
   }
 

src/common/config.rs

@@ -6,84 +6,99 @@ use crate::core::mysql::pool::MySqlConnectionManager;
 use crate::core::postgres::pool::PostgresConnectionManager;
 use crate::ts_generator::information_schema::DBSchema;
 use clap::Parser;
-use lazy_static::lazy_static;
+use std::sync::LazyLock;
 use std::{collections::HashMap, sync::Arc};
 use tokio::{runtime::Handle, sync::Mutex, task};
 
 // The file contains all implicitly dependent variables or state that files need for the logic
 // We have a lot of states that we need to drill down into each methods
-lazy_static! {
-    pub static ref CLI_ARGS: Cli = Cli::parse();
-    pub static ref CONFIG: Config =  Config::new();
+pub static CLI_ARGS: LazyLock<Cli> = LazyLock::new(Cli::parse);
+pub static CONFIG: LazyLock<Config> = LazyLock::new(Config::new);
 
-    // 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 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 DB_SCHEMA: LazyLock<Arc<Mutex<DBSchema>>> = LazyLock::new(|| Arc::new(Mutex::new(DBSchema::new())));
+// TODO: move this to errors.rs
+pub static ERR_DB_CONNECTION_ISSUE: LazyLock<String> = LazyLock::new(|| {
+  "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_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(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
-                            .expect(&ERR_DB_CONNECTION_ISSUE);
-                        let db_conn = DBConn::PostgresConn(Mutex::new(pool));
+// 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
+pub static DB_CONN_CACHE: LazyLock<HashMap<String, Arc<Mutex<DBConn>>>> = LazyLock::new(|| {
+  let mut cache = HashMap::new();
+  for connection in CONFIG.connections.keys() {
+    let connection_config = CONFIG
+      .connections
+      .get(connection)
+      .unwrap_or_else(|| panic!("Invalid connection name - {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(), connection.to_string());
+          let pool = bb8::Pool::builder()
+            .max_size(connection_config.pool_size)
+            .connection_timeout(std::time::Duration::from_secs(connection_config.connection_timeout))
+            .build(manager)
+            .await
+            .expect(&ERR_DB_CONNECTION_ISSUE);
 
-                        let conn = match &db_conn {
-                            DBConn::PostgresConn(conn) => conn,
-                            _ => panic!("Invalid connection type"),
-                        };
+          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(connection_config.pool_size)
+            .connection_timeout(std::time::Duration::from_secs(connection_config.connection_timeout))
+            .build(manager)
+            .await
+            .expect(&ERR_DB_CONNECTION_ISSUE);
 
-                        if connection_config.pg_search_path.is_some() {
-                            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
-                                    .expect(&ERR_DB_CONNECTION_ISSUE);
+          let db_conn = DBConn::PostgresConn(Mutex::new(pool));
 
-                                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
-                    }))
+          let conn = match &db_conn {
+            DBConn::PostgresConn(conn) => conn,
+            _ => panic!("Invalid connection type"),
+          };
 
-                }
-            };
-            cache.insert(connection.to_owned(), Arc::new(Mutex::new(conn)));
-        };
-        cache
-    };
+          if connection_config.pg_search_path.is_some() {
+            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.expect(&ERR_DB_CONNECTION_ISSUE);
 
-    // This variable holds a singleton of DBConnections that is used to get a DBConn from the cache
-    // DBConn is used to access the raw connection to the database or run `prepare` statement against each connection
-    pub static ref DB_CONNECTIONS: Arc<Mutex<DBConnections<'static>>> = {
-        let db_connections = DBConnections::new(&DB_CONN_CACHE);
-        Arc::new(Mutex::new(db_connections))
+              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
+        })
+      }),
     };
-}
+    cache.insert(connection.to_owned(), Arc::new(Mutex::new(conn)));
+  }
+  cache
+});
+
+// This variable holds a singleton of DBConnections that is used to get a DBConn from the cache
+// DBConn is used to access the raw connection to the database or run `prepare` statement against each connection
+pub static DB_CONNECTIONS: LazyLock<Arc<Mutex<DBConnections<'static>>>> = LazyLock::new(|| {
+  let db_connections = DBConnections::new(&DB_CONN_CACHE);
+  Arc::new(Mutex::new(db_connections))
+});

src/common/lazy.rs

@@ -1,6 +1,7 @@
 // TODO: Add documentation including examples
 // TODO: Use SQLX_TS_LOG env var to set log level
 
+#[allow(unused_macros)]
 macro_rules! debug {
     ($arg:tt) => ({
         use crate::common::lazy::CONFIG;
@@ -90,6 +91,7 @@ macro_rules! error {
     ($arg:tt, $($arg2:tt)*) => ({
         use crate::common::lazy::CONFIG;
         use crate::common::types::LogLevel;
+
         if CONFIG.log_level.gte(&LogLevel::Error) {
             use colored::*;
             let level = "[ERROR]".red();
@@ -98,3 +100,5 @@ macro_rules! error {
         }
     });
 }
+
+pub(crate) use error;

src/common/logger.rs

@@ -1,15 +1,20 @@
 use async_trait::async_trait;
-use mysql_async::{prelude::*, Conn, Error, Opts};
+use mysql_async::{prelude::*, Conn, Error, IoError, Opts};
 use tokio::{runtime::Handle, task};
 
 #[derive(Clone, Debug)]
 pub struct MySqlConnectionManager {
   conn_url: String,
+  // The connection name that user has defined in .sqlxrc.json
+  connection_name: String,
 }
 
 impl MySqlConnectionManager {
-  pub fn new(conn_url: String) -> Self {
-    Self { conn_url }
+  pub fn new(conn_url: String, connection_name: String) -> Self {
+    Self {
+      conn_url,
+      connection_name,
+    }
   }
 }
 
@@ -19,12 +24,50 @@ impl bb8::ManageConnection for MySqlConnectionManager {
   type Error = Error;
 
   async fn connect(&self) -> Result<Self::Connection, Self::Error> {
+    let connection_name = &self.connection_name;
     let conn_opts = Opts::from_url(self.conn_url.as_str())?;
-    Conn::new(conn_opts).await
+
+    let conn = Conn::new(conn_opts).await.map_err(|err| {
+      match err {
+        Error::Driver(driver_error) => {
+          panic!("Driver error occurred while connecting to MySQL database - connection: {connection_name}, error: {driver_error}");
+        }
+        Error::Io(io_err) => {
+          match io_err {
+            IoError::Io(io_err) => {
+              if io_err.kind() == std::io::ErrorKind::ConnectionRefused {
+                panic!("Connection Refused - check your connection config for MySQL database - connection: {connection_name}")
+              } else {
+                panic!("I/O error occurred while connection to MySQL database - connection: {connection_name}, error: {io_err}")
+              }
+            }
+            IoError::Tls(tls_err) => {
+              panic!("TLS error occurred while connecting to MySQL database - connection: {connection_name}, error: {tls_err}");
+            }
+          }
+        }
+        Error::Other(other_err) => {
+          panic!("An unexpected error occurred while connecting to MySQL database - connection: {connection_name}, error: {other_err}");
+        }
+        Error::Server(server_err) => {
+          panic!("Server error occurred while connecting to MySQL database - connection: {connection_name}, error: {server_err}");
+        }
+        Error::Url(_) => {
+          panic!("Invalid URL format for MySQL connection string - connection: {connection_name}");
+        }
+      }
+    }).unwrap();
+
+    Ok(conn)
   }
 
   async fn is_valid(&self, conn: &mut Self::Connection) -> Result<(), Self::Error> {
-    conn.query("SELECT version()").await.map(|_: Vec<String>| ())
+    let connection_name = &self.connection_name;
+    conn
+      .query("SELECT 1")
+      .await
+      .map(|_: Vec<String>| ())
+      .map_err(|err| panic!("Failed to validate MySQL connection for connection: {connection_name}. Error: {err}"))
   }
 
   fn has_broken(&self, conn: &mut Self::Connection) -> bool {

src/core/mysql/pool.rs

@@ -27,10 +27,7 @@ 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
-      .expect("Failed to retrieve a connection from the pool. Consider increasing the connection pool size");
+    let mut conn = conn.get().await?;
     let result = conn.query::<Row, String>(explain_query).await;
 
     if let Err(err) = result {