From fe6c40dd75a8a980009623e7b84e56974ba026b1 Mon Sep 17 00:00:00 2001 From: mohs8421 Date: Sun, 28 Aug 2022 05:13:51 +0200 Subject: [PATCH 01/15] Introducing sqlx-error feature (#750) * feat: Introducing feature "sqlx-error" Purpose of this feature is to not convert errors given from sqlx into strings to ease further analysis of the error and react to it accordingly. This implementation uses a feature switch and an additional error kind to avoid interfering with existing implementations without this feature enabled. See discussion https://github.com/SeaQL/sea-orm/discussions/709 * fix: Align feature "sqlx-error" with merged Migration error kind Due to the merge, an other error kind had been introduced and the DbErr became Eq and Clone, however Eq cannot easily be derived from, so I went back to PartialEq, and since the sqlx error does not implement clone, this was converted into an Arc, to allow cloning of the new kind too. * fix: Repairing failing jobs Several jobs had failed as I missed to correct the return values of a few methods in transaction.rs and had a wrong understanding of map_err at that point. * feat: realigning with latest changes in sea-orm, different approach Instead of the former approach to introduce a new error kind, now the existing error types get extended, for now only Exec and Query, because these are the most relevant for the requirement context. Afterwards it might still be possible to add some further detail information. See discussion https://github.com/SeaQL/sea-orm/discussions/709 * Update src/driver/sqlx_mysql.rs Integrating fixes done by @Sculas Co-authored-by: Sculas * Update src/driver/sqlx_postgres.rs Integrating fixes done by @Sculas Co-authored-by: Sculas * Update src/driver/sqlx_sqlite.rs Integrating fixes done by @Sculas Co-authored-by: Sculas * feat: reworking feature with thiserror Following the latest suggestions I changed the implementation to utilize `thiserror`. Now there are more error kinds to be able to see the different kinds directly in src/error.rs To ensure the behaviour is as expected, I also introduce a further test, which checks for a uniqueness failure. Co-authored-by: Sculas --- Cargo.toml | 1 + issues/324/src/model.rs | 5 +--- issues/400/src/model.rs | 7 ++--- src/database/mock.rs | 2 +- src/database/transaction.rs | 49 +++++++++++++++++++------------ src/driver/sqlx_common.rs | 6 ++-- src/driver/sqlx_mysql.rs | 28 +++++------------- src/driver/sqlx_postgres.rs | 28 +++++------------- src/driver/sqlx_sqlite.rs | 28 +++++------------- src/error.rs | 58 +++++++++++++++++++++++++++---------- src/executor/insert.rs | 6 ++-- src/executor/query.rs | 29 +++++++------------ src/executor/update.rs | 6 ++-- tests/crud/error.rs | 56 +++++++++++++++++++++++++++++++++++ tests/crud/mod.rs | 2 ++ tests/crud_tests.rs | 1 + 16 files changed, 183 insertions(+), 129 deletions(-) create mode 100644 tests/crud/error.rs diff --git a/Cargo.toml b/Cargo.toml index 277a2a68..a2c0384f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ uuid = { version = "^1", features = ["serde", "v4"], optional = true } ouroboros = "0.15" url = "^2.2" once_cell = "1.8" +thiserror = "^1" [dev-dependencies] smol = { version = "^1.2" } diff --git a/issues/324/src/model.rs b/issues/324/src/model.rs index 9ec35a07..37174db9 100644 --- a/issues/324/src/model.rs +++ b/issues/324/src/model.rs @@ -26,10 +26,7 @@ macro_rules! impl_try_from_u64_err { ($newtype: ident) => { impl sea_orm::TryFromU64 for $newtype { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::DbErr::Exec(format!( - "{} cannot be converted from u64", - stringify!($newtype) - ))) + Err(sea_orm::ConvertFromU64(stringify!($newtype))) } } }; diff --git a/issues/400/src/model.rs b/issues/400/src/model.rs index a53eeb74..6d9044c4 100644 --- a/issues/400/src/model.rs +++ b/issues/400/src/model.rs @@ -1,5 +1,5 @@ -use std::marker::PhantomData; use sea_orm::entity::prelude::*; +use std::marker::PhantomData; #[derive(Clone, Debug, PartialEq, DeriveEntityModel)] #[sea_orm(table_name = "model")] @@ -31,10 +31,7 @@ impl From> for Uuid { impl sea_orm::TryFromU64 for AccountId { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::DbErr::Exec(format!( - "{} cannot be converted from u64", - stringify!(AccountId) - ))) + Err(sea_orm::ConvertFromU64(stringify!(AccountId))) } } diff --git a/src/database/mock.rs b/src/database/mock.rs index 7af7cdb5..a1dbc795 100644 --- a/src/database/mock.rs +++ b/src/database/mock.rs @@ -102,7 +102,7 @@ impl MockDatabaseTrait for MockDatabase { result: ExecResultHolder::Mock(std::mem::take(&mut self.exec_results[counter])), }) } else { - Err(DbErr::Exec("`exec_results` buffer is empty.".to_owned())) + Err(DbErr::Query("`exec_results` buffer is empty.".to_owned())) } } diff --git a/src/database/transaction.rs b/src/database/transaction.rs index cb1ad122..66ae25e5 100644 --- a/src/database/transaction.rs +++ b/src/database/transaction.rs @@ -238,6 +238,17 @@ impl DatabaseTransaction { } } } + + #[cfg(feature = "sqlx-dep")] + fn map_err_ignore_not_found( + err: Result, sqlx::Error>, + ) -> Result, DbErr> { + if let Err(sqlx::Error::RowNotFound) = err { + Ok(None) + } else { + err.map_err(|e| sqlx_error_to_query_err(e)) + } + } } impl Drop for DatabaseTransaction { @@ -258,13 +269,14 @@ impl ConnectionTrait for DatabaseTransaction { async fn execute(&self, stmt: Statement) -> Result { debug_print!("{}", stmt); - let _res = match &mut *self.conn.lock().await { + match &mut *self.conn.lock().await { #[cfg(feature = "sqlx-mysql")] InnerConnection::MySql(conn) => { let query = crate::driver::sqlx_mysql::sqlx_query(&stmt); crate::metric::metric!(self.metric_callback, &stmt, { query.execute(conn).await.map(Into::into) }) + .map_err(sqlx_error_to_exec_err) } #[cfg(feature = "sqlx-postgres")] InnerConnection::Postgres(conn) => { @@ -272,6 +284,7 @@ impl ConnectionTrait for DatabaseTransaction { crate::metric::metric!(self.metric_callback, &stmt, { query.execute(conn).await.map(Into::into) }) + .map_err(sqlx_error_to_exec_err) } #[cfg(feature = "sqlx-sqlite")] InnerConnection::Sqlite(conn) => { @@ -279,14 +292,13 @@ impl ConnectionTrait for DatabaseTransaction { crate::metric::metric!(self.metric_callback, &stmt, { query.execute(conn).await.map(Into::into) }) + .map_err(sqlx_error_to_exec_err) } #[cfg(feature = "mock")] InnerConnection::Mock(conn) => return conn.execute(stmt), #[allow(unreachable_patterns)] _ => unreachable!(), - }; - #[cfg(feature = "sqlx-dep")] - _res.map_err(sqlx_error_to_exec_err) + } } #[instrument(level = "trace")] @@ -294,32 +306,32 @@ impl ConnectionTrait for DatabaseTransaction { async fn query_one(&self, stmt: Statement) -> Result, DbErr> { debug_print!("{}", stmt); - let _res = match &mut *self.conn.lock().await { + match &mut *self.conn.lock().await { #[cfg(feature = "sqlx-mysql")] InnerConnection::MySql(conn) => { let query = crate::driver::sqlx_mysql::sqlx_query(&stmt); - query.fetch_one(conn).await.map(|row| Some(row.into())) + Self::map_err_ignore_not_found( + query.fetch_one(conn).await.map(|row| Some(row.into())), + ) } #[cfg(feature = "sqlx-postgres")] InnerConnection::Postgres(conn) => { let query = crate::driver::sqlx_postgres::sqlx_query(&stmt); - query.fetch_one(conn).await.map(|row| Some(row.into())) + Self::map_err_ignore_not_found( + query.fetch_one(conn).await.map(|row| Some(row.into())), + ) } #[cfg(feature = "sqlx-sqlite")] InnerConnection::Sqlite(conn) => { let query = crate::driver::sqlx_sqlite::sqlx_query(&stmt); - query.fetch_one(conn).await.map(|row| Some(row.into())) + Self::map_err_ignore_not_found( + query.fetch_one(conn).await.map(|row| Some(row.into())), + ) } #[cfg(feature = "mock")] InnerConnection::Mock(conn) => return conn.query_one(stmt), #[allow(unreachable_patterns)] _ => unreachable!(), - }; - #[cfg(feature = "sqlx-dep")] - if let Err(sqlx::Error::RowNotFound) = _res { - Ok(None) - } else { - _res.map_err(sqlx_error_to_query_err) } } @@ -328,7 +340,7 @@ impl ConnectionTrait for DatabaseTransaction { async fn query_all(&self, stmt: Statement) -> Result, DbErr> { debug_print!("{}", stmt); - let _res = match &mut *self.conn.lock().await { + match &mut *self.conn.lock().await { #[cfg(feature = "sqlx-mysql")] InnerConnection::MySql(conn) => { let query = crate::driver::sqlx_mysql::sqlx_query(&stmt); @@ -336,6 +348,7 @@ impl ConnectionTrait for DatabaseTransaction { .fetch_all(conn) .await .map(|rows| rows.into_iter().map(|r| r.into()).collect()) + .map_err(sqlx_error_to_query_err) } #[cfg(feature = "sqlx-postgres")] InnerConnection::Postgres(conn) => { @@ -344,6 +357,7 @@ impl ConnectionTrait for DatabaseTransaction { .fetch_all(conn) .await .map(|rows| rows.into_iter().map(|r| r.into()).collect()) + .map_err(sqlx_error_to_query_err) } #[cfg(feature = "sqlx-sqlite")] InnerConnection::Sqlite(conn) => { @@ -352,14 +366,13 @@ impl ConnectionTrait for DatabaseTransaction { .fetch_all(conn) .await .map(|rows| rows.into_iter().map(|r| r.into()).collect()) + .map_err(sqlx_error_to_query_err) } #[cfg(feature = "mock")] InnerConnection::Mock(conn) => return conn.query_all(stmt), #[allow(unreachable_patterns)] _ => unreachable!(), - }; - #[cfg(feature = "sqlx-dep")] - _res.map_err(sqlx_error_to_query_err) + } } } diff --git a/src/driver/sqlx_common.rs b/src/driver/sqlx_common.rs index 18ca059d..91509813 100644 --- a/src/driver/sqlx_common.rs +++ b/src/driver/sqlx_common.rs @@ -2,15 +2,15 @@ use crate::DbErr; /// Converts an [sqlx::error] execution error to a [DbErr] pub fn sqlx_error_to_exec_err(err: sqlx::Error) -> DbErr { - DbErr::Exec(err.to_string()) + DbErr::ExecSqlX(err) } /// Converts an [sqlx::error] query error to a [DbErr] pub fn sqlx_error_to_query_err(err: sqlx::Error) -> DbErr { - DbErr::Query(err.to_string()) + DbErr::QuerySqlX(err) } /// Converts an [sqlx::error] connection error to a [DbErr] pub fn sqlx_error_to_conn_err(err: sqlx::Error) -> DbErr { - DbErr::Conn(err.to_string()) + DbErr::ConnSqlX(err) } diff --git a/src/driver/sqlx_mysql.rs b/src/driver/sqlx_mysql.rs index 01b840dd..e1f1d416 100644 --- a/src/driver/sqlx_mysql.rs +++ b/src/driver/sqlx_mysql.rs @@ -45,7 +45,7 @@ impl SqlxMySqlConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::Conn(e.to_string()))?; + .map_err(|e| DbErr::ConnSqlX(e))?; use sqlx::ConnectOptions; if !options.sqlx_logging { opt.disable_statement_logging(); @@ -89,9 +89,7 @@ impl SqlxMySqlPoolConnection { } }) } else { - Err(DbErr::Exec( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -107,14 +105,12 @@ impl SqlxMySqlPoolConnection { Ok(row) => Ok(Some(row.into())), Err(err) => match err { sqlx::Error::RowNotFound => Ok(None), - _ => Err(DbErr::Query(err.to_string())), + _ => Err(sqlx_error_to_query_err(err)), }, } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -132,9 +128,7 @@ impl SqlxMySqlPoolConnection { } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -150,9 +144,7 @@ impl SqlxMySqlPoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -162,9 +154,7 @@ impl SqlxMySqlPoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_mysql(conn, self.metric_callback.clone()).await } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -185,9 +175,7 @@ impl SqlxMySqlPoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - ))) + Err(TransactionError::Connection(DbErr::ConnFromPool)) } } diff --git a/src/driver/sqlx_postgres.rs b/src/driver/sqlx_postgres.rs index bd561148..6d070115 100644 --- a/src/driver/sqlx_postgres.rs +++ b/src/driver/sqlx_postgres.rs @@ -45,7 +45,7 @@ impl SqlxPostgresConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::Conn(e.to_string()))?; + .map_err(|e| DbErr::ConnSqlX(e))?; use sqlx::ConnectOptions; if !options.sqlx_logging { opt.disable_statement_logging(); @@ -89,9 +89,7 @@ impl SqlxPostgresPoolConnection { } }) } else { - Err(DbErr::Exec( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -107,14 +105,12 @@ impl SqlxPostgresPoolConnection { Ok(row) => Ok(Some(row.into())), Err(err) => match err { sqlx::Error::RowNotFound => Ok(None), - _ => Err(DbErr::Query(err.to_string())), + _ => Err(sqlx_error_to_query_err(err)), }, } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -132,9 +128,7 @@ impl SqlxPostgresPoolConnection { } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -150,9 +144,7 @@ impl SqlxPostgresPoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -162,9 +154,7 @@ impl SqlxPostgresPoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_postgres(conn, self.metric_callback.clone()).await } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -185,9 +175,7 @@ impl SqlxPostgresPoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - ))) + Err(TransactionError::Connection(DbErr::ConnFromPool)) } } diff --git a/src/driver/sqlx_sqlite.rs b/src/driver/sqlx_sqlite.rs index 8bf56214..55e768b0 100644 --- a/src/driver/sqlx_sqlite.rs +++ b/src/driver/sqlx_sqlite.rs @@ -46,7 +46,7 @@ impl SqlxSqliteConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::Conn(e.to_string()))?; + .map_err(|e| DbErr::ConnSqlX(e))?; if options.sqlcipher_key.is_some() { opt = opt.pragma("key", options.sqlcipher_key.clone().unwrap()); } @@ -96,9 +96,7 @@ impl SqlxSqlitePoolConnection { } }) } else { - Err(DbErr::Exec( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -114,14 +112,12 @@ impl SqlxSqlitePoolConnection { Ok(row) => Ok(Some(row.into())), Err(err) => match err { sqlx::Error::RowNotFound => Ok(None), - _ => Err(DbErr::Query(err.to_string())), + _ => Err(sqlx_error_to_query_err(err)), }, } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -139,9 +135,7 @@ impl SqlxSqlitePoolConnection { } }) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -157,9 +151,7 @@ impl SqlxSqlitePoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -169,9 +161,7 @@ impl SqlxSqlitePoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_sqlite(conn, self.metric_callback.clone()).await } else { - Err(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - )) + Err(DbErr::ConnFromPool) } } @@ -192,9 +182,7 @@ impl SqlxSqlitePoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::Query( - "Failed to acquire connection from pool.".to_owned(), - ))) + Err(TransactionError::Connection(DbErr::ConnFromPool)) } } diff --git a/src/error.rs b/src/error.rs index aecc3f52..30b2909d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,38 +1,64 @@ +#[cfg(feature = "sqlx-dep")] +use sqlx::error::Error as SqlxError; +use thiserror::Error; + /// An error from unsuccessful database operations -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Error, Debug)] pub enum DbErr { + /// This error happens, when a pool was not able to create a connection + #[error("Failed to acquire connection from pool.")] + ConnFromPool, + /// Error in case of invalid type conversion attempts + #[error("fail to convert '{0}' into '{1}'")] + CannotConvertInto(String, String), + /// Error in case of invalid type conversion from an u64 + #[error("{0} cannot be converted from u64")] + ConvertFromU64(String), + /// After an insert statement it was impossible to retrieve the last_insert_id + #[error("Fail to unpack last_insert_id")] + InsertCouldNotUnpackInsertId, + /// When updating, a model should know it's primary key to check + /// if the record has been correctly updated, otherwise this error will occur + #[error("Fail to get primary key from model")] + UpdateCouldNotGetPrimaryKey, /// There was a problem with the database connection + #[error("Connection Error: {0}")] Conn(String), + /// There was a problem with the database connection from sqlx + #[cfg(feature = "sqlx-dep")] + #[error("Connection Error: {0}")] + ConnSqlX(#[source] SqlxError), /// An operation did not execute successfully - Exec(String), + #[cfg(feature = "sqlx-dep")] + #[error("Execution Error: {0}")] + ExecSqlX(#[source] SqlxError), + /// An error occurred while performing a query, with more details from sqlx + #[cfg(feature = "sqlx-dep")] + #[error("Query Error: {0}")] + QuerySqlX(#[source] SqlxError), /// An error occurred while performing a query + #[error("Query Error: {0}")] Query(String), /// The record was not found in the database + #[error("RecordNotFound Error: {0}")] RecordNotFound(String), /// A custom error + #[error("Custom Error: {0}")] Custom(String), /// Error occurred while parsing value as target type + #[error("Type Error: {0}")] Type(String), /// Error occurred while parsing json value as target type + #[error("Json Error: {0}")] Json(String), /// A migration error + #[error("Migration Error: {0}")] Migration(String), } -impl std::error::Error for DbErr {} - -impl std::fmt::Display for DbErr { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - Self::Conn(s) => write!(f, "Connection Error: {}", s), - Self::Exec(s) => write!(f, "Execution Error: {}", s), - Self::Query(s) => write!(f, "Query Error: {}", s), - Self::RecordNotFound(s) => write!(f, "RecordNotFound Error: {}", s), - Self::Custom(s) => write!(f, "Custom Error: {}", s), - Self::Type(s) => write!(f, "Type Error: {}", s), - Self::Json(s) => write!(f, "Json Error: {}", s), - Self::Migration(s) => write!(f, "Migration Error: {}", s), - } +impl PartialEq for DbErr { + fn eq(&self, other: &Self) -> bool { + self.to_string() == other.to_string() } } diff --git a/src/executor/insert.rs b/src/executor/insert.rs index 5f3f9c95..f7bb0629 100644 --- a/src/executor/insert.rs +++ b/src/executor/insert.rs @@ -130,7 +130,7 @@ where Some(value_tuple) => FromValueTuple::from_value_tuple(value_tuple), None => match last_insert_id_opt { Some(last_insert_id) => last_insert_id, - None => return Err(DbErr::Exec("Fail to unpack last_insert_id".to_owned())), + None => return Err(DbErr::InsertCouldNotUnpackInsertId), }, }; Ok(InsertResult { last_insert_id }) @@ -176,6 +176,8 @@ where }; match found { Some(model) => Ok(model), - None => Err(DbErr::Exec("Failed to find inserted item".to_owned())), + None => Err(DbErr::RecordNotFound( + "Failed to find inserted item".to_owned(), + )), } } diff --git a/src/executor/query.rs b/src/executor/query.rs index 06361a45..0f1b22b2 100644 --- a/src/executor/query.rs +++ b/src/executor/query.rs @@ -379,8 +379,9 @@ impl TryGetable for Decimal { use rust_decimal::prelude::FromPrimitive; match val { Some(v) => Decimal::from_f64(v).ok_or_else(|| { - TryGetError::DbErr(DbErr::Query( - "Failed to convert f64 into Decimal".to_owned(), + TryGetError::DbErr(DbErr::CannotConvertInto( + "f64".to_owned(), + "Decimal".to_owned(), )) }), None => Err(TryGetError::Null(column)), @@ -708,10 +709,7 @@ macro_rules! try_from_u64_err { ( $type: ty ) => { impl TryFromU64 for $type { fn try_from_u64(_: u64) -> Result { - Err(DbErr::Exec(format!( - "{} cannot be converted from u64", - stringify!($type) - ))) + Err(DbErr::ConvertFromU64(stringify!($type).to_string())) } } }; @@ -722,10 +720,7 @@ macro_rules! try_from_u64_err { $( $gen_type: TryFromU64, )* { fn try_from_u64(_: u64) -> Result { - Err(DbErr::Exec(format!( - "{} cannot be converted from u64", - stringify!(($($gen_type,)*)) - ))) + Err(DbErr::ConvertFromU64(stringify!($($gen_type,)*).to_string())) } } }; @@ -744,11 +739,7 @@ macro_rules! try_from_u64_numeric { fn try_from_u64(n: u64) -> Result { use std::convert::TryInto; n.try_into().map_err(|_| { - DbErr::Exec(format!( - "fail to convert '{}' into '{}'", - n, - stringify!($type) - )) + DbErr::CannotConvertInto(n.to_string(), stringify!($type).to_string()) }) } } @@ -828,9 +819,11 @@ mod tests { #[test] fn from_try_get_error() { // TryGetError::DbErr - let expected = DbErr::Query("expected error message".to_owned()); - let try_get_error = TryGetError::DbErr(expected.clone()); - assert_eq!(DbErr::from(try_get_error), expected); + let try_get_error = TryGetError::DbErr(DbErr::Query("expected error message".to_owned())); + assert_eq!( + DbErr::from(try_get_error), + DbErr::Query("expected error message".to_owned()) + ); // TryGetError::Null let try_get_error = TryGetError::Null("column".to_owned()); diff --git a/src/executor/update.rs b/src/executor/update.rs index f00c536f..cd9337f7 100644 --- a/src/executor/update.rs +++ b/src/executor/update.rs @@ -122,7 +122,7 @@ where Updater::new(query).check_record_exists().exec(db).await?; let primary_key_value = match model.get_primary_key_value() { Some(val) => FromValueTuple::from_value_tuple(val), - None => return Err(DbErr::Exec("Fail to get primary key from model".to_owned())), + None => return Err(DbErr::UpdateCouldNotGetPrimaryKey), }; let found = ::find_by_id(primary_key_value) .one(db) @@ -130,7 +130,9 @@ where // If we cannot select the updated row from db by the cached primary key match found { Some(model) => Ok(model), - None => Err(DbErr::Exec("Failed to find inserted item".to_owned())), + None => Err(DbErr::RecordNotFound( + "Failed to find inserted item".to_owned(), + )), } } } diff --git a/tests/crud/error.rs b/tests/crud/error.rs new file mode 100644 index 00000000..8dfab3fa --- /dev/null +++ b/tests/crud/error.rs @@ -0,0 +1,56 @@ +pub use super::*; +use rust_decimal_macros::dec; +use sea_orm::DbErr; +#[cfg(any( + feature = "sqlx-mysql", + feature = "sqlx-sqlite", + feature = "sqlx-postgres" +))] +use sqlx::Error; +use uuid::Uuid; + +pub async fn test_cake_error_sqlx(db: &DbConn) { + let mud_cake = cake::ActiveModel { + name: Set("Moldy Cake".to_owned()), + price: Set(dec!(10.25)), + gluten_free: Set(false), + serial: Set(Uuid::new_v4()), + bakery_id: Set(None), + ..Default::default() + }; + + let cake = mud_cake.save(db).await.expect("could not insert cake"); + + // if compiling without sqlx, this assignment will complain, + // but the whole test is useless in that case anyway. + #[allow(unused_variables)] + let error: DbErr = cake + .into_active_model() + .insert(db) + .await + .expect_err("inserting should fail due to duplicate primary key"); + + #[cfg(any(feature = "sqlx-mysql", feature = "sqlx-sqlite"))] + match error { + DbErr::ExecSqlX(sqlx_error) => match sqlx_error { + Error::Database(e) => { + #[cfg(feature = "sqlx-mysql")] + assert_eq!(e.code().unwrap(), "23000"); + #[cfg(feature = "sqlx-sqlite")] + assert_eq!(e.code().unwrap(), "1555"); + } + _ => panic!("Unexpected sqlx-error kind"), + }, + _ => panic!("Unexpected Error kind"), + } + #[cfg(feature = "sqlx-postgres")] + match error { + DbErr::QuerySqlX(sqlx_error) => match sqlx_error { + Error::Database(e) => { + assert_eq!(e.code().unwrap(), "23505"); + } + _ => panic!("Unexpected sqlx-error kind"), + }, + _ => panic!("Unexpected Error kind"), + } +} diff --git a/tests/crud/mod.rs b/tests/crud/mod.rs index 916878c8..3a629d7a 100644 --- a/tests/crud/mod.rs +++ b/tests/crud/mod.rs @@ -3,6 +3,7 @@ pub mod create_cake; pub mod create_lineitem; pub mod create_order; pub mod deletes; +pub mod error; pub mod updates; pub use create_baker::*; @@ -10,6 +11,7 @@ pub use create_cake::*; pub use create_lineitem::*; pub use create_order::*; pub use deletes::*; +pub use error::*; pub use updates::*; pub use super::common::bakery_chain::*; diff --git a/tests/crud_tests.rs b/tests/crud_tests.rs index bc2a3c97..9f7ea404 100644 --- a/tests/crud_tests.rs +++ b/tests/crud_tests.rs @@ -35,5 +35,6 @@ pub async fn create_entities(db: &DatabaseConnection) { test_update_deleted_customer(db).await; test_delete_cake(db).await; + test_cake_error_sqlx(db).await; test_delete_bakery(db).await; } From 0b754eab0b1b9a8fcbcefce48878e0340514edb2 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 28 Aug 2022 12:59:09 +0800 Subject: [PATCH 02/15] Refactor Type Errors --- issues/324/src/model.rs | 2 +- issues/400/src/model.rs | 2 +- src/error.rs | 21 ++++++++++++++------- src/executor/query.rs | 22 ++++++++++++---------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/issues/324/src/model.rs b/issues/324/src/model.rs index 37174db9..940bc9ca 100644 --- a/issues/324/src/model.rs +++ b/issues/324/src/model.rs @@ -26,7 +26,7 @@ macro_rules! impl_try_from_u64_err { ($newtype: ident) => { impl sea_orm::TryFromU64 for $newtype { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::ConvertFromU64(stringify!($newtype))) + Err(sea_orm::CannotConvertFromU64(stringify!($newtype))) } } }; diff --git a/issues/400/src/model.rs b/issues/400/src/model.rs index 6d9044c4..25e4732f 100644 --- a/issues/400/src/model.rs +++ b/issues/400/src/model.rs @@ -31,7 +31,7 @@ impl From> for Uuid { impl sea_orm::TryFromU64 for AccountId { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::ConvertFromU64(stringify!(AccountId))) + Err(sea_orm::CannotConvertFromU64(stringify!(AccountId))) } } diff --git a/src/error.rs b/src/error.rs index 30b2909d..6d594095 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,15 +5,22 @@ use thiserror::Error; /// An error from unsuccessful database operations #[derive(Error, Debug)] pub enum DbErr { - /// This error happens, when a pool was not able to create a connection + /// This error can happen when the connection pool is fully-utilized #[error("Failed to acquire connection from pool.")] ConnFromPool, - /// Error in case of invalid type conversion attempts - #[error("fail to convert '{0}' into '{1}'")] - CannotConvertInto(String, String), - /// Error in case of invalid type conversion from an u64 - #[error("{0} cannot be converted from u64")] - ConvertFromU64(String), + /// Runtime type conversion error + #[error("Error converting `{from}` into `{into}`: {source}")] + TryIntoErr { + /// From type + from: &'static str, + /// Into type + into: &'static str, + /// TryError + source: Box, + }, + /// Type error: the specified type cannot be converted from u64. This is not a runtime error. + #[error("Type `{0}` cannot be converted from u64")] + CannotConvertFromU64(&'static str), /// After an insert statement it was impossible to retrieve the last_insert_id #[error("Fail to unpack last_insert_id")] InsertCouldNotUnpackInsertId, diff --git a/src/executor/query.rs b/src/executor/query.rs index 0f1b22b2..c2d0a051 100644 --- a/src/executor/query.rs +++ b/src/executor/query.rs @@ -376,13 +376,13 @@ impl TryGetable for Decimal { let val: Option = row .try_get(column.as_str()) .map_err(|e| TryGetError::DbErr(crate::sqlx_error_to_query_err(e)))?; - use rust_decimal::prelude::FromPrimitive; match val { - Some(v) => Decimal::from_f64(v).ok_or_else(|| { - TryGetError::DbErr(DbErr::CannotConvertInto( - "f64".to_owned(), - "Decimal".to_owned(), - )) + Some(v) => Decimal::try_from(v).map_err(|e| { + TryGetError::DbErr(DbErr::TryIntoErr { + from: "f64", + into: "Decimal", + source: Box::new(e), + }) }), None => Err(TryGetError::Null(column)), } @@ -709,7 +709,7 @@ macro_rules! try_from_u64_err { ( $type: ty ) => { impl TryFromU64 for $type { fn try_from_u64(_: u64) -> Result { - Err(DbErr::ConvertFromU64(stringify!($type).to_string())) + Err(DbErr::CannotConvertFromU64(stringify!($type))) } } }; @@ -720,7 +720,7 @@ macro_rules! try_from_u64_err { $( $gen_type: TryFromU64, )* { fn try_from_u64(_: u64) -> Result { - Err(DbErr::ConvertFromU64(stringify!($($gen_type,)*).to_string())) + Err(DbErr::CannotConvertFromU64(stringify!($($gen_type,)*))) } } }; @@ -738,8 +738,10 @@ macro_rules! try_from_u64_numeric { impl TryFromU64 for $type { fn try_from_u64(n: u64) -> Result { use std::convert::TryInto; - n.try_into().map_err(|_| { - DbErr::CannotConvertInto(n.to_string(), stringify!($type).to_string()) + n.try_into().map_err(|e| DbErr::TryIntoErr { + from: stringify!(u64), + into: stringify!($type), + source: Box::new(e), }) } } From 348e841139fcef289c8e60c3d23261e68e443df0 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 28 Aug 2022 13:30:51 +0800 Subject: [PATCH 03/15] Refactor ColumnFromStrErr --- sea-orm-macros/src/derives/column.rs | 15 +++++++++++++-- src/entity/column.rs | 2 +- src/error.rs | 20 ++++++++++---------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/sea-orm-macros/src/derives/column.rs b/sea-orm-macros/src/derives/column.rs index 42e1dfe1..c60a91d6 100644 --- a/sea-orm-macros/src/derives/column.rs +++ b/sea-orm-macros/src/derives/column.rs @@ -71,7 +71,7 @@ pub fn impl_default_as_str(ident: &Ident, data: &Data) -> syn::Result syn::Result { let data_enum = match data { Data::Enum(data_enum) => data_enum, @@ -91,6 +91,17 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result ) }); + let entity_name = data_enum + .variants + .first() + .map(|column| { + let column_iden = column.ident.clone(); + quote!( + #ident::#column_iden.entity_name().to_string() + ) + }) + .unwrap(); + Ok(quote!( #[automatically_derived] impl std::str::FromStr for #ident { @@ -99,7 +110,7 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result fn from_str(s: &str) -> std::result::Result { match s { #(#columns),*, - _ => Err(sea_orm::ColumnFromStrErr(format!("Failed to parse '{}' as `{}`", s, stringify!(#ident)))), + _ => Err(sea_orm::ColumnFromStrErr{ string: s.to_owned(), entity: #entity_name }), } } } diff --git a/src/entity/column.rs b/src/entity/column.rs index 8dbff6ea..6f946f51 100644 --- a/src/entity/column.rs +++ b/src/entity/column.rs @@ -521,7 +521,7 @@ mod tests { )); assert!(matches!( fruit::Column::from_str("does_not_exist"), - Err(crate::ColumnFromStrErr(_)) + Err(crate::ColumnFromStrErr { .. }) )); } diff --git a/src/error.rs b/src/error.rs index 6d594095..a5db227e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -16,7 +16,7 @@ pub enum DbErr { /// Into type into: &'static str, /// TryError - source: Box, + source: Box, }, /// Type error: the specified type cannot be converted from u64. This is not a runtime error. #[error("Type `{0}` cannot be converted from u64")] @@ -69,14 +69,14 @@ impl PartialEq for DbErr { } } -/// An error from a failed column operation when trying to convert the column to a string -#[derive(Debug, Clone)] -pub struct ColumnFromStrErr(pub String); +impl Eq for DbErr {} -impl std::error::Error for ColumnFromStrErr {} - -impl std::fmt::Display for ColumnFromStrErr { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "{}", self.0.as_str()) - } +/// Error during `impl FromStr for Entity::Column` +#[derive(Error, Debug)] +#[error("Failed to match \"{string}\" as Column for `{entity}`")] +pub struct ColumnFromStrErr { + /// Source of error + pub string: String, + /// Entity this column belongs to + pub entity: String, } From 0ce0f495511c981fd09bfd84578bf6f97c45f32b Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 28 Aug 2022 13:51:21 +0800 Subject: [PATCH 04/15] Refactor SqlxError; --- src/database/db_connection.rs | 12 +++++++++--- src/database/mod.rs | 6 +++--- src/driver/sqlx_common.rs | 8 ++++---- src/driver/sqlx_mysql.rs | 2 +- src/driver/sqlx_postgres.rs | 2 +- src/driver/sqlx_sqlite.rs | 2 +- src/error.rs | 27 +++++++++++++++------------ src/executor/query.rs | 4 ++-- 8 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/database/db_connection.rs b/src/database/db_connection.rs index ac4fd7e6..3fa48ec0 100644 --- a/src/database/db_connection.rs +++ b/src/database/db_connection.rs @@ -116,7 +116,9 @@ impl ConnectionTrait for DatabaseConnection { DatabaseConnection::SqlxSqlitePoolConnection(conn) => conn.execute(stmt).await, #[cfg(feature = "mock")] DatabaseConnection::MockDatabaseConnection(conn) => conn.execute(stmt), - DatabaseConnection::Disconnected => Err(DbErr::Conn("Disconnected".to_owned())), + DatabaseConnection::Disconnected => { + Err(DbErr::Conn(RuntimeErr::Internal("Disconnected".to_owned()))) + } } } @@ -132,7 +134,9 @@ impl ConnectionTrait for DatabaseConnection { DatabaseConnection::SqlxSqlitePoolConnection(conn) => conn.query_one(stmt).await, #[cfg(feature = "mock")] DatabaseConnection::MockDatabaseConnection(conn) => conn.query_one(stmt), - DatabaseConnection::Disconnected => Err(DbErr::Conn("Disconnected".to_owned())), + DatabaseConnection::Disconnected => { + Err(DbErr::Conn(RuntimeErr::Internal("Disconnected".to_owned()))) + } } } @@ -148,7 +152,9 @@ impl ConnectionTrait for DatabaseConnection { DatabaseConnection::SqlxSqlitePoolConnection(conn) => conn.query_all(stmt).await, #[cfg(feature = "mock")] DatabaseConnection::MockDatabaseConnection(conn) => conn.query_all(stmt), - DatabaseConnection::Disconnected => Err(DbErr::Conn("Disconnected".to_owned())), + DatabaseConnection::Disconnected => { + Err(DbErr::Conn(RuntimeErr::Internal("Disconnected".to_owned()))) + } } } diff --git a/src/database/mod.rs b/src/database/mod.rs index 4a24ec88..22a173c5 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -18,7 +18,7 @@ pub use stream::*; use tracing::instrument; pub use transaction::*; -use crate::DbErr; +use crate::{DbErr, RuntimeErr}; /// Defines a database #[derive(Debug, Default)] @@ -73,10 +73,10 @@ impl Database { if crate::MockDatabaseConnector::accepts(&opt.url) { return crate::MockDatabaseConnector::connect(&opt.url).await; } - Err(DbErr::Conn(format!( + Err(DbErr::Conn(RuntimeErr::Internal(format!( "The connection string '{}' has no supporting driver.", opt.url - ))) + )))) } } diff --git a/src/driver/sqlx_common.rs b/src/driver/sqlx_common.rs index 91509813..37a250ed 100644 --- a/src/driver/sqlx_common.rs +++ b/src/driver/sqlx_common.rs @@ -1,16 +1,16 @@ -use crate::DbErr; +use crate::{DbErr, RuntimeErr}; /// Converts an [sqlx::error] execution error to a [DbErr] pub fn sqlx_error_to_exec_err(err: sqlx::Error) -> DbErr { - DbErr::ExecSqlX(err) + DbErr::Exec(RuntimeErr::SqlxError(err)) } /// Converts an [sqlx::error] query error to a [DbErr] pub fn sqlx_error_to_query_err(err: sqlx::Error) -> DbErr { - DbErr::QuerySqlX(err) + DbErr::Query(RuntimeErr::SqlxError(err)) } /// Converts an [sqlx::error] connection error to a [DbErr] pub fn sqlx_error_to_conn_err(err: sqlx::Error) -> DbErr { - DbErr::ConnSqlX(err) + DbErr::Conn(RuntimeErr::SqlxError(err)) } diff --git a/src/driver/sqlx_mysql.rs b/src/driver/sqlx_mysql.rs index e1f1d416..c15197a4 100644 --- a/src/driver/sqlx_mysql.rs +++ b/src/driver/sqlx_mysql.rs @@ -45,7 +45,7 @@ impl SqlxMySqlConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::ConnSqlX(e))?; + .map_err(sqlx_error_to_conn_err)?; use sqlx::ConnectOptions; if !options.sqlx_logging { opt.disable_statement_logging(); diff --git a/src/driver/sqlx_postgres.rs b/src/driver/sqlx_postgres.rs index 6d070115..78cc4f3c 100644 --- a/src/driver/sqlx_postgres.rs +++ b/src/driver/sqlx_postgres.rs @@ -45,7 +45,7 @@ impl SqlxPostgresConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::ConnSqlX(e))?; + .map_err(sqlx_error_to_conn_err)?; use sqlx::ConnectOptions; if !options.sqlx_logging { opt.disable_statement_logging(); diff --git a/src/driver/sqlx_sqlite.rs b/src/driver/sqlx_sqlite.rs index 55e768b0..f2b17a95 100644 --- a/src/driver/sqlx_sqlite.rs +++ b/src/driver/sqlx_sqlite.rs @@ -46,7 +46,7 @@ impl SqlxSqliteConnector { let mut opt = options .url .parse::() - .map_err(|e| DbErr::ConnSqlX(e))?; + .map_err(sqlx_error_to_conn_err)?; if options.sqlcipher_key.is_some() { opt = opt.pragma("key", options.sqlcipher_key.clone().unwrap()); } diff --git a/src/error.rs b/src/error.rs index a5db227e..26bd5ddc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,22 +30,13 @@ pub enum DbErr { UpdateCouldNotGetPrimaryKey, /// There was a problem with the database connection #[error("Connection Error: {0}")] - Conn(String), - /// There was a problem with the database connection from sqlx - #[cfg(feature = "sqlx-dep")] - #[error("Connection Error: {0}")] - ConnSqlX(#[source] SqlxError), + Conn(#[source] RuntimeErr), /// An operation did not execute successfully - #[cfg(feature = "sqlx-dep")] #[error("Execution Error: {0}")] - ExecSqlX(#[source] SqlxError), - /// An error occurred while performing a query, with more details from sqlx - #[cfg(feature = "sqlx-dep")] - #[error("Query Error: {0}")] - QuerySqlX(#[source] SqlxError), + Exec(#[source] RuntimeErr), /// An error occurred while performing a query #[error("Query Error: {0}")] - Query(String), + Query(#[source] RuntimeErr), /// The record was not found in the database #[error("RecordNotFound Error: {0}")] RecordNotFound(String), @@ -63,6 +54,18 @@ pub enum DbErr { Migration(String), } +/// Runtime error +#[derive(Error, Debug)] +pub enum RuntimeErr { + #[cfg(feature = "sqlx-dep")] + /// SQLx Error + #[error("{0}")] + SqlxError(SqlxError), + #[error("{0}")] + /// Error generated from within SeaORM + Internal(String), +} + impl PartialEq for DbErr { fn eq(&self, other: &Self) -> bool { self.to_string() == other.to_string() diff --git a/src/executor/query.rs b/src/executor/query.rs index c2d0a051..f27b1a2d 100644 --- a/src/executor/query.rs +++ b/src/executor/query.rs @@ -41,7 +41,7 @@ impl From for DbErr { match e { TryGetError::DbErr(e) => e, TryGetError::Null(s) => { - DbErr::Query(format!("error occurred while decoding {}: Null", s)) + DbErr::Type(format!("A null value was encountered while decoding {}", s)) } } } @@ -627,7 +627,7 @@ where fn try_get_many_with_slice_len_of(len: usize, cols: &[String]) -> Result<(), TryGetError> { if cols.len() < len { - Err(TryGetError::DbErr(DbErr::Query(format!( + Err(TryGetError::DbErr(DbErr::Type(format!( "Expect {} column names supplied but got slice of length {}", len, cols.len() From 85533a3bb3f9cfdab92212ce6c8d3ad1ac468dd3 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sun, 28 Aug 2022 14:24:24 +0800 Subject: [PATCH 05/15] Give up and fix tests --- sea-orm-macros/src/derives/column.rs | 13 +------------ src/database/mock.rs | 10 +++++++--- src/entity/column.rs | 2 +- src/error.rs | 9 ++------- src/executor/query.rs | 12 +++++++----- tests/crud/error.rs | 6 +++--- tests/transaction_tests.rs | 12 +++++++++--- 7 files changed, 30 insertions(+), 34 deletions(-) diff --git a/sea-orm-macros/src/derives/column.rs b/sea-orm-macros/src/derives/column.rs index c60a91d6..3a0f9314 100644 --- a/sea-orm-macros/src/derives/column.rs +++ b/sea-orm-macros/src/derives/column.rs @@ -91,17 +91,6 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result ) }); - let entity_name = data_enum - .variants - .first() - .map(|column| { - let column_iden = column.ident.clone(); - quote!( - #ident::#column_iden.entity_name().to_string() - ) - }) - .unwrap(); - Ok(quote!( #[automatically_derived] impl std::str::FromStr for #ident { @@ -110,7 +99,7 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result fn from_str(s: &str) -> std::result::Result { match s { #(#columns),*, - _ => Err(sea_orm::ColumnFromStrErr{ string: s.to_owned(), entity: #entity_name }), + _ => Err(sea_orm::ColumnFromStrErr(s.to_owned())), } } } diff --git a/src/database/mock.rs b/src/database/mock.rs index a1dbc795..2c5b8d38 100644 --- a/src/database/mock.rs +++ b/src/database/mock.rs @@ -102,7 +102,9 @@ impl MockDatabaseTrait for MockDatabase { result: ExecResultHolder::Mock(std::mem::take(&mut self.exec_results[counter])), }) } else { - Err(DbErr::Query("`exec_results` buffer is empty.".to_owned())) + Err(DbErr::Query(RuntimeErr::Internal( + "`exec_results` buffer is empty.".to_owned(), + ))) } } @@ -121,7 +123,9 @@ impl MockDatabaseTrait for MockDatabase { }) .collect()) } else { - Err(DbErr::Query("`query_results` buffer is empty.".to_owned())) + Err(DbErr::Query(RuntimeErr::Internal( + "`query_results` buffer is empty.".to_owned(), + ))) } } @@ -176,7 +180,7 @@ impl MockRow { where T: ValueType, { - T::try_from(self.values.get(col).unwrap().clone()).map_err(|e| DbErr::Query(e.to_string())) + T::try_from(self.values.get(col).unwrap().clone()).map_err(|e| DbErr::Type(e.to_string())) } /// An iterator over the keys and values of a mock row diff --git a/src/entity/column.rs b/src/entity/column.rs index 6f946f51..8dbff6ea 100644 --- a/src/entity/column.rs +++ b/src/entity/column.rs @@ -521,7 +521,7 @@ mod tests { )); assert!(matches!( fruit::Column::from_str("does_not_exist"), - Err(crate::ColumnFromStrErr { .. }) + Err(crate::ColumnFromStrErr(_)) )); } diff --git a/src/error.rs b/src/error.rs index 26bd5ddc..34a828fb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -76,10 +76,5 @@ impl Eq for DbErr {} /// Error during `impl FromStr for Entity::Column` #[derive(Error, Debug)] -#[error("Failed to match \"{string}\" as Column for `{entity}`")] -pub struct ColumnFromStrErr { - /// Source of error - pub string: String, - /// Entity this column belongs to - pub entity: String, -} +#[error("Failed to match \"{0}\" as Column")] +pub struct ColumnFromStrErr(pub String); diff --git a/src/executor/query.rs b/src/executor/query.rs index f27b1a2d..90697fb3 100644 --- a/src/executor/query.rs +++ b/src/executor/query.rs @@ -816,20 +816,22 @@ try_from_u64_err!(uuid::Uuid); #[cfg(test)] mod tests { use super::TryGetError; - use crate::error::DbErr; + use crate::error::*; #[test] fn from_try_get_error() { // TryGetError::DbErr - let try_get_error = TryGetError::DbErr(DbErr::Query("expected error message".to_owned())); + let try_get_error = TryGetError::DbErr(DbErr::Query(RuntimeErr::Internal( + "expected error message".to_owned(), + ))); assert_eq!( DbErr::from(try_get_error), - DbErr::Query("expected error message".to_owned()) + DbErr::Query(RuntimeErr::Internal("expected error message".to_owned())) ); // TryGetError::Null let try_get_error = TryGetError::Null("column".to_owned()); - let expected = "error occurred while decoding column: Null".to_owned(); - assert_eq!(DbErr::from(try_get_error), DbErr::Query(expected)); + let expected = "A null value was encountered while decoding column".to_owned(); + assert_eq!(DbErr::from(try_get_error), DbErr::Type(expected)); } } diff --git a/tests/crud/error.rs b/tests/crud/error.rs index 8dfab3fa..4828d9cd 100644 --- a/tests/crud/error.rs +++ b/tests/crud/error.rs @@ -1,6 +1,6 @@ pub use super::*; use rust_decimal_macros::dec; -use sea_orm::DbErr; +use sea_orm::error::*; #[cfg(any( feature = "sqlx-mysql", feature = "sqlx-sqlite", @@ -32,7 +32,7 @@ pub async fn test_cake_error_sqlx(db: &DbConn) { #[cfg(any(feature = "sqlx-mysql", feature = "sqlx-sqlite"))] match error { - DbErr::ExecSqlX(sqlx_error) => match sqlx_error { + DbErr::Exec(RuntimeErr::SqlxError(error)) => match error { Error::Database(e) => { #[cfg(feature = "sqlx-mysql")] assert_eq!(e.code().unwrap(), "23000"); @@ -45,7 +45,7 @@ pub async fn test_cake_error_sqlx(db: &DbConn) { } #[cfg(feature = "sqlx-postgres")] match error { - DbErr::QuerySqlX(sqlx_error) => match sqlx_error { + DbErr::Query(RuntimeErr::SqlxError(error)) => match error { Error::Database(e) => { assert_eq!(e.code().unwrap(), "23505"); } diff --git a/tests/transaction_tests.rs b/tests/transaction_tests.rs index 1059da8e..9faab72a 100644 --- a/tests/transaction_tests.rs +++ b/tests/transaction_tests.rs @@ -508,7 +508,9 @@ pub async fn transaction_nested() { assert_eq!(bakeries.len(), 4); if true { - Err(DbErr::Query("Force Rollback!".to_owned())) + Err(DbErr::Query(RuntimeErr::Internal( + "Force Rollback!".to_owned(), + ))) } else { Ok(()) } @@ -633,7 +635,9 @@ pub async fn transaction_nested() { assert_eq!(bakeries.len(), 7); if true { - Err(DbErr::Query("Force Rollback!".to_owned())) + Err(DbErr::Query(RuntimeErr::Internal( + "Force Rollback!".to_owned(), + ))) } else { Ok(()) } @@ -652,7 +656,9 @@ pub async fn transaction_nested() { assert_eq!(bakeries.len(), 6); if true { - Err(DbErr::Query("Force Rollback!".to_owned())) + Err(DbErr::Query(RuntimeErr::Internal( + "Force Rollback!".to_owned(), + ))) } else { Ok(()) } From 0efdfc6742dd49b9b26d72a3102ea2df99a2454f Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 1 Sep 2022 15:57:58 +0800 Subject: [PATCH 06/15] Typo --- src/database/mock.rs | 2 +- src/executor/update.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/database/mock.rs b/src/database/mock.rs index 2c5b8d38..1677938f 100644 --- a/src/database/mock.rs +++ b/src/database/mock.rs @@ -102,7 +102,7 @@ impl MockDatabaseTrait for MockDatabase { result: ExecResultHolder::Mock(std::mem::take(&mut self.exec_results[counter])), }) } else { - Err(DbErr::Query(RuntimeErr::Internal( + Err(DbErr::Exec(RuntimeErr::Internal( "`exec_results` buffer is empty.".to_owned(), ))) } diff --git a/src/executor/update.rs b/src/executor/update.rs index cd9337f7..1edd9461 100644 --- a/src/executor/update.rs +++ b/src/executor/update.rs @@ -131,7 +131,7 @@ where match found { Some(model) => Ok(model), None => Err(DbErr::RecordNotFound( - "Failed to find inserted item".to_owned(), + "Failed to find updated item".to_owned(), )), } } From 4e835f2ee1e962e3c5a5d737a5919bca2b9e42da Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 1 Sep 2022 16:38:47 +0800 Subject: [PATCH 07/15] . --- sea-orm-macros/src/derives/column.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/sea-orm-macros/src/derives/column.rs b/sea-orm-macros/src/derives/column.rs index 3a0f9314..6d4e212b 100644 --- a/sea-orm-macros/src/derives/column.rs +++ b/sea-orm-macros/src/derives/column.rs @@ -90,6 +90,7 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result #column_str_snake | #column_str_mixed => Ok(#ident::#column_iden) ) }); + Ok(quote!( #[automatically_derived] From 6564ddac15488c990609fa16689f8813d1b280fa Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 1 Sep 2022 16:39:18 +0800 Subject: [PATCH 08/15] Testing [issues] [cli] --- sea-orm-macros/src/derives/column.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/sea-orm-macros/src/derives/column.rs b/sea-orm-macros/src/derives/column.rs index 6d4e212b..3a0f9314 100644 --- a/sea-orm-macros/src/derives/column.rs +++ b/sea-orm-macros/src/derives/column.rs @@ -90,7 +90,6 @@ pub fn impl_col_from_str(ident: &Ident, data: &Data) -> syn::Result #column_str_snake | #column_str_mixed => Ok(#ident::#column_iden) ) }); - Ok(quote!( #[automatically_derived] From 5f3210c62a52822ee1e88898b63c8cb48cbf16b7 Mon Sep 17 00:00:00 2001 From: Billy Chan Date: Thu, 1 Sep 2022 16:52:03 +0800 Subject: [PATCH 09/15] Fix [issues] --- issues/324/src/model.rs | 2 +- issues/400/src/model.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/issues/324/src/model.rs b/issues/324/src/model.rs index 940bc9ca..1a2720be 100644 --- a/issues/324/src/model.rs +++ b/issues/324/src/model.rs @@ -26,7 +26,7 @@ macro_rules! impl_try_from_u64_err { ($newtype: ident) => { impl sea_orm::TryFromU64 for $newtype { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::CannotConvertFromU64(stringify!($newtype))) + Err(sea_orm::DbErr::CannotConvertFromU64(stringify!($newtype))) } } }; diff --git a/issues/400/src/model.rs b/issues/400/src/model.rs index 25e4732f..134397ac 100644 --- a/issues/400/src/model.rs +++ b/issues/400/src/model.rs @@ -31,7 +31,7 @@ impl From> for Uuid { impl sea_orm::TryFromU64 for AccountId { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::CannotConvertFromU64(stringify!(AccountId))) + Err(sea_orm::DbErr::CannotConvertFromU64(stringify!(AccountId))) } } From 1c8ef335440357c3131879fed94c9f1d5b2098e2 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sat, 3 Sep 2022 20:44:45 +0800 Subject: [PATCH 10/15] Update src/error.rs Co-authored-by: Sculas --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 34a828fb..f4851d92 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,7 +22,7 @@ pub enum DbErr { #[error("Type `{0}` cannot be converted from u64")] CannotConvertFromU64(&'static str), /// After an insert statement it was impossible to retrieve the last_insert_id - #[error("Fail to unpack last_insert_id")] + #[error("Failed to unpack last_insert_id")] InsertCouldNotUnpackInsertId, /// When updating, a model should know it's primary key to check /// if the record has been correctly updated, otherwise this error will occur From 4d95218430e7251ba51d3a071f53d0a69d93d971 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sat, 3 Sep 2022 20:44:59 +0800 Subject: [PATCH 11/15] Update src/error.rs Co-authored-by: Sculas --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index f4851d92..02920737 100644 --- a/src/error.rs +++ b/src/error.rs @@ -26,7 +26,7 @@ pub enum DbErr { InsertCouldNotUnpackInsertId, /// When updating, a model should know it's primary key to check /// if the record has been correctly updated, otherwise this error will occur - #[error("Fail to get primary key from model")] + #[error("Failed to get primary key from model")] UpdateCouldNotGetPrimaryKey, /// There was a problem with the database connection #[error("Connection Error: {0}")] From 4993c6ab2df095494e19f8f5379e25178b201161 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sat, 3 Sep 2022 20:45:09 +0800 Subject: [PATCH 12/15] Update src/error.rs Co-authored-by: Sculas --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 02920737..813f4545 100644 --- a/src/error.rs +++ b/src/error.rs @@ -6,7 +6,7 @@ use thiserror::Error; #[derive(Error, Debug)] pub enum DbErr { /// This error can happen when the connection pool is fully-utilized - #[error("Failed to acquire connection from pool.")] + #[error("Failed to acquire connection from pool")] ConnFromPool, /// Runtime type conversion error #[error("Error converting `{from}` into `{into}`: {source}")] From 36f09c524e1886b869405cb333eb41d68785be58 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Sat, 3 Sep 2022 20:45:17 +0800 Subject: [PATCH 13/15] Update src/error.rs Co-authored-by: Sculas --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 813f4545..3f2e5375 100644 --- a/src/error.rs +++ b/src/error.rs @@ -19,7 +19,7 @@ pub enum DbErr { source: Box, }, /// Type error: the specified type cannot be converted from u64. This is not a runtime error. - #[error("Type `{0}` cannot be converted from u64")] + #[error("Type '{0}' cannot be converted from u64")] CannotConvertFromU64(&'static str), /// After an insert statement it was impossible to retrieve the last_insert_id #[error("Failed to unpack last_insert_id")] From 122525543167c6cb544a537cb7c4dc0b9abafa2f Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Wed, 14 Sep 2022 00:28:24 +0800 Subject: [PATCH 14/15] Address comments --- issues/324/src/model.rs | 2 +- issues/400/src/model.rs | 2 +- src/driver/sqlx_mysql.rs | 12 ++++++------ src/driver/sqlx_postgres.rs | 12 ++++++------ src/driver/sqlx_sqlite.rs | 12 ++++++------ src/error.rs | 12 ++++++------ src/executor/insert.rs | 2 +- src/executor/query.rs | 4 ++-- src/executor/update.rs | 2 +- 9 files changed, 30 insertions(+), 30 deletions(-) diff --git a/issues/324/src/model.rs b/issues/324/src/model.rs index 1a2720be..8b335c25 100644 --- a/issues/324/src/model.rs +++ b/issues/324/src/model.rs @@ -26,7 +26,7 @@ macro_rules! impl_try_from_u64_err { ($newtype: ident) => { impl sea_orm::TryFromU64 for $newtype { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::DbErr::CannotConvertFromU64(stringify!($newtype))) + Err(sea_orm::DbErr::ConvertFromU64(stringify!($newtype))) } } }; diff --git a/issues/400/src/model.rs b/issues/400/src/model.rs index 134397ac..28a44d64 100644 --- a/issues/400/src/model.rs +++ b/issues/400/src/model.rs @@ -31,7 +31,7 @@ impl From> for Uuid { impl sea_orm::TryFromU64 for AccountId { fn try_from_u64(_n: u64) -> Result { - Err(sea_orm::DbErr::CannotConvertFromU64(stringify!(AccountId))) + Err(sea_orm::DbErr::ConvertFromU64(stringify!(AccountId))) } } diff --git a/src/driver/sqlx_mysql.rs b/src/driver/sqlx_mysql.rs index c15197a4..e3d0ddbf 100644 --- a/src/driver/sqlx_mysql.rs +++ b/src/driver/sqlx_mysql.rs @@ -89,7 +89,7 @@ impl SqlxMySqlPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -110,7 +110,7 @@ impl SqlxMySqlPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -128,7 +128,7 @@ impl SqlxMySqlPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -144,7 +144,7 @@ impl SqlxMySqlPoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -154,7 +154,7 @@ impl SqlxMySqlPoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_mysql(conn, self.metric_callback.clone()).await } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -175,7 +175,7 @@ impl SqlxMySqlPoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::ConnFromPool)) + Err(TransactionError::Connection(DbErr::ConnectionAcquire)) } } diff --git a/src/driver/sqlx_postgres.rs b/src/driver/sqlx_postgres.rs index 78cc4f3c..085abaa8 100644 --- a/src/driver/sqlx_postgres.rs +++ b/src/driver/sqlx_postgres.rs @@ -89,7 +89,7 @@ impl SqlxPostgresPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -110,7 +110,7 @@ impl SqlxPostgresPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -128,7 +128,7 @@ impl SqlxPostgresPoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -144,7 +144,7 @@ impl SqlxPostgresPoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -154,7 +154,7 @@ impl SqlxPostgresPoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_postgres(conn, self.metric_callback.clone()).await } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -175,7 +175,7 @@ impl SqlxPostgresPoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::ConnFromPool)) + Err(TransactionError::Connection(DbErr::ConnectionAcquire)) } } diff --git a/src/driver/sqlx_sqlite.rs b/src/driver/sqlx_sqlite.rs index f2b17a95..36e9dfa4 100644 --- a/src/driver/sqlx_sqlite.rs +++ b/src/driver/sqlx_sqlite.rs @@ -96,7 +96,7 @@ impl SqlxSqlitePoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -117,7 +117,7 @@ impl SqlxSqlitePoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -135,7 +135,7 @@ impl SqlxSqlitePoolConnection { } }) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -151,7 +151,7 @@ impl SqlxSqlitePoolConnection { self.metric_callback.clone(), ))) } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -161,7 +161,7 @@ impl SqlxSqlitePoolConnection { if let Ok(conn) = self.pool.acquire().await { DatabaseTransaction::new_sqlite(conn, self.metric_callback.clone()).await } else { - Err(DbErr::ConnFromPool) + Err(DbErr::ConnectionAcquire) } } @@ -182,7 +182,7 @@ impl SqlxSqlitePoolConnection { .map_err(|e| TransactionError::Connection(e))?; transaction.run(callback).await } else { - Err(TransactionError::Connection(DbErr::ConnFromPool)) + Err(TransactionError::Connection(DbErr::ConnectionAcquire)) } } diff --git a/src/error.rs b/src/error.rs index 3f2e5375..e45b3834 100644 --- a/src/error.rs +++ b/src/error.rs @@ -7,7 +7,7 @@ use thiserror::Error; pub enum DbErr { /// This error can happen when the connection pool is fully-utilized #[error("Failed to acquire connection from pool")] - ConnFromPool, + ConnectionAcquire, /// Runtime type conversion error #[error("Error converting `{from}` into `{into}`: {source}")] TryIntoErr { @@ -20,14 +20,14 @@ pub enum DbErr { }, /// Type error: the specified type cannot be converted from u64. This is not a runtime error. #[error("Type '{0}' cannot be converted from u64")] - CannotConvertFromU64(&'static str), + ConvertFromU64(&'static str), /// After an insert statement it was impossible to retrieve the last_insert_id #[error("Failed to unpack last_insert_id")] - InsertCouldNotUnpackInsertId, + UnpackInsertId, /// When updating, a model should know it's primary key to check /// if the record has been correctly updated, otherwise this error will occur #[error("Failed to get primary key from model")] - UpdateCouldNotGetPrimaryKey, + UpdateGetPrimeryKey, /// There was a problem with the database connection #[error("Connection Error: {0}")] Conn(#[source] RuntimeErr), @@ -57,12 +57,12 @@ pub enum DbErr { /// Runtime error #[derive(Error, Debug)] pub enum RuntimeErr { - #[cfg(feature = "sqlx-dep")] /// SQLx Error + #[cfg(feature = "sqlx-dep")] #[error("{0}")] SqlxError(SqlxError), - #[error("{0}")] /// Error generated from within SeaORM + #[error("{0}")] Internal(String), } diff --git a/src/executor/insert.rs b/src/executor/insert.rs index f7bb0629..33d0d4ed 100644 --- a/src/executor/insert.rs +++ b/src/executor/insert.rs @@ -130,7 +130,7 @@ where Some(value_tuple) => FromValueTuple::from_value_tuple(value_tuple), None => match last_insert_id_opt { Some(last_insert_id) => last_insert_id, - None => return Err(DbErr::InsertCouldNotUnpackInsertId), + None => return Err(DbErr::UnpackInsertId), }, }; Ok(InsertResult { last_insert_id }) diff --git a/src/executor/query.rs b/src/executor/query.rs index 90697fb3..3b53542b 100644 --- a/src/executor/query.rs +++ b/src/executor/query.rs @@ -709,7 +709,7 @@ macro_rules! try_from_u64_err { ( $type: ty ) => { impl TryFromU64 for $type { fn try_from_u64(_: u64) -> Result { - Err(DbErr::CannotConvertFromU64(stringify!($type))) + Err(DbErr::ConvertFromU64(stringify!($type))) } } }; @@ -720,7 +720,7 @@ macro_rules! try_from_u64_err { $( $gen_type: TryFromU64, )* { fn try_from_u64(_: u64) -> Result { - Err(DbErr::CannotConvertFromU64(stringify!($($gen_type,)*))) + Err(DbErr::ConvertFromU64(stringify!($($gen_type,)*))) } } }; diff --git a/src/executor/update.rs b/src/executor/update.rs index 1edd9461..7fa8b242 100644 --- a/src/executor/update.rs +++ b/src/executor/update.rs @@ -122,7 +122,7 @@ where Updater::new(query).check_record_exists().exec(db).await?; let primary_key_value = match model.get_primary_key_value() { Some(val) => FromValueTuple::from_value_tuple(val), - None => return Err(DbErr::UpdateCouldNotGetPrimaryKey), + None => return Err(DbErr::UpdateGetPrimeryKey), }; let found = ::find_by_id(primary_key_value) .one(db) From a44017f679cf66e3b15c8c987afca7334b9641f1 Mon Sep 17 00:00:00 2001 From: Chris Tsang Date: Fri, 7 Oct 2022 00:15:36 +0800 Subject: [PATCH 15/15] Reorder variants --- src/error.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/error.rs b/src/error.rs index e45b3834..01c4f21f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -18,6 +18,15 @@ pub enum DbErr { /// TryError source: Box, }, + /// There was a problem with the database connection + #[error("Connection Error: {0}")] + Conn(#[source] RuntimeErr), + /// An operation did not execute successfully + #[error("Execution Error: {0}")] + Exec(#[source] RuntimeErr), + /// An error occurred while performing a query + #[error("Query Error: {0}")] + Query(#[source] RuntimeErr), /// Type error: the specified type cannot be converted from u64. This is not a runtime error. #[error("Type '{0}' cannot be converted from u64")] ConvertFromU64(&'static str), @@ -28,15 +37,6 @@ pub enum DbErr { /// if the record has been correctly updated, otherwise this error will occur #[error("Failed to get primary key from model")] UpdateGetPrimeryKey, - /// There was a problem with the database connection - #[error("Connection Error: {0}")] - Conn(#[source] RuntimeErr), - /// An operation did not execute successfully - #[error("Execution Error: {0}")] - Exec(#[source] RuntimeErr), - /// An error occurred while performing a query - #[error("Query Error: {0}")] - Query(#[source] RuntimeErr), /// The record was not found in the database #[error("RecordNotFound Error: {0}")] RecordNotFound(String),