From 306355a50b65dc119b47e071a882200998dd35c8 Mon Sep 17 00:00:00 2001 From: Jason Shin Date: Fri, 26 Jul 2024 00:34:10 +1000 Subject: [PATCH] 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 --- book/src/api/1.3.configs-file-based.md | 4 +++- dprint.json | 11 ----------- src/common/cli.rs | 2 +- src/common/config.rs | 15 +++++++++++++-- src/common/errors.rs | 6 ++++++ src/common/lazy.rs | 23 ++++++++++++++++++----- src/common/logger.rs | 26 ++++++++++++++++++++++++++ src/common/mod.rs | 2 ++ src/common/types.rs | 1 + src/core/connection.rs | 4 ++-- src/core/mysql/prepare.rs | 3 ++- src/scan_folder.rs | 23 +++++++++++++++-------- src/ts_generator/information_schema.rs | 12 ++++++------ 13 files changed, 95 insertions(+), 37 deletions(-) delete mode 100644 dprint.json create mode 100644 src/common/errors.rs diff --git a/book/src/api/1.3.configs-file-based.md b/book/src/api/1.3.configs-file-based.md index 15d36361..c118b563 100644 --- 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 deleted file mode 100644 index ab9e469d..00000000 --- a/dprint.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "typescript": { - }, - "includes": ["samples/**/*.{ts,tsx,js,jsx,cjs,mjs}"], - "excludes": [ - "**/node_modules" - ], - "plugins": [ - "https://plugins.dprint.dev/typescript-0.71.1.wasm" - ] -} diff --git a/src/common/cli.rs b/src/common/cli.rs index c443db84..f64b36fa 100644 --- a/src/common/cli.rs +++ b/src/common/cli.rs @@ -74,7 +74,7 @@ pub struct Cli { #[clap(long, short)] pub message_format: Option, - /// 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, } diff --git a/src/common/config.rs b/src/common/config.rs index dd4e9904..3106223a 100644 --- a/src/common/config.rs +++ b/src/common/config.rs @@ -52,8 +52,12 @@ pub struct DbConnectionConfig { pub db_name: Option, #[serde(rename = "PG_SEARCH_PATH")] pub pg_search_path: Option, + #[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::(f.as_str()).unwrap()); diff --git a/src/common/errors.rs b/src/common/errors.rs new file mode 100644 index 00000000..d955658c --- /dev/null +++ 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 index 3624faaa..afac00ed 100644 --- a/src/common/lazy.rs +++ b/src/common/lazy.rs @@ -19,13 +19,15 @@ 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> = 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>> = { 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 => { @@ -33,7 +35,9 @@ lazy_static! { 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)) })) } @@ -41,7 +45,9 @@ lazy_static! { 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 index fad7f8e8..db0d9eac 100644 --- 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 index 803b77c4..01108013 100644 --- 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 index ed526d9e..1ce38d1a 100644 --- 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 index 844b1772..b84ee62f 100644 --- 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 index 4067fa36..c82b4c88 100644 --- 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::(explain_query).await; if let Err(err) = result { diff --git a/src/scan_folder.rs b/src/scan_folder.rs index d4d497dd..c62fa52d 100644 --- 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 { 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 { diff --git a/src/ts_generator/information_schema.rs b/src/ts_generator/information_schema.rs index a4f377c8..eef1787e 100644 --- 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 = 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 = 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::(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",