From a0ef76164fe64e3a44244000c30073ec5e98a21e Mon Sep 17 00:00:00 2001 From: Dmitriy Pleshevskiy Date: Mon, 7 Jun 2021 00:33:59 +0300 Subject: [PATCH] chore: improve error handling --- migra/src/clients/mod.rs | 2 +- migra/src/clients/mysql.rs | 19 +++--- migra/src/clients/postgres.rs | 12 ++-- migra/src/clients/sqlite.rs | 14 ++--- migra/src/error.rs | 56 ----------------- migra/src/errors.rs | 86 +++++++++++++++++++++++++++ migra/src/fs.rs | 2 +- migra/src/lib.rs | 4 +- migra/src/managers.rs | 14 ++--- migra_cli/src/commands/apply.rs | 6 +- migra_cli/src/database/transaction.rs | 2 +- migra_cli/src/main.rs | 6 +- 12 files changed, 123 insertions(+), 100 deletions(-) delete mode 100644 migra/src/error.rs create mode 100644 migra/src/errors.rs diff --git a/migra/src/clients/mod.rs b/migra/src/clients/mod.rs index 7a08f94..a306036 100644 --- a/migra/src/clients/mod.rs +++ b/migra/src/clients/mod.rs @@ -3,7 +3,7 @@ // #![allow(clippy::module_name_repetitions)] // #![allow(clippy::missing_errors_doc)] -use crate::error::MigraResult; +use crate::errors::MigraResult; use crate::managers::{ManageMigrations, ManageTransaction}; pub trait OpenDatabaseConnection diff --git a/migra/src/clients/mysql.rs b/migra/src/clients/mysql.rs index 18f921e..5294e8e 100644 --- a/migra/src/clients/mysql.rs +++ b/migra/src/clients/mysql.rs @@ -1,5 +1,5 @@ use super::OpenDatabaseConnection; -use crate::error::{Error, MigraResult, StdResult}; +use crate::errors::{DbKind, Error, MigraResult, StdResult}; use crate::managers::{BatchExecute, ManageMigrations, ManageTransaction}; use crate::migration; use mysql::prelude::*; @@ -15,7 +15,7 @@ impl OpenDatabaseConnection for Client { fn manual(connection_string: &str, migrations_table_name: &str) -> MigraResult { let conn = Pool::new_manual(1, 1, connection_string) .and_then(|pool| pool.get_conn()) - .map_err(|_| Error::FailedDatabaseConnection)?; + .map_err(|err| Error::db(err.into(), DbKind::DatabaseConnection))?; Ok(Client { conn, @@ -43,31 +43,28 @@ impl ManageMigrations for Client { ); self.batch_execute(&stmt) - .map_err(|_| Error::FailedCreateMigrationsTable) + .map_err(|err| Error::db(err, DbKind::CreateMigrationsTable)) } fn insert_migration(&mut self, name: &str) -> MigraResult { let stmt = format!( - "INSERT INTO {} (name) VALUES ($1)", + "INSERT INTO {} (name) VALUES (?)", &self.migrations_table_name ); self.conn .exec_first(&stmt, (name,)) .map(Option::unwrap_or_default) - .map_err(|_| Error::FailedInsertMigration) + .map_err(|err| Error::db(err.into(), DbKind::InsertMigration)) } fn delete_migration(&mut self, name: &str) -> MigraResult { - let stmt = format!( - "DELETE FROM {} WHERE name = $1", - &self.migrations_table_name - ); + let stmt = format!("DELETE FROM {} WHERE name = ?", &self.migrations_table_name); self.conn .exec_first(&stmt, (name,)) .map(Option::unwrap_or_default) - .map_err(|_| Error::FailedDeleteMigration) + .map_err(|err| Error::db(err.into(), DbKind::DeleteMigration)) } fn get_applied_migrations(&mut self) -> MigraResult { @@ -79,7 +76,7 @@ impl ManageMigrations for Client { self.conn .query::(stmt) .map(From::from) - .map_err(|_| Error::FailedGetAppliedMigrations) + .map_err(|err| Error::db(err.into(), DbKind::GetAppliedMigrations)) } } diff --git a/migra/src/clients/postgres.rs b/migra/src/clients/postgres.rs index 52358e3..898c49c 100644 --- a/migra/src/clients/postgres.rs +++ b/migra/src/clients/postgres.rs @@ -1,5 +1,5 @@ use super::OpenDatabaseConnection; -use crate::error::{Error, MigraResult, StdResult}; +use crate::errors::{DbKind, Error, MigraResult, StdResult}; use crate::managers::{BatchExecute, ManageMigrations, ManageTransaction}; use crate::migration; use postgres::{Client as PostgresClient, NoTls}; @@ -21,7 +21,7 @@ impl fmt::Debug for Client { impl OpenDatabaseConnection for Client { fn manual(connection_string: &str, migrations_table_name: &str) -> MigraResult { let client = PostgresClient::connect(connection_string, NoTls) - .map_err(|_| Error::FailedDatabaseConnection)?; + .map_err(|err| Error::db(err.into(), DbKind::DatabaseConnection))?; Ok(Client { client, migrations_table_name: migrations_table_name.to_owned(), @@ -48,7 +48,7 @@ impl ManageMigrations for Client { ); self.batch_execute(&stmt) - .map_err(|_| Error::FailedCreateMigrationsTable) + .map_err(|err| Error::db(err, DbKind::CreateMigrationsTable)) } fn insert_migration(&mut self, name: &str) -> MigraResult { @@ -59,7 +59,7 @@ impl ManageMigrations for Client { self.client .execute(stmt.as_str(), &[&name]) - .map_err(|_| Error::FailedInsertMigration) + .map_err(|err| Error::db(err.into(), DbKind::InsertMigration)) } fn delete_migration(&mut self, name: &str) -> MigraResult { @@ -70,7 +70,7 @@ impl ManageMigrations for Client { self.client .execute(stmt.as_str(), &[&name]) - .map_err(|_| Error::FailedDeleteMigration) + .map_err(|err| Error::db(err.into(), DbKind::DeleteMigration)) } fn get_applied_migrations(&mut self) -> MigraResult { @@ -87,7 +87,7 @@ impl ManageMigrations for Client { .collect::, _>>() }) .map(From::from) - .map_err(|_| Error::FailedGetAppliedMigrations) + .map_err(|err| Error::db(err.into(), DbKind::GetAppliedMigrations)) } } diff --git a/migra/src/clients/sqlite.rs b/migra/src/clients/sqlite.rs index 3d17040..bc055d8 100644 --- a/migra/src/clients/sqlite.rs +++ b/migra/src/clients/sqlite.rs @@ -1,5 +1,5 @@ use super::OpenDatabaseConnection; -use crate::error::{Error, MigraResult, StdResult}; +use crate::errors::{DbKind, Error, MigraResult, StdResult}; use crate::managers::{BatchExecute, ManageMigrations, ManageTransaction}; use crate::migration; use rusqlite::Connection; @@ -12,8 +12,8 @@ pub struct Client { impl OpenDatabaseConnection for Client { fn manual(connection_string: &str, migrations_table_name: &str) -> MigraResult { - let conn = - Connection::open(connection_string).map_err(|_| Error::FailedDatabaseConnection)?; + let conn = Connection::open(connection_string) + .map_err(|err| Error::db(err.into(), DbKind::DatabaseConnection))?; Ok(Client { conn, migrations_table_name: migrations_table_name.to_owned(), @@ -40,7 +40,7 @@ impl ManageMigrations for Client { ); self.batch_execute(&stmt) - .map_err(|_| Error::FailedCreateMigrationsTable) + .map_err(|err| Error::db(err, DbKind::CreateMigrationsTable)) } fn insert_migration(&mut self, name: &str) -> MigraResult { @@ -52,7 +52,7 @@ impl ManageMigrations for Client { self.conn .execute(&stmt, [name]) .map(|res| res as u64) - .map_err(|_| Error::FailedInsertMigration) + .map_err(|err| Error::db(err.into(), DbKind::InsertMigration)) } fn delete_migration(&mut self, name: &str) -> MigraResult { @@ -64,7 +64,7 @@ impl ManageMigrations for Client { self.conn .execute(&stmt, [name]) .map(|res| res as u64) - .map_err(|_| Error::FailedDeleteMigration) + .map_err(|err| Error::db(err.into(), DbKind::DeleteMigration)) } fn get_applied_migrations(&mut self) -> MigraResult { @@ -80,7 +80,7 @@ impl ManageMigrations for Client { .collect::, _>>() }) .map(From::from) - .map_err(|_| Error::FailedGetAppliedMigrations) + .map_err(|err| Error::db(err.into(), DbKind::GetAppliedMigrations)) } } diff --git a/migra/src/error.rs b/migra/src/error.rs deleted file mode 100644 index 79c1a62..0000000 --- a/migra/src/error.rs +++ /dev/null @@ -1,56 +0,0 @@ -use std::fmt; -use std::io; - -pub type StdResult = Result>; -pub type MigraResult = Result; - -#[derive(Debug)] -pub enum Error { - FailedDatabaseConnection, - - FailedOpenTransaction, - FailedCommitTransaction, - FailedRollbackTransaction, - - FailedCreateMigrationsTable, - FailedApplySql, - FailedInsertMigration, - FailedDeleteMigration, - FailedGetAppliedMigrations, - - Io(io::Error), -} - -impl fmt::Display for Error { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Error::FailedDatabaseConnection => fmt.write_str("Failed database connection"), - Error::FailedOpenTransaction => fmt.write_str("Failed to open a transaction"), - Error::FailedCommitTransaction => fmt.write_str("Failed to commit a transaction"), - Error::FailedRollbackTransaction => fmt.write_str("Failed to rollback a transaction"), - Error::FailedCreateMigrationsTable => { - fmt.write_str("Failed to create a migrations table") - } - Error::FailedApplySql => fmt.write_str("Failed to apply sql"), - Error::FailedInsertMigration => fmt.write_str("Failed to insert a migration"), - Error::FailedDeleteMigration => fmt.write_str("Failed to delete a migration"), - Error::FailedGetAppliedMigrations => fmt.write_str("Failed to get applied migrations"), - Error::Io(ref error) => write!(fmt, "{}", error), - } - } -} - -impl std::error::Error for Error {} - -impl PartialEq for Error { - fn eq(&self, other: &Self) -> bool { - std::mem::discriminant(self) == std::mem::discriminant(other) - } -} - -impl From for Error { - #[inline] - fn from(err: io::Error) -> Error { - Error::Io(err) - } -} diff --git a/migra/src/errors.rs b/migra/src/errors.rs new file mode 100644 index 0000000..182ce67 --- /dev/null +++ b/migra/src/errors.rs @@ -0,0 +1,86 @@ +use std::fmt; +use std::io; + +pub type StdError = Box; +pub type StdResult = Result; +pub type MigraResult = Result; + +#[derive(Debug)] +pub enum Error { + Db(DbError), + Io(io::Error), +} + +impl fmt::Display for Error { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Error::Db(ref error) => write!(fmt, "{}", error), + Error::Io(ref error) => write!(fmt, "{}", error), + } + } +} + +impl std::error::Error for Error {} + +impl PartialEq for Error { + fn eq(&self, other: &Self) -> bool { + std::mem::discriminant(self) == std::mem::discriminant(other) + } +} + +impl From for Error { + #[inline] + fn from(err: io::Error) -> Error { + Error::Io(err) + } +} + +impl Error { + #[must_use] + pub fn db(origin: StdError, kind: DbKind) -> Self { + Error::Db(DbError { kind, origin }) + } +} + +#[derive(Debug)] +pub enum DbKind { + DatabaseConnection, + + OpenTransaction, + CommitTransaction, + RollbackTransaction, + + CreateMigrationsTable, + ApplySql, + InsertMigration, + DeleteMigration, + GetAppliedMigrations, +} + +impl fmt::Display for DbKind { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DbKind::DatabaseConnection => fmt.write_str("Failed database connection"), + DbKind::OpenTransaction => fmt.write_str("Failed to open a transaction"), + DbKind::CommitTransaction => fmt.write_str("Failed to commit a transaction"), + DbKind::RollbackTransaction => fmt.write_str("Failed to rollback a transaction"), + DbKind::CreateMigrationsTable => fmt.write_str("Failed to create a migrations table"), + DbKind::ApplySql => fmt.write_str("Failed to apply sql"), + DbKind::InsertMigration => fmt.write_str("Failed to insert a migration"), + DbKind::DeleteMigration => fmt.write_str("Failed to delete a migration"), + DbKind::GetAppliedMigrations => fmt.write_str("Failed to get applied migrations"), + } + } +} + +#[derive(Debug)] +pub struct DbError { + kind: DbKind, + origin: StdError, +} + +impl fmt::Display for DbError { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "{} - {}", &self.kind, &self.origin) + } +} diff --git a/migra/src/fs.rs b/migra/src/fs.rs index f6afbd2..0992d02 100644 --- a/migra/src/fs.rs +++ b/migra/src/fs.rs @@ -1,4 +1,4 @@ -use crate::error::MigraResult; +use crate::errors::MigraResult; use crate::migration; use crate::migration::{DOWNGRADE_MIGRATION_FILE_NAME, UPGRADE_MIGRATION_FILE_NAME}; use std::io; diff --git a/migra/src/lib.rs b/migra/src/lib.rs index c3a65d9..3e4fe82 100644 --- a/migra/src/lib.rs +++ b/migra/src/lib.rs @@ -8,7 +8,7 @@ pub mod fs; pub mod managers; pub mod migration; -mod error; -pub use error::{Error, MigraResult as Result, StdResult}; +mod errors; +pub use errors::{Error, MigraResult as Result, StdResult}; pub use migration::Migration; diff --git a/migra/src/managers.rs b/migra/src/managers.rs index 3a7c4e1..5cb1e6a 100644 --- a/migra/src/managers.rs +++ b/migra/src/managers.rs @@ -1,4 +1,4 @@ -use crate::error::{Error, MigraResult, StdResult}; +use crate::errors::{DbKind, Error, MigraResult, StdResult}; use crate::migration::{self, Migration}; use std::path::Path; @@ -9,26 +9,24 @@ pub trait BatchExecute { pub trait ManageTransaction: BatchExecute { fn begin_transaction(&mut self) -> MigraResult<()> { self.batch_execute("BEGIN") - .map_err(|_| Error::FailedOpenTransaction) + .map_err(|err| Error::db(err, DbKind::OpenTransaction)) } fn rollback_transaction(&mut self) -> MigraResult<()> { self.batch_execute("ROLLBACK") - .map_err(|_| Error::FailedRollbackTransaction) + .map_err(|err| Error::db(err, DbKind::RollbackTransaction)) } fn commit_transaction(&mut self) -> MigraResult<()> { self.batch_execute("COMMIT") - .map_err(|_| Error::FailedCommitTransaction) + .map_err(|err| Error::db(err, DbKind::CommitTransaction)) } } pub trait ManageMigrations: BatchExecute { fn apply_sql(&mut self, sql: &str) -> MigraResult<()> { - self.batch_execute(sql).map_err(|err| { - dbg!(err); - Error::FailedApplySql - }) + self.batch_execute(sql) + .map_err(|err| Error::db(err, DbKind::ApplySql)) } fn create_migrations_table(&mut self) -> MigraResult<()>; diff --git a/migra_cli/src/commands/apply.rs b/migra_cli/src/commands/apply.rs index f62d6c8..70ea777 100644 --- a/migra_cli/src/commands/apply.rs +++ b/migra_cli/src/commands/apply.rs @@ -4,11 +4,7 @@ use crate::opts::ApplyCommandOpt; pub(crate) fn apply_sql(app: &App, cmd_opts: &ApplyCommandOpt) -> migra::StdResult<()> { let config = app.config()?; - let mut client = crate::client::create( - &config.database.client(), - &config.database.connection_string()?, - &config.migrations.table_name(), - )?; + let mut client = crate::client::create_from_config(&config)?; let file_contents = cmd_opts .file_paths diff --git a/migra_cli/src/database/transaction.rs b/migra_cli/src/database/transaction.rs index 74f86fd..ca8abca 100644 --- a/migra_cli/src/database/transaction.rs +++ b/migra_cli/src/database/transaction.rs @@ -1,5 +1,5 @@ use super::client_rusqlite::Connection::AnyConnection; -use crate::error::StdResult; +use crate::errors::StdResult; pub trait ManageTransaction { fn begin_transaction(&self, conn: &mut AnyConnection) -> migra::StdResult<()>; diff --git a/migra_cli/src/main.rs b/migra_cli/src/main.rs index dcae36a..85148f6 100644 --- a/migra_cli/src/main.rs +++ b/migra_cli/src/main.rs @@ -20,9 +20,11 @@ use app::App; use config::Config; use opts::{AppOpt, StructOpt}; -fn main() -> migra::StdResult<()> { +fn main() { #[cfg(feature = "dotenv")] dotenv::dotenv().ok(); - App::new(AppOpt::from_args()).run_command() + if let Err(err) = App::new(AppOpt::from_args()).run_command() { + eprintln!("Error: {}", err); + } }