From ce3ce1ccbf28e4a8100dc78ed4e26dada2a299ef Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Thu, 31 May 2018 22:30:25 -0400 Subject: [PATCH 01/11] Convert sql/ and query-sql/ to failure. sql/query-sql --- query-sql/src/lib.rs | 3 ++- sql/Cargo.toml | 3 ++- sql/src/lib.rs | 31 +++++++++++-------------------- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/query-sql/src/lib.rs b/query-sql/src/lib.rs index 2bba08eb..47a1655c 100644 --- a/query-sql/src/lib.rs +++ b/query-sql/src/lib.rs @@ -42,6 +42,7 @@ use mentat_sql::{ BuildQueryResult, QueryBuilder, QueryFragment, + SQLError, SQLiteQueryBuilder, SQLQuery, }; @@ -629,7 +630,7 @@ impl QueryFragment for SelectQuery { } impl SelectQuery { - pub fn to_sql_query(&self) -> mentat_sql::Result { + pub fn to_sql_query(&self) -> Result { let mut builder = SQLiteQueryBuilder::new(); self.push_sql(&mut builder).map(|_| builder.finish()) } diff --git a/sql/Cargo.toml b/sql/Cargo.toml index f762bae5..5385a41e 100644 --- a/sql/Cargo.toml +++ b/sql/Cargo.toml @@ -4,7 +4,8 @@ version = "0.0.1" workspace = ".." [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" ordered-float = "0.5" [dependencies.rusqlite] diff --git a/sql/src/lib.rs b/sql/src/lib.rs index 6e2e845a..8893c341 100644 --- a/sql/src/lib.rs +++ b/sql/src/lib.rs @@ -7,9 +7,9 @@ // under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. +extern crate failure; -#[macro_use] -extern crate error_chain; +#[macro_use] extern crate failure_derive; extern crate ordered_float; extern crate rusqlite; @@ -29,25 +29,16 @@ use mentat_core::{ pub use rusqlite::types::Value; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } +#[derive(Debug, Fail)] +pub enum SQLError { + #[fail(display = "invalid parameter name: {}", _0)] + InvalidParameterName(String), - errors { - InvalidParameterName(name: String) { - description("invalid parameter name") - display("invalid parameter name: '{}'", name) - } - - BindParamCouldBeGenerated(name: String) { - description("parameter name could be generated") - display("parameter name could be generated: '{}'", name) - } - } + #[fail(display = "parameter name could be generated: '{}'", _0)] + BindParamCouldBeGenerated(String) } -pub type BuildQueryResult = Result<()>; +pub type BuildQueryResult = Result<(), SQLError>; /// We want to accumulate values that will later be substituted into a SQL statement execution. /// This struct encapsulates the generated string and the _initial_ argument list. @@ -213,12 +204,12 @@ impl QueryBuilder for SQLiteQueryBuilder { // Do some validation first. // This is not free, but it's probably worth it for now. if !name.chars().all(|c| char::is_alphanumeric(c) || c == '_') { - bail!(ErrorKind::InvalidParameterName(name.to_string())); + return Err(SQLError::InvalidParameterName(name.to_string())) } if name.starts_with(self.arg_prefix.as_str()) && name.chars().skip(self.arg_prefix.len()).all(char::is_numeric) { - bail!(ErrorKind::BindParamCouldBeGenerated(name.to_string())); + return Err(SQLError::BindParamCouldBeGenerated(name.to_string())) } self.push_sql("$"); From 326fe881a0f7932bb8fe19225a7a06f375b18cea Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Thu, 31 May 2018 22:31:21 -0400 Subject: [PATCH 02/11] Convert query-algebrizer/ to failure. --- query-algebrizer/Cargo.toml | 3 +- query-algebrizer/src/clauses/convert.rs | 12 +- query-algebrizer/src/clauses/fulltext.rs | 44 +++--- query-algebrizer/src/clauses/ground.rs | 27 ++-- query-algebrizer/src/clauses/inputs.rs | 4 +- query-algebrizer/src/clauses/mod.rs | 5 +- query-algebrizer/src/clauses/not.rs | 14 +- query-algebrizer/src/clauses/predicate.rs | 14 +- query-algebrizer/src/clauses/resolve.rs | 23 ++- query-algebrizer/src/clauses/tx_log_api.rs | 27 ++-- query-algebrizer/src/clauses/where_fn.rs | 4 +- query-algebrizer/src/errors.rs | 174 +++++++++++---------- query-algebrizer/src/lib.rs | 22 +-- query-algebrizer/src/validate.rs | 8 +- query-algebrizer/tests/ground.rs | 38 +++-- query-algebrizer/tests/predicate.rs | 10 +- query-algebrizer/tests/utils/mod.rs | 4 +- 17 files changed, 222 insertions(+), 211 deletions(-) diff --git a/query-algebrizer/Cargo.toml b/query-algebrizer/Cargo.toml index d544a29d..5d44143e 100644 --- a/query-algebrizer/Cargo.toml +++ b/query-algebrizer/Cargo.toml @@ -4,7 +4,8 @@ version = "0.0.1" workspace = ".." [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" [dependencies.mentat_core] path = "../core" diff --git a/query-algebrizer/src/clauses/convert.rs b/query-algebrizer/src/clauses/convert.rs index 9c9c1ab1..f4a1ad93 100644 --- a/query-algebrizer/src/clauses/convert.rs +++ b/query-algebrizer/src/clauses/convert.rs @@ -28,7 +28,7 @@ use clauses::{ }; use errors::{ - ErrorKind, + AlgebrizerError, Result, }; @@ -80,12 +80,12 @@ impl ValueTypes for FnArg { &FnArg::Constant(NonIntegerConstant::BigInteger(_)) => { // Not yet implemented. - bail!(ErrorKind::UnsupportedArgument) + bail!(AlgebrizerError::UnsupportedArgument) }, // These don't make sense here. TODO: split FnArg into scalar and non-scalar… &FnArg::Vector(_) | - &FnArg::SrcVar(_) => bail!(ErrorKind::UnsupportedArgument), + &FnArg::SrcVar(_) => bail!(AlgebrizerError::UnsupportedArgument), // These are all straightforward. &FnArg::Constant(NonIntegerConstant::Boolean(_)) => ValueTypeSet::of_one(ValueType::Boolean), @@ -196,7 +196,7 @@ impl ConjoiningClauses { FnArg::Variable(in_var) => { // TODO: technically you could ground an existing variable inside the query…. if !self.input_variables.contains(&in_var) { - bail!(ErrorKind::UnboundVariable((*in_var.0).clone())); + bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone())) } match self.bound_value(&in_var) { // The type is already known if it's a bound variable…. @@ -205,7 +205,7 @@ impl ConjoiningClauses { // The variable is present in `:in`, but it hasn't yet been provided. // This is a restriction we will eventually relax: we don't yet have a way // to collect variables as part of a computed table or substitution. - bail!(ErrorKind::UnboundVariable((*in_var.0).clone())) + bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone())) }, } }, @@ -215,7 +215,7 @@ impl ConjoiningClauses { // These don't make sense here. FnArg::Vector(_) | - FnArg::SrcVar(_) => bail!(ErrorKind::InvalidGroundConstant), + FnArg::SrcVar(_) => bail!(AlgebrizerError::InvalidGroundConstant), // These are all straightforward. FnArg::Constant(NonIntegerConstant::Boolean(x)) => { diff --git a/query-algebrizer/src/clauses/fulltext.rs b/query-algebrizer/src/clauses/fulltext.rs index 1fbae1f2..5e97fb52 100644 --- a/query-algebrizer/src/clauses/fulltext.rs +++ b/query-algebrizer/src/clauses/fulltext.rs @@ -30,8 +30,9 @@ use clauses::{ }; use errors::{ + AlgebrizerError, BindingError, - ErrorKind, + InvalidBinding, Result, }; @@ -53,17 +54,17 @@ impl ConjoiningClauses { #[allow(unused_variables)] pub(crate) fn apply_fulltext(&mut self, known: Known, where_fn: WhereFn) -> Result<()> { if where_fn.args.len() != 3 { - bail!(ErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3)); + bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3)); } if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have exactly four bindings. Destructure them now. @@ -71,17 +72,18 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count < 1 || bindings_count > 4 { - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), - BindingError::InvalidNumberOfBindings { - number: bindings.len(), - expected: 4, - })); + bail!(InvalidBinding::new(where_fn.operator.clone(), + BindingError::InvalidNumberOfBindings { + number: bindings.len(), + expected: 4, + }) + ); } bindings }, Binding::BindScalar(_) | Binding::BindTuple(_) | - Binding::BindColl(_) => bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)), + Binding::BindColl(_) => bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRel)), }; let mut bindings = bindings.into_iter(); let b_entity = bindings.next().unwrap(); @@ -94,7 +96,7 @@ impl ConjoiningClauses { // TODO: process source variables. match args.next().unwrap() { FnArg::SrcVar(SrcVar::DefaultSrc) => {}, - _ => bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), + _ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), } let schema = known.schema; @@ -114,10 +116,10 @@ impl ConjoiningClauses { match self.bound_value(&v) { Some(TypedValue::Ref(entid)) => Some(entid), Some(tv) => { - bail!(ErrorKind::InputTypeDisagreement(v.name().clone(), ValueType::Ref, tv.value_type())); + bail!(AlgebrizerError::InputTypeDisagreement(v.name().clone(), ValueType::Ref, tv.value_type())) }, None => { - bail!(ErrorKind::UnboundVariable((*v.0).clone())); + bail!(AlgebrizerError::UnboundVariable((*v.0).clone())) } } }, @@ -127,10 +129,10 @@ impl ConjoiningClauses { // An unknown ident, or an entity that isn't present in the store, or isn't a fulltext // attribute, is likely enough to be a coding error that we choose to bail instead of // marking the pattern as known-empty. - let a = a.ok_or(ErrorKind::InvalidArgument(where_fn.operator.clone(), "attribute", 1))?; + let a = a.ok_or(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "attribute", 1))?; let attribute = schema.attribute_for_entid(a) .cloned() - .ok_or(ErrorKind::InvalidArgument(where_fn.operator.clone(), + .ok_or(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "attribute", 1))?; if !attribute.fulltext { @@ -169,18 +171,18 @@ impl ConjoiningClauses { FnArg::Variable(in_var) => { match self.bound_value(&in_var) { Some(t @ TypedValue::String(_)) => Either::Left(t), - Some(_) => bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2)), + Some(_) => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2)), None => { // Regardless of whether we'll be providing a string later, or the value // comes from a column, it must be a string. if self.known_type(&in_var) != Some(ValueType::String) { - bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2)); + bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2)) } if self.input_variables.contains(&in_var) { // Sorry, we haven't implemented late binding. // TODO: implement this. - bail!(ErrorKind::UnboundVariable((*in_var.0).clone())); + bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone())) } else { // It must be bound earlier in the query. We already established that // it must be a string column. @@ -189,13 +191,13 @@ impl ConjoiningClauses { .and_then(|bindings| bindings.get(0).cloned()) { Either::Right(binding) } else { - bail!(ErrorKind::UnboundVariable((*in_var.0).clone())); + bail!(AlgebrizerError::UnboundVariable((*in_var.0).clone())) } } }, } }, - _ => bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "string", 2)), + _ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "string", 2)), }; let qv = match search { @@ -244,7 +246,7 @@ impl ConjoiningClauses { // We do not allow the score to be bound. if self.value_bindings.contains_key(var) || self.input_variables.contains(var) { - bail!(ErrorKind::InvalidBinding(var.name(), BindingError::UnexpectedBinding)); + bail!(InvalidBinding::new(var.name(), BindingError::UnexpectedBinding)); } // We bind the value ourselves. This handily takes care of substituting into existing uses. diff --git a/query-algebrizer/src/clauses/ground.rs b/query-algebrizer/src/clauses/ground.rs index cedb2e2a..18f68d64 100644 --- a/query-algebrizer/src/clauses/ground.rs +++ b/query-algebrizer/src/clauses/ground.rs @@ -31,8 +31,9 @@ use clauses::{ use clauses::convert::ValueConversion; use errors::{ + AlgebrizerError, BindingError, - ErrorKind, + InvalidBinding, Result, }; @@ -117,19 +118,19 @@ impl ConjoiningClauses { pub(crate) fn apply_ground(&mut self, known: Known, where_fn: WhereFn) -> Result<()> { if where_fn.args.len() != 1 { - bail!(ErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 1)); + bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 1)); } let mut args = where_fn.args.into_iter(); if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } let schema = known.schema; @@ -145,7 +146,7 @@ impl ConjoiningClauses { // Just the same, but we bind more than one column at a time. if children.len() != places.len() { // Number of arguments don't match the number of values. TODO: better error message. - bail!(ErrorKind::GroundBindingsMismatch); + bail!(AlgebrizerError::GroundBindingsMismatch) } for (place, arg) in places.into_iter().zip(children.into_iter()) { self.apply_ground_place(schema, place, arg)? // TODO: short-circuit on impossible. @@ -159,7 +160,7 @@ impl ConjoiningClauses { // are all in a single structure. That makes it substantially simpler! (Binding::BindColl(var), FnArg::Vector(children)) => { if children.is_empty() { - bail!(ErrorKind::InvalidGroundConstant); + bail!(AlgebrizerError::InvalidGroundConstant) } // Turn a collection of arguments into a Vec of `TypedValue`s of the same type. @@ -177,7 +178,7 @@ impl ConjoiningClauses { if accumulated_types.insert(tv.value_type()) && !accumulated_types.is_unit() { // Values not all of the same type. - Some(Err(ErrorKind::InvalidGroundConstant.into())) + Some(Err(AlgebrizerError::InvalidGroundConstant.into())) } else { Some(Ok(tv)) } @@ -208,7 +209,7 @@ impl ConjoiningClauses { (Binding::BindRel(places), FnArg::Vector(rows)) => { if rows.is_empty() { - bail!(ErrorKind::InvalidGroundConstant); + bail!(AlgebrizerError::InvalidGroundConstant) } // Grab the known types to which these args must conform, and track @@ -229,7 +230,7 @@ impl ConjoiningClauses { if expected_width == 0 { // They can't all be placeholders. - bail!(ErrorKind::InvalidGroundConstant); + bail!(AlgebrizerError::InvalidGroundConstant) } // Accumulate values into `matrix` and types into `a_t_f_c`. @@ -245,7 +246,7 @@ impl ConjoiningClauses { FnArg::Vector(cols) => { // Make sure that every row is the same length. if cols.len() != full_width { - bail!(ErrorKind::InvalidGroundConstant); + bail!(AlgebrizerError::InvalidGroundConstant) } // TODO: don't accumulate twice. @@ -280,13 +281,13 @@ impl ConjoiningClauses { let inserted = acc.insert(val.value_type()); if inserted && !acc.is_unit() { // Heterogeneous types. - bail!(ErrorKind::InvalidGroundConstant); + bail!(AlgebrizerError::InvalidGroundConstant) } matrix.push(val); } }, - _ => bail!(ErrorKind::InvalidGroundConstant), + _ => bail!(AlgebrizerError::InvalidGroundConstant), } } @@ -312,7 +313,7 @@ impl ConjoiningClauses { self.collect_named_bindings(schema, names, types, matrix); Ok(()) }, - (_, _) => bail!(ErrorKind::InvalidGroundConstant), + (_, _) => bail!(AlgebrizerError::InvalidGroundConstant), } } } diff --git a/query-algebrizer/src/clauses/inputs.rs b/query-algebrizer/src/clauses/inputs.rs index 6a4c73af..35c4371d 100644 --- a/query-algebrizer/src/clauses/inputs.rs +++ b/query-algebrizer/src/clauses/inputs.rs @@ -20,7 +20,7 @@ use mentat_query::{ }; use errors::{ - ErrorKind, + AlgebrizerError, Result, }; @@ -72,7 +72,7 @@ impl QueryInputs { let old = types.insert(var.clone(), t); if let Some(old) = old { if old != t { - bail!(ErrorKind::InputTypeDisagreement(var.name(), old, t)); + bail!(AlgebrizerError::InputTypeDisagreement(var.name(), old, t)); } } } diff --git a/query-algebrizer/src/clauses/mod.rs b/query-algebrizer/src/clauses/mod.rs index 29c840bf..2fca66b0 100644 --- a/query-algebrizer/src/clauses/mod.rs +++ b/query-algebrizer/src/clauses/mod.rs @@ -53,8 +53,7 @@ use mentat_query::{ }; use errors::{ - Error, - ErrorKind, + AlgebrizerError, Result, }; @@ -1014,7 +1013,7 @@ impl ConjoiningClauses { let qa = self.extracted_types .get(&var) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name())))?; + .ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()))?; self.wheres.add_intersection(ColumnConstraint::HasTypes { value: qa.0.clone(), value_types: types, diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 299c62ec..05b88330 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -17,7 +17,7 @@ use mentat_query::{ use clauses::ConjoiningClauses; use errors::{ - ErrorKind, + AlgebrizerError, Result, }; @@ -45,7 +45,7 @@ impl ConjoiningClauses { let col = self.column_bindings.get(&v).unwrap()[0].clone(); template.column_bindings.insert(v.clone(), vec![col]); } else { - bail!(ErrorKind::UnboundVariable(v.name())); + bail!(AlgebrizerError::UnboundVariable(v.name())); } } @@ -111,8 +111,7 @@ mod testing { }; use errors::{ - Error, - ErrorKind, + AlgebrizerError, }; use types::{ @@ -553,10 +552,9 @@ mod testing { :in ?y :where (not [?x :foo/knows ?y])]"#; let parsed = parse_find_string(query).expect("parse failed"); - let err = algebrize(known, parsed).err(); - assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::UnboundVariable(var), _) => { assert_eq!(var, PlainSymbol("?x".to_string())); }, + let err = algebrize(known, parsed).expect_err("algebrization should have failed"); + match err.downcast().expect("expected AlgebrizerError") { + AlgebrizerError::UnboundVariable(var) => { assert_eq!(var, PlainSymbol("?x".to_string())); }, x => panic!("expected Unbound Variable error, got {:?}", x), } } diff --git a/query-algebrizer/src/clauses/predicate.rs b/query-algebrizer/src/clauses/predicate.rs index add9eb8c..e58ecf7f 100644 --- a/query-algebrizer/src/clauses/predicate.rs +++ b/query-algebrizer/src/clauses/predicate.rs @@ -26,8 +26,8 @@ use clauses::ConjoiningClauses; use clauses::convert::ValueTypes; use errors::{ + AlgebrizerError, Result, - ErrorKind, }; use types::{ @@ -53,7 +53,7 @@ impl ConjoiningClauses { if let Some(op) = Inequality::from_datalog_operator(predicate.operator.0.as_str()) { self.apply_inequality(known, op, predicate) } else { - bail!(ErrorKind::UnknownFunction(predicate.operator.clone())) + bail!(AlgebrizerError::UnknownFunction(predicate.operator.clone())) } } @@ -69,7 +69,7 @@ impl ConjoiningClauses { pub(crate) fn apply_type_anno(&mut self, anno: &TypeAnnotation) -> Result<()> { match ValueType::from_keyword(&anno.value_type) { Some(value_type) => self.add_type_requirement(anno.variable.clone(), ValueTypeSet::of_one(value_type)), - None => bail!(ErrorKind::InvalidArgumentType(PlainSymbol::plain("type"), ValueTypeSet::any(), 2)), + None => bail!(AlgebrizerError::InvalidArgumentType(PlainSymbol::plain("type"), ValueTypeSet::any(), 2)), } Ok(()) } @@ -80,7 +80,7 @@ impl ConjoiningClauses { /// - Accumulates an `Inequality` constraint into the `wheres` list. pub(crate) fn apply_inequality(&mut self, known: Known, comparison: Inequality, predicate: Predicate) -> Result<()> { if predicate.args.len() != 2 { - bail!(ErrorKind::InvalidNumberOfArguments(predicate.operator.clone(), predicate.args.len(), 2)); + bail!(AlgebrizerError::InvalidNumberOfArguments(predicate.operator.clone(), predicate.args.len(), 2)); } // Go from arguments -- parser output -- to columns or values. @@ -97,13 +97,13 @@ impl ConjoiningClauses { let mut left_types = self.potential_types(known.schema, &left)? .intersection(&supported_types); if left_types.is_empty() { - bail!(ErrorKind::InvalidArgumentType(predicate.operator.clone(), supported_types, 0)); + bail!(AlgebrizerError::InvalidArgumentType(predicate.operator.clone(), supported_types, 0)); } let mut right_types = self.potential_types(known.schema, &right)? .intersection(&supported_types); if right_types.is_empty() { - bail!(ErrorKind::InvalidArgumentType(predicate.operator.clone(), supported_types, 1)); + bail!(AlgebrizerError::InvalidArgumentType(predicate.operator.clone(), supported_types, 1)); } // We would like to allow longs to compare to doubles. @@ -150,7 +150,7 @@ impl ConjoiningClauses { left_v = self.resolve_ref_argument(known.schema, &predicate.operator, 0, left)?; right_v = self.resolve_ref_argument(known.schema, &predicate.operator, 1, right)?; } else { - bail!(ErrorKind::InvalidArgumentType(predicate.operator.clone(), supported_types, 0)); + bail!(AlgebrizerError::InvalidArgumentType(predicate.operator.clone(), supported_types, 0)); } // These arguments must be variables or instant/numeric constants. diff --git a/query-algebrizer/src/clauses/resolve.rs b/query-algebrizer/src/clauses/resolve.rs index 8384a01d..87a18cf3 100644 --- a/query-algebrizer/src/clauses/resolve.rs +++ b/query-algebrizer/src/clauses/resolve.rs @@ -24,9 +24,8 @@ use mentat_query::{ use clauses::ConjoiningClauses; use errors::{ + AlgebrizerError, Result, - Error, - ErrorKind, }; use types::{ @@ -50,14 +49,14 @@ impl ConjoiningClauses { if v.value_type().is_numeric() { Ok(QueryValue::TypedValue(v)) } else { - bail!(ErrorKind::InputTypeDisagreement(var.name().clone(), ValueType::Long, v.value_type())); + bail!(AlgebrizerError::InputTypeDisagreement(var.name().clone(), ValueType::Long, v.value_type())) } } else { self.constrain_var_to_numeric(var.clone()); self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + .ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()).into()) } }, // Can't be an entid. @@ -71,7 +70,7 @@ impl ConjoiningClauses { Constant(NonIntegerConstant::BigInteger(_)) | Vector(_) => { self.mark_known_empty(EmptyBecause::NonNumericArgument); - bail!(ErrorKind::InvalidArgument(function.clone(), "numeric", position)); + bail!(AlgebrizerError::InvalidArgument(function.clone(), "numeric", position)) }, Constant(NonIntegerConstant::Float(f)) => Ok(QueryValue::TypedValue(TypedValue::Double(f))), } @@ -84,13 +83,13 @@ impl ConjoiningClauses { FnArg::Variable(var) => { match self.bound_value(&var) { Some(TypedValue::Instant(v)) => Ok(QueryValue::TypedValue(TypedValue::Instant(v))), - Some(v) => bail!(ErrorKind::InputTypeDisagreement(var.name().clone(), ValueType::Instant, v.value_type())), + Some(v) => bail!(AlgebrizerError::InputTypeDisagreement(var.name().clone(), ValueType::Instant, v.value_type())), None => { self.constrain_var_to_type(var.clone(), ValueType::Instant); self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + .ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()).into()) }, } }, @@ -109,7 +108,7 @@ impl ConjoiningClauses { Constant(NonIntegerConstant::BigInteger(_)) | Vector(_) => { self.mark_known_empty(EmptyBecause::NonInstantArgument); - bail!(ErrorKind::InvalidArgumentType(function.clone(), ValueType::Instant.into(), position)); + bail!(AlgebrizerError::InvalidArgumentType(function.clone(), ValueType::Instant.into(), position)) }, } } @@ -128,14 +127,14 @@ impl ConjoiningClauses { self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + .ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()).into()) } }, EntidOrInteger(i) => Ok(QueryValue::TypedValue(TypedValue::Ref(i))), IdentOrKeyword(i) => { schema.get_entid(&i) .map(|known_entid| QueryValue::Entid(known_entid.into())) - .ok_or_else(|| Error::from_kind(ErrorKind::UnrecognizedIdent(i.to_string()))) + .ok_or_else(|| AlgebrizerError::UnrecognizedIdent(i.to_string()).into()) }, Constant(NonIntegerConstant::Boolean(_)) | Constant(NonIntegerConstant::Float(_)) | @@ -146,7 +145,7 @@ impl ConjoiningClauses { SrcVar(_) | Vector(_) => { self.mark_known_empty(EmptyBecause::NonEntityArgument); - bail!(ErrorKind::InvalidArgumentType(function.clone(), ValueType::Ref.into(), position)); + bail!(AlgebrizerError::InvalidArgumentType(function.clone(), ValueType::Ref.into(), position)) }, } @@ -173,7 +172,7 @@ impl ConjoiningClauses { self.column_bindings .get(&var) .and_then(|cols| cols.first().map(|col| QueryValue::Column(col.clone()))) - .ok_or_else(|| Error::from_kind(ErrorKind::UnboundVariable(var.name()))) + .ok_or_else(|| AlgebrizerError::UnboundVariable(var.name()).into()) }, } }, diff --git a/query-algebrizer/src/clauses/tx_log_api.rs b/query-algebrizer/src/clauses/tx_log_api.rs index 2fc7dce8..d7dbb686 100644 --- a/query-algebrizer/src/clauses/tx_log_api.rs +++ b/query-algebrizer/src/clauses/tx_log_api.rs @@ -25,8 +25,9 @@ use clauses::{ }; use errors::{ + AlgebrizerError, BindingError, - ErrorKind, + InvalidBinding, Result, }; @@ -60,17 +61,17 @@ impl ConjoiningClauses { // transactions that impact one of the given attributes. pub(crate) fn apply_tx_ids(&mut self, known: Known, where_fn: WhereFn) -> Result<()> { if where_fn.args.len() != 3 { - bail!(ErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3)); + bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 3)); } if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have exactly one binding. Destructure it now. @@ -78,7 +79,7 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count != 1 { - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::InvalidNumberOfBindings { number: bindings_count, expected: 1, @@ -92,7 +93,7 @@ impl ConjoiningClauses { Binding::BindColl(v) => v, Binding::BindScalar(_) | Binding::BindTuple(_) => { - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRelOrBindColl)) + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRelOrBindColl)) }, }; @@ -101,7 +102,7 @@ impl ConjoiningClauses { // TODO: process source variables. match args.next().unwrap() { FnArg::SrcVar(SrcVar::DefaultSrc) => {}, - _ => bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), + _ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), } let tx1 = self.resolve_tx_argument(&known.schema, &where_fn.operator, 1, args.next().unwrap())?; @@ -138,17 +139,17 @@ impl ConjoiningClauses { pub(crate) fn apply_tx_data(&mut self, known: Known, where_fn: WhereFn) -> Result<()> { if where_fn.args.len() != 2 { - bail!(ErrorKind::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 2)); + bail!(AlgebrizerError::InvalidNumberOfArguments(where_fn.operator.clone(), where_fn.args.len(), 2)); } if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have at most five bindings. Destructure them now. @@ -156,7 +157,7 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count < 1 || bindings_count > 5 { - bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), + bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::InvalidNumberOfBindings { number: bindings.len(), expected: 5, @@ -166,7 +167,7 @@ impl ConjoiningClauses { }, Binding::BindScalar(_) | Binding::BindTuple(_) | - Binding::BindColl(_) => bail!(ErrorKind::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)), + Binding::BindColl(_) => bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRel)), }; let mut bindings = bindings.into_iter(); let b_e = bindings.next().unwrap_or(VariableOrPlaceholder::Placeholder); @@ -180,7 +181,7 @@ impl ConjoiningClauses { // TODO: process source variables. match args.next().unwrap() { FnArg::SrcVar(SrcVar::DefaultSrc) => {}, - _ => bail!(ErrorKind::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), + _ => bail!(AlgebrizerError::InvalidArgument(where_fn.operator.clone(), "source variable", 0)), } let tx = self.resolve_tx_argument(&known.schema, &where_fn.operator, 1, args.next().unwrap())?; diff --git a/query-algebrizer/src/clauses/where_fn.rs b/query-algebrizer/src/clauses/where_fn.rs index 7cf81188..845a6a64 100644 --- a/query-algebrizer/src/clauses/where_fn.rs +++ b/query-algebrizer/src/clauses/where_fn.rs @@ -17,7 +17,7 @@ use clauses::{ }; use errors::{ - ErrorKind, + AlgebrizerError, Result, }; @@ -39,7 +39,7 @@ impl ConjoiningClauses { "ground" => self.apply_ground(known, where_fn), "tx-data" => self.apply_tx_data(known, where_fn), "tx-ids" => self.apply_tx_ids(known, where_fn), - _ => bail!(ErrorKind::UnknownFunction(where_fn.operator.clone())), + _ => bail!(AlgebrizerError::UnknownFunction(where_fn.operator.clone())), } } } diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index 6e2e1154..bba74686 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -10,8 +10,18 @@ extern crate mentat_query; +use std; // To refer to std::result::Result. +use std::fmt; +use std::fmt::Display; + +use failure::{ + Backtrace, + Context, + Error, + Fail, +}; + use mentat_core::{ - EdnParseError, ValueType, ValueTypeSet, }; @@ -20,7 +30,53 @@ use self::mentat_query::{ PlainSymbol, }; -#[derive(Clone, Debug, Eq, PartialEq)] +pub type Result = std::result::Result; + +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + +#[derive(Debug)] +pub struct InvalidBinding { + pub function: PlainSymbol, + pub inner: Context +} + +impl InvalidBinding { + pub fn new(function: PlainSymbol, inner: BindingError) -> InvalidBinding { + InvalidBinding { + function: function, + inner: Context::new(inner) + } + } +} + +impl Fail for InvalidBinding { + fn cause(&self) -> Option<&Fail> { + self.inner.cause() + } + + fn backtrace(&self) -> Option<&Backtrace> { + self.inner.backtrace() + } +} + +impl Display for InvalidBinding { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "invalid binding for {}: {:?}", self.function, self.inner) + } +} + +impl Display for BindingError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "BindingError: {:?}", self) + } +} + +#[derive(Clone, Debug, Eq, PartialEq, Fail)] pub enum BindingError { NoBoundVariable, UnexpectedBinding, @@ -40,98 +96,52 @@ pub enum BindingError { InvalidNumberOfBindings { number: usize, expected: usize }, } -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } +#[derive(Debug, Fail)] +pub enum AlgebrizerError { + #[fail(display = "{} var {} is duplicated", _0, _1)] + DuplicateVariableError(PlainSymbol, &'static str), - foreign_links { - EdnParseError(EdnParseError); - } + #[fail(display = "unexpected FnArg")] + UnsupportedArgument, - errors { - UnsupportedArgument { - description("unexpected FnArg") - display("unexpected FnArg") - } + #[fail(display = "value of type {} provided for var {}, expected {}", _0, _1, _2)] + InputTypeDisagreement(PlainSymbol, ValueType, ValueType), - InputTypeDisagreement(var: PlainSymbol, declared: ValueType, provided: ValueType) { - description("input type disagreement") - display("value of type {} provided for var {}, expected {}", provided, var, declared) - } + #[fail(display = "invalid number of arguments to {}: expected {}, got {}.", _0, _1, _2)] + InvalidNumberOfArguments(PlainSymbol, usize, usize), - UnrecognizedIdent(ident: String) { - description("no entid found for ident") - display("no entid found for ident: {}", ident) - } + #[fail(display = "invalid argument to {}: expected {} in position {}.", _0, _1, _2)] + InvalidArgument(PlainSymbol, &'static str, usize), - UnknownFunction(name: PlainSymbol) { - description("no such function") - display("no function named {}", name) - } + #[fail(display = "invalid argument to {}: expected one of {:?} in position {}.", _0, _1, _2)] + InvalidArgumentType(PlainSymbol, ValueTypeSet, usize), - InvalidNumberOfArguments(function: PlainSymbol, number: usize, expected: usize) { - description("invalid number of arguments") - display("invalid number of arguments to {}: expected {}, got {}.", function, expected, number) - } + // TODO: flesh this out. + #[fail(display = "invalid expression in ground constant")] + InvalidGroundConstant, - UnboundVariable(name: PlainSymbol) { - description("unbound variable in order clause or function call") - display("unbound variable: {}", name) - } + #[fail(display = "invalid limit {} of type {}: expected natural number.", _0, _1)] + InvalidLimit(String, ValueType), - InvalidBinding(function: PlainSymbol, binding_error: BindingError) { - description("invalid binding") - display("invalid binding for {}: {:?}.", function, binding_error) - } + #[fail(display = "mismatched bindings in ground")] + GroundBindingsMismatch, - GroundBindingsMismatch { - description("mismatched bindings in ground") - display("mismatched bindings in ground") - } + #[fail(display = "no entid found for ident: {}", _0)] + UnrecognizedIdent(String), - InvalidGroundConstant { - // TODO: flesh this out. - description("invalid expression in ground constant") - display("invalid expression in ground constant") - } + #[fail(display = "no function named {}", _0)] + UnknownFunction(PlainSymbol), - InvalidArgument(function: PlainSymbol, expected: &'static str, position: usize) { - description("invalid argument") - display("invalid argument to {}: expected {} in position {}.", function, expected, position) - } + #[fail(display = ":limit var {} not present in :in", _0)] + UnknownLimitVar(PlainSymbol), - InvalidArgumentType(function: PlainSymbol, expected_types: ValueTypeSet, position: usize) { - description("invalid argument") - display("invalid argument to {}: expected one of {:?} in position {}.", function, expected_types, position) - } + #[fail(display = "unbound variable {} in order clause or function call", _0)] + UnboundVariable(PlainSymbol), - InvalidLimit(val: String, kind: ValueType) { - description("invalid limit") - display("invalid limit {} of type {}: expected natural number.", val, kind) - } + // TODO: flesh out. + #[fail(display = "non-matching variables in 'or' clause")] + NonMatchingVariablesInOrClause, - NonMatchingVariablesInOrClause { - // TODO: flesh out. - description("non-matching variables in 'or' clause") - display("non-matching variables in 'or' clause") - } - - NonMatchingVariablesInNotClause { - // TODO: flesh out. - description("non-matching variables in 'not' clause") - display("non-matching variables in 'not' clause") - } - - DuplicateVariableError(name: PlainSymbol, clause: &'static str) { - description("duplicate variables") - display("{} var {} is duplicated", clause, name) - } - - UnknownLimitVar(name: PlainSymbol) { - description(":limit var not present in :in") - display(":limit var {} not present in :in", name) - } - } + #[fail(display = "non-matching variables in 'not' clause")] + NonMatchingVariablesInNotClause, } - diff --git a/query-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 90704b7a..0a137110 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -8,10 +8,9 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -#![recursion_limit="128"] +extern crate failure; -#[macro_use] -extern crate error_chain; +#[macro_use] extern crate failure_derive; extern crate mentat_core; extern crate mentat_query; @@ -20,6 +19,7 @@ use std::collections::BTreeSet; use std::ops::Sub; use std::rc::Rc; +#[macro_use] mod errors; mod types; mod validate; @@ -48,10 +48,10 @@ use mentat_query::{ }; pub use errors::{ + AlgebrizerError, BindingError, - Error, - ErrorKind, Result, + InvalidBinding, }; pub use clauses::{ @@ -216,7 +216,7 @@ fn validate_and_simplify_order(cc: &ConjoiningClauses, order: Option> // Fail if the var isn't bound by the query. if !cc.column_bindings.contains_key(&var) { - bail!(ErrorKind::UnboundVariable(var.name())); + bail!(AlgebrizerError::UnboundVariable(var.name())) } // Otherwise, determine if we also need to order by type… @@ -242,14 +242,14 @@ fn simplify_limit(mut query: AlgebraicQuery) -> Result { Some(TypedValue::Long(n)) => { if n <= 0 { // User-specified limits should always be natural numbers (> 0). - bail!(ErrorKind::InvalidLimit(n.to_string(), ValueType::Long)); + bail!(AlgebrizerError::InvalidLimit(n.to_string(), ValueType::Long)) } else { Some(Limit::Fixed(n as u64)) } }, Some(val) => { // Same. - bail!(ErrorKind::InvalidLimit(format!("{:?}", val), val.value_type())); + bail!(AlgebrizerError::InvalidLimit(format!("{:?}", val), val.value_type())) }, None => { // We know that the limit variable is mentioned in `:in`. @@ -357,7 +357,7 @@ impl FindQuery { for var in parsed.in_vars.into_iter() { if !set.insert(var.clone()) { - bail!(ErrorKind::DuplicateVariableError(var.name(), ":in")); + bail!(AlgebrizerError::DuplicateVariableError(var.name(), ":in")); } } @@ -369,7 +369,7 @@ impl FindQuery { for var in parsed.with.into_iter() { if !set.insert(var.clone()) { - bail!(ErrorKind::DuplicateVariableError(var.name(), ":with")); + bail!(AlgebrizerError::DuplicateVariableError(var.name(), ":with")); } } @@ -379,7 +379,7 @@ impl FindQuery { // Make sure that if we have `:limit ?x`, `?x` appears in `:in`. if let Limit::Variable(ref v) = parsed.limit { if !in_vars.contains(v) { - bail!(ErrorKind::UnknownLimitVar(v.name())); + bail!(AlgebrizerError::UnknownLimitVar(v.name())); } } diff --git a/query-algebrizer/src/validate.rs b/query-algebrizer/src/validate.rs index 5bac7fde..1dba97fb 100644 --- a/query-algebrizer/src/validate.rs +++ b/query-algebrizer/src/validate.rs @@ -19,7 +19,7 @@ use mentat_query::{ }; use errors::{ - ErrorKind, + AlgebrizerError, Result, }; @@ -56,7 +56,7 @@ pub(crate) fn validate_or_join(or_join: &OrJoin) -> Result<()> { let template = clauses.next().unwrap().collect_mentioned_variables(); for clause in clauses { if template != clause.collect_mentioned_variables() { - bail!(ErrorKind::NonMatchingVariablesInOrClause); + bail!(AlgebrizerError::NonMatchingVariablesInOrClause) } } Ok(()) @@ -67,7 +67,7 @@ pub(crate) fn validate_or_join(or_join: &OrJoin) -> Result<()> { let var_set: BTreeSet = vars.iter().cloned().collect(); for clause in &or_join.clauses { if !var_set.is_subset(&clause.collect_mentioned_variables()) { - bail!(ErrorKind::NonMatchingVariablesInOrClause); + bail!(AlgebrizerError::NonMatchingVariablesInOrClause) } } Ok(()) @@ -85,7 +85,7 @@ pub(crate) fn validate_not_join(not_join: &NotJoin) -> Result<()> { // The joined vars must each appear somewhere in the clause's mentioned variables. let var_set: BTreeSet = vars.iter().cloned().collect(); if !var_set.is_subset(¬_join.collect_mentioned_variables()) { - bail!(ErrorKind::NonMatchingVariablesInNotClause); + bail!(AlgebrizerError::NonMatchingVariablesInNotClause) } Ok(()) }, diff --git a/query-algebrizer/tests/ground.rs b/query-algebrizer/tests/ground.rs index b7e8341f..d3e74faf 100644 --- a/query-algebrizer/tests/ground.rs +++ b/query-algebrizer/tests/ground.rs @@ -30,10 +30,10 @@ use mentat_query::{ }; use mentat_query_algebrizer::{ + AlgebrizerError, BindingError, ComputedTable, - Error, - ErrorKind, + InvalidBinding, Known, QueryInputs, }; @@ -256,8 +256,8 @@ fn test_ground_coll_heterogeneous_types() { let schema = prepopulated_schema(); let known = Known::for_schema(&schema); let e = bails(known, &q); - match e { - Error(ErrorKind::InvalidGroundConstant, _) => { + match e.downcast().expect("proper error") { + AlgebrizerError::InvalidGroundConstant => { }, _ => { panic!(); @@ -271,8 +271,8 @@ fn test_ground_rel_heterogeneous_types() { let schema = prepopulated_schema(); let known = Known::for_schema(&schema); let e = bails(known, &q); - match e { - Error(ErrorKind::InvalidGroundConstant, _) => { + match e.downcast().expect("proper error") { + AlgebrizerError::InvalidGroundConstant => { }, _ => { panic!(); @@ -285,11 +285,10 @@ fn test_ground_tuple_duplicate_vars() { let q = r#"[:find ?x :where [?x :foo/age ?v] [(ground [8 10]) [?x ?x]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q); - match e { - Error(ErrorKind::InvalidBinding(v, e), _) => { - assert_eq!(v, PlainSymbol::plain("ground")); - assert_eq!(e, BindingError::RepeatedBoundVariable); + let e: InvalidBinding = bails(known, &q).downcast().expect("proper error"); + assert_eq!(e.function, PlainSymbol::plain("ground")); + match e.inner.get_context() { + &BindingError::RepeatedBoundVariable => { }, _ => { panic!(); @@ -302,11 +301,10 @@ fn test_ground_rel_duplicate_vars() { let q = r#"[:find ?x :where [?x :foo/age ?v] [(ground [[8 10]]) [[?x ?x]]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q); - match e { - Error(ErrorKind::InvalidBinding(v, e), _) => { - assert_eq!(v, PlainSymbol::plain("ground")); - assert_eq!(e, BindingError::RepeatedBoundVariable); + let e: InvalidBinding = bails(known, &q).downcast().expect("expected InvalidBinding"); + assert_eq!(e.function, PlainSymbol::plain("ground")); + match e.inner.get_context() { + &BindingError::RepeatedBoundVariable => { }, _ => { panic!(); @@ -319,9 +317,9 @@ fn test_ground_nonexistent_variable_invalid() { let q = r#"[:find ?x ?e :where [?e _ ?x] (not [(ground 17) ?v])]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q); + let e = bails(known, &q).downcast().expect("proper error"); match e { - Error(ErrorKind::UnboundVariable(PlainSymbol(v)), _) => { + AlgebrizerError::UnboundVariable(PlainSymbol(v)) => { assert_eq!(v, "?v".to_string()); }, _ => { @@ -343,9 +341,9 @@ fn test_unbound_input_variable_invalid() { let i = QueryInputs::new(types, BTreeMap::default()).expect("valid QueryInputs"); - let e = bails_with_inputs(known, &q, i); + let e = bails_with_inputs(known, &q, i).downcast().expect("proper error"); match e { - Error(ErrorKind::UnboundVariable(v), _) => { + AlgebrizerError::UnboundVariable(v) => { assert_eq!(v.0, "?x"); }, _ => { diff --git a/query-algebrizer/tests/predicate.rs b/query-algebrizer/tests/predicate.rs index 6aa0d929..7a2e4591 100644 --- a/query-algebrizer/tests/predicate.rs +++ b/query-algebrizer/tests/predicate.rs @@ -31,8 +31,8 @@ use mentat_query::{ }; use mentat_query_algebrizer::{ + AlgebrizerError, EmptyBecause, - ErrorKind, Known, QueryInputs, }; @@ -78,8 +78,8 @@ fn test_instant_predicates_require_instants() { :where [?e :foo/date ?t] [(> ?t "2017-06-16T00:56:41.257Z")]]"#; - match bails(known, query).0 { - ErrorKind::InvalidArgumentType(op, why, idx) => { + match bails(known, query).downcast().expect("proper cause") { + AlgebrizerError::InvalidArgumentType(op, why, idx) => { assert_eq!(op, PlainSymbol::plain(">")); assert_eq!(why, ValueTypeSet::of_numeric_and_instant_types()); assert_eq!(idx, 1); @@ -91,8 +91,8 @@ fn test_instant_predicates_require_instants() { :where [?e :foo/date ?t] [(> "2017-06-16T00:56:41.257Z", ?t)]]"#; - match bails(known, query).0 { - ErrorKind::InvalidArgumentType(op, why, idx) => { + match bails(known, query).downcast().expect("proper cause") { + AlgebrizerError::InvalidArgumentType(op, why, idx) => { assert_eq!(op, PlainSymbol::plain(">")); assert_eq!(why, ValueTypeSet::of_numeric_and_instant_types()); assert_eq!(idx, 0); // We get this right. diff --git a/query-algebrizer/tests/utils/mod.rs b/query-algebrizer/tests/utils/mod.rs index 9ca8185d..88bae48c 100644 --- a/query-algebrizer/tests/utils/mod.rs +++ b/query-algebrizer/tests/utils/mod.rs @@ -13,6 +13,9 @@ // this module will get warnings otherwise). #![allow(dead_code)] +extern crate failure; +use self::failure::Error; + use mentat_core::{ Attribute, Entid, @@ -26,7 +29,6 @@ use mentat_query::{ use mentat_query_algebrizer::{ ConjoiningClauses, - Error, Known, QueryInputs, algebrize, From 061967f268994fd467850b82ccf5f008d38e12f9 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 16:10:21 -0400 Subject: [PATCH 03/11] Convert query-pull/ to failure. --- query-pull/Cargo.toml | 3 ++- query-pull/src/errors.rs | 29 ++++++++++++----------------- query-pull/src/lib.rs | 10 ++++++---- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/query-pull/Cargo.toml b/query-pull/Cargo.toml index cc5e8b68..fe7b44c7 100644 --- a/query-pull/Cargo.toml +++ b/query-pull/Cargo.toml @@ -4,7 +4,8 @@ version = "0.0.1" workspace = ".." [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" [dependencies.rusqlite] version = "0.13" diff --git a/query-pull/src/errors.rs b/query-pull/src/errors.rs index 94c38426..628a1170 100644 --- a/query-pull/src/errors.rs +++ b/query-pull/src/errors.rs @@ -8,28 +8,23 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. +use std; // To refer to std::result::Result. + use mentat_core::{ Entid, }; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } +use failure::{ + Error, +}; - errors { - UnnamedAttribute(id: Entid) { - description("unnamed attribute") - display("attribute {:?} has no name", id) - } +pub type Result = std::result::Result; - RepeatedDbId { - description(":db/id repeated") - display(":db/id repeated") - } - } +#[derive(Debug, Fail)] +pub enum PullError { + #[fail(display = "attribute {:?} has no name", _0)] + UnnamedAttribute(Entid), - links { - DbError(::mentat_db::Error, ::mentat_db::ErrorKind); - } + #[fail(display = ":db/id repeated")] + RepeatedDbId, } diff --git a/query-pull/src/lib.rs b/query-pull/src/lib.rs index 1b2d7804..22b48de6 100644 --- a/query-pull/src/lib.rs +++ b/query-pull/src/lib.rs @@ -57,8 +57,10 @@ /// [*])) ///! ``` +extern crate failure; + #[macro_use] -extern crate error_chain; +extern crate failure_derive; extern crate rusqlite; @@ -101,7 +103,7 @@ use mentat_query::{ pub mod errors; use errors::{ - ErrorKind, + PullError, Result, }; @@ -163,7 +165,7 @@ impl Puller { // In the unlikely event that we have an attribute with no name, we bail. schema.get_ident(*i) .map(|ident| ValueRc::new(ident.clone())) - .ok_or_else(|| ErrorKind::UnnamedAttribute(*i)) + .ok_or_else(|| PullError::UnnamedAttribute(*i)) }; let mut names: BTreeMap> = Default::default(); @@ -192,7 +194,7 @@ impl Puller { &PullConcreteAttribute::Ident(ref i) if i.as_ref() == db_id.as_ref() => { // We only allow :db/id once. if db_id_alias.is_some() { - bail!(ErrorKind::RepeatedDbId); + Err(PullError::RepeatedDbId)? } db_id_alias = Some(alias.unwrap_or_else(|| db_id.to_value_rc())); }, From c075434f840a28ebe01d6b5c26c690b005b49728 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 16:53:31 -0400 Subject: [PATCH 04/11] Convert query_projector/ to failure. --- query-projector/Cargo.toml | 3 +- query-projector/src/aggregates.rs | 12 ++-- query-projector/src/errors.rs | 96 ++++++++++++----------------- query-projector/src/lib.rs | 34 +++++----- query-projector/src/project.rs | 22 +++---- query-projector/src/pull.rs | 4 +- query-projector/tests/aggregates.rs | 7 +-- 7 files changed, 83 insertions(+), 95 deletions(-) diff --git a/query-projector/Cargo.toml b/query-projector/Cargo.toml index 1bdcc4c1..be414ef0 100644 --- a/query-projector/Cargo.toml +++ b/query-projector/Cargo.toml @@ -4,7 +4,8 @@ version = "0.0.1" workspace = ".." [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" indexmap = "1" [dependencies.rusqlite] diff --git a/query-projector/src/aggregates.rs b/query-projector/src/aggregates.rs index 99808fa7..8fde9b2a 100644 --- a/query-projector/src/aggregates.rs +++ b/query-projector/src/aggregates.rs @@ -33,7 +33,7 @@ use mentat_query_sql::{ }; use errors::{ - ErrorKind, + ProjectorError, Result, }; @@ -79,7 +79,7 @@ impl SimpleAggregationOp { pub(crate) fn is_applicable_to_types(&self, possibilities: ValueTypeSet) -> Result { use self::SimpleAggregationOp::*; if possibilities.is_empty() { - bail!(ErrorKind::CannotProjectImpossibleBinding(*self)) + bail!(ProjectorError::CannotProjectImpossibleBinding(*self)) } match self { @@ -92,7 +92,7 @@ impl SimpleAggregationOp { // The mean of a set of numeric values will always, for our purposes, be a double. Ok(ValueType::Double) } else { - bail!(ErrorKind::CannotApplyAggregateOperationToTypes(*self, possibilities)) + bail!(ProjectorError::CannotApplyAggregateOperationToTypes(*self, possibilities)) } }, &Sum => { @@ -104,7 +104,7 @@ impl SimpleAggregationOp { Ok(ValueType::Long) } } else { - bail!(ErrorKind::CannotApplyAggregateOperationToTypes(*self, possibilities)) + bail!(ProjectorError::CannotApplyAggregateOperationToTypes(*self, possibilities)) } }, @@ -124,7 +124,7 @@ impl SimpleAggregationOp { // These types are unordered. Keyword | Ref | Uuid => { - bail!(ErrorKind::CannotApplyAggregateOperationToTypes(*self, possibilities)) + bail!(ProjectorError::CannotApplyAggregateOperationToTypes(*self, possibilities)) }, } } else { @@ -139,7 +139,7 @@ impl SimpleAggregationOp { Ok(ValueType::Long) } } else { - bail!(ErrorKind::CannotApplyAggregateOperationToTypes(*self, possibilities)) + bail!(ProjectorError::CannotApplyAggregateOperationToTypes(*self, possibilities)) } } }, diff --git a/query-projector/src/errors.rs b/query-projector/src/errors.rs index 8db51d21..d25366eb 100644 --- a/query-projector/src/errors.rs +++ b/query-projector/src/errors.rs @@ -8,72 +8,58 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -use rusqlite; +use std; // To refer to std::result::Result. + +use failure::{ + Error, +}; use mentat_core::{ ValueTypeSet, }; -use mentat_db; - use mentat_query::{ PlainSymbol, }; -use mentat_query_pull; - use aggregates::{ SimpleAggregationOp, }; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - errors { - /// We're just not done yet. Message that the feature is recognized but not yet - /// implemented. - NotYetImplemented(t: String) { - description("not yet implemented") - display("not yet implemented: {}", t) - } - CannotProjectImpossibleBinding(op: SimpleAggregationOp) { - description("no possible types for variable in projection list") - display("no possible types for value provided to {:?}", op) - } - CannotApplyAggregateOperationToTypes(op: SimpleAggregationOp, types: ValueTypeSet) { - description("cannot apply projection operation to types") - display("cannot apply projection operation {:?} to types {:?}", op, types) - } - InvalidProjection(t: String) { - description("invalid projection") - display("invalid projection: {}", t) - } - UnboundVariable(var: PlainSymbol) { - description("cannot project unbound variable") - display("cannot project unbound variable {:?}", var) - } - NoTypeAvailableForVariable(var: PlainSymbol) { - description("cannot find type for variable") - display("cannot find type for variable {:?}", var) - } - UnexpectedResultsType(actual: &'static str, expected: &'static str) { - description("unexpected query results type") - display("expected {}, got {}", expected, actual) - } - AmbiguousAggregates(min_max_count: usize, corresponding_count: usize) { - description("ambiguous aggregates") - display("min/max expressions: {} (max 1), corresponding: {}", min_max_count, corresponding_count) - } - } - - foreign_links { - Rusqlite(rusqlite::Error); - } - - links { - DbError(mentat_db::Error, mentat_db::ErrorKind); - PullError(mentat_query_pull::errors::Error, mentat_query_pull::errors::ErrorKind); - } +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + +pub type Result = std::result::Result; + +#[derive(Debug, Fail)] +pub enum ProjectorError { + /// We're just not done yet. Message that the feature is recognized but not yet + /// implemented. + #[fail(display = "not yet implemented: {}", _0)] + NotYetImplemented(String), + + #[fail(display = "no possible types for value provided to {:?}", _0)] + CannotProjectImpossibleBinding(SimpleAggregationOp), + + #[fail(display = "cannot apply projection operation {:?} to types {:?}", _0, _1)] + CannotApplyAggregateOperationToTypes(SimpleAggregationOp, ValueTypeSet), + + #[fail(display = "invalid projection: {}", _0)] + InvalidProjection(String), + + #[fail(display = "cannot project unbound variable {:?}", _0)] + UnboundVariable(PlainSymbol), + + #[fail(display = "cannot find type for variable {:?}", _0)] + NoTypeAvailableForVariable(PlainSymbol), + + #[fail(display = "expected {}, got {}", _0, _1)] + UnexpectedResultsType(&'static str, &'static str), + + #[fail(display = "min/max expressions: {} (max 1), corresponding: {}", _0, _1)] + AmbiguousAggregates(usize, usize), } diff --git a/query-projector/src/lib.rs b/query-projector/src/lib.rs index 94bc9d5b..c74fcc31 100644 --- a/query-projector/src/lib.rs +++ b/query-projector/src/lib.rs @@ -8,8 +8,10 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. +extern crate failure; + #[macro_use] -extern crate error_chain; +extern crate failure_derive; extern crate indexmap; extern crate rusqlite; @@ -67,12 +69,14 @@ use mentat_query_sql::{ Projection, }; +#[macro_use] +pub mod errors; + mod aggregates; mod project; mod projectors; mod pull; mod relresult; -pub mod errors; pub use aggregates::{ SimpleAggregationOp, @@ -109,7 +113,7 @@ pub use relresult::{ }; use errors::{ - ErrorKind, + ProjectorError, Result, }; @@ -296,35 +300,35 @@ impl QueryResults { pub fn into_scalar(self) -> Result> { match self { QueryResults::Scalar(o) => Ok(o), - QueryResults::Coll(_) => bail!(ErrorKind::UnexpectedResultsType("coll", "scalar")), - QueryResults::Tuple(_) => bail!(ErrorKind::UnexpectedResultsType("tuple", "scalar")), - QueryResults::Rel(_) => bail!(ErrorKind::UnexpectedResultsType("rel", "scalar")), + QueryResults::Coll(_) => bail!(ProjectorError::UnexpectedResultsType("coll", "scalar")), + QueryResults::Tuple(_) => bail!(ProjectorError::UnexpectedResultsType("tuple", "scalar")), + QueryResults::Rel(_) => bail!(ProjectorError::UnexpectedResultsType("rel", "scalar")), } } pub fn into_coll(self) -> Result> { match self { - QueryResults::Scalar(_) => bail!(ErrorKind::UnexpectedResultsType("scalar", "coll")), + QueryResults::Scalar(_) => bail!(ProjectorError::UnexpectedResultsType("scalar", "coll")), QueryResults::Coll(c) => Ok(c), - QueryResults::Tuple(_) => bail!(ErrorKind::UnexpectedResultsType("tuple", "coll")), - QueryResults::Rel(_) => bail!(ErrorKind::UnexpectedResultsType("rel", "coll")), + QueryResults::Tuple(_) => bail!(ProjectorError::UnexpectedResultsType("tuple", "coll")), + QueryResults::Rel(_) => bail!(ProjectorError::UnexpectedResultsType("rel", "coll")), } } pub fn into_tuple(self) -> Result>> { match self { - QueryResults::Scalar(_) => bail!(ErrorKind::UnexpectedResultsType("scalar", "tuple")), - QueryResults::Coll(_) => bail!(ErrorKind::UnexpectedResultsType("coll", "tuple")), + QueryResults::Scalar(_) => bail!(ProjectorError::UnexpectedResultsType("scalar", "tuple")), + QueryResults::Coll(_) => bail!(ProjectorError::UnexpectedResultsType("coll", "tuple")), QueryResults::Tuple(t) => Ok(t), - QueryResults::Rel(_) => bail!(ErrorKind::UnexpectedResultsType("rel", "tuple")), + QueryResults::Rel(_) => bail!(ProjectorError::UnexpectedResultsType("rel", "tuple")), } } pub fn into_rel(self) -> Result> { match self { - QueryResults::Scalar(_) => bail!(ErrorKind::UnexpectedResultsType("scalar", "rel")), - QueryResults::Coll(_) => bail!(ErrorKind::UnexpectedResultsType("coll", "rel")), - QueryResults::Tuple(_) => bail!(ErrorKind::UnexpectedResultsType("tuple", "rel")), + QueryResults::Scalar(_) => bail!(ProjectorError::UnexpectedResultsType("scalar", "rel")), + QueryResults::Coll(_) => bail!(ProjectorError::UnexpectedResultsType("coll", "rel")), + QueryResults::Tuple(_) => bail!(ProjectorError::UnexpectedResultsType("tuple", "rel")), QueryResults::Rel(r) => Ok(r), } } diff --git a/query-projector/src/project.rs b/query-projector/src/project.rs index 57ec1619..92d43433 100644 --- a/query-projector/src/project.rs +++ b/query-projector/src/project.rs @@ -55,7 +55,7 @@ use aggregates::{ }; use errors::{ - ErrorKind, + ProjectorError, Result, }; @@ -127,14 +127,14 @@ fn candidate_type_column(cc: &ConjoiningClauses, var: &Variable) -> Result<(Colu let type_name = VariableColumn::VariableTypeTag(var.clone()).column_name(); (ColumnOrExpression::Column(alias), type_name) }) - .ok_or_else(|| ErrorKind::UnboundVariable(var.name()).into()) + .ok_or_else(|| ProjectorError::UnboundVariable(var.name()).into()) } fn cc_column(cc: &ConjoiningClauses, var: &Variable) -> Result { cc.column_bindings .get(var) .and_then(|cols| cols.get(0).cloned()) - .ok_or_else(|| ErrorKind::UnboundVariable(var.name()).into()) + .ok_or_else(|| ProjectorError::UnboundVariable(var.name()).into()) } fn candidate_column(cc: &ConjoiningClauses, var: &Variable) -> Result<(ColumnOrExpression, Name)> { @@ -211,18 +211,18 @@ pub(crate) fn project_elements<'a, I: IntoIterator>( match e { &Element::Variable(ref var) => { if outer_variables.contains(var) { - bail!(ErrorKind::InvalidProjection(format!("Duplicate variable {} in query.", var))); + bail!(ProjectorError::InvalidProjection(format!("Duplicate variable {} in query.", var))); } if corresponded_variables.contains(var) { - bail!(ErrorKind::InvalidProjection(format!("Can't project both {} and `(the {})` from a query.", var, var))); + bail!(ProjectorError::InvalidProjection(format!("Can't project both {} and `(the {})` from a query.", var, var))); } }, &Element::Corresponding(ref var) => { if outer_variables.contains(var) { - bail!(ErrorKind::InvalidProjection(format!("Can't project both {} and `(the {})` from a query.", var, var))); + bail!(ProjectorError::InvalidProjection(format!("Can't project both {} and `(the {})` from a query.", var, var))); } if corresponded_variables.contains(var) { - bail!(ErrorKind::InvalidProjection(format!("`(the {})` appears twice in query.", var))); + bail!(ProjectorError::InvalidProjection(format!("`(the {})` appears twice in query.", var))); } }, &Element::Aggregate(_) => { @@ -346,7 +346,7 @@ pub(crate) fn project_elements<'a, I: IntoIterator>( i += 1; } else { // TODO: complex aggregates. - bail!(ErrorKind::NotYetImplemented("complex aggregates".into())); + bail!(ProjectorError::NotYetImplemented("complex aggregates".into())); } }, } @@ -355,13 +355,13 @@ pub(crate) fn project_elements<'a, I: IntoIterator>( match (min_max_count, corresponded_variables.len()) { (0, 0) | (_, 0) => {}, (0, _) => { - bail!(ErrorKind::InvalidProjection("Warning: used `the` without `min` or `max`.".to_string())); + bail!(ProjectorError::InvalidProjection("Warning: used `the` without `min` or `max`.".to_string())); }, (1, _) => { // This is the success case! }, (n, c) => { - bail!(ErrorKind::AmbiguousAggregates(n, c)); + bail!(ProjectorError::AmbiguousAggregates(n, c)); }, } @@ -465,7 +465,7 @@ pub(crate) fn project_elements<'a, I: IntoIterator>( .extracted_types .get(&var) .cloned() - .ok_or_else(|| ErrorKind::NoTypeAvailableForVariable(var.name().clone()))?; + .ok_or_else(|| ProjectorError::NoTypeAvailableForVariable(var.name().clone()))?; inner_projection.push(ProjectedColumn(ColumnOrExpression::Column(type_col), type_name.clone())); } if group { diff --git a/query-projector/src/pull.rs b/query-projector/src/pull.rs index bca0c217..b0ee05e1 100644 --- a/query-projector/src/pull.rs +++ b/query-projector/src/pull.rs @@ -30,9 +30,7 @@ use mentat_query_pull::{ Puller, }; -use errors::{ - Result, -}; +use errors::Result; use super::{ Index, diff --git a/query-projector/tests/aggregates.rs b/query-projector/tests/aggregates.rs index 6b148ed3..584f6431 100644 --- a/query-projector/tests/aggregates.rs +++ b/query-projector/tests/aggregates.rs @@ -99,11 +99,10 @@ fn test_the_without_max_or_min() { let projection = query_projection(&schema, &algebrized); assert!(projection.is_err()); use ::mentat_query_projector::errors::{ - ErrorKind, - Error, + ProjectorError, }; - match projection { - Result::Err(Error(ErrorKind::InvalidProjection(s) , _)) => { + match projection.err().expect("expected failure").downcast().expect("expected specific error") { + ProjectorError::InvalidProjection(s) => { assert_eq!(s.as_str(), "Warning: used `the` without `min` or `max`."); }, _ => panic!(), From 836fdb3a35a9d8f093aee408ed3f703e9a975543 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 16:59:20 -0400 Subject: [PATCH 05/11] Convert query_translator/ to failure. --- query-translator/Cargo.toml | 3 ++- query-translator/src/lib.rs | 18 ++++-------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/query-translator/Cargo.toml b/query-translator/Cargo.toml index b729a12d..b77b449b 100644 --- a/query-translator/Cargo.toml +++ b/query-translator/Cargo.toml @@ -4,7 +4,8 @@ version = "0.0.1" workspace = ".." [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" [dependencies.mentat_core] path = "../core" diff --git a/query-translator/src/lib.rs b/query-translator/src/lib.rs index 35ea8a5e..b122c670 100644 --- a/query-translator/src/lib.rs +++ b/query-translator/src/lib.rs @@ -8,8 +8,7 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -#[macro_use] -extern crate error_chain; +extern crate failure; extern crate mentat_core; extern crate mentat_query; extern crate mentat_query_algebrizer; @@ -17,6 +16,8 @@ extern crate mentat_query_projector; extern crate mentat_query_sql; extern crate mentat_sql; +use failure::Error; + mod translate; pub use mentat_query_sql::{ @@ -29,15 +30,4 @@ pub use translate::{ query_to_select, }; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - foreign_links { - } - - links { - ProjectorError(mentat_query_projector::errors::Error, mentat_query_projector::errors::ErrorKind); - } -} +type Result = std::result::Result; From 4e0192933484fff8480c4f20e69308125a6c94b3 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 18:07:09 -0400 Subject: [PATCH 06/11] Convert src/ to failure. --- Cargo.toml | 3 +- src/conn.rs | 50 +++++++-------- src/entity_builder.rs | 13 ++-- src/errors.rs | 137 +++++++++++++++--------------------------- src/lib.rs | 6 +- src/query.rs | 8 +-- src/query_builder.rs | 4 +- src/store.rs | 4 +- src/vocabulary.rs | 20 +++--- tests/query.rs | 74 ++++++++--------------- tests/vocabulary.rs | 9 +-- 11 files changed, 131 insertions(+), 197 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 33d49058..93ee9ca6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,8 @@ rustc_version = "0.2" [dependencies] chrono = "0.4" -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" lazy_static = "0.2" time = "0.1" uuid = { version = "0.5", features = ["v4", "serde"] } diff --git a/src/conn.rs b/src/conn.rs index 894de855..be872a88 100644 --- a/src/conn.rs +++ b/src/conn.rs @@ -93,7 +93,10 @@ use entity_builder::{ TermBuilder, }; -use errors::*; +use errors::{ + Result, + MentatError, +}; use query::{ Known, @@ -474,7 +477,7 @@ impl<'a, 'c> InProgress<'a, 'c> { // Retrying is tracked by https://github.com/mozilla/mentat/issues/357. // This should not occur -- an attempt to take a competing IMMEDIATE transaction // will fail with `SQLITE_BUSY`, causing this function to abort. - bail!("Lost the transact() race!"); + bail!(MentatError::UnexpectedLostTransactRace); } // Commit the SQLite transaction while we hold the mutex. @@ -506,7 +509,7 @@ impl<'a, 'c> InProgress<'a, 'c> { cache_action: CacheAction) -> Result<()> { let attribute_entid: Entid = self.schema .attribute_for_ident(&attribute) - .ok_or_else(|| ErrorKind::UnknownAttribute(attribute.to_string()))?.1.into(); + .ok_or_else(|| MentatError::UnknownAttribute(attribute.to_string()))?.1.into(); match cache_action { CacheAction::Register => { @@ -579,17 +582,16 @@ impl Conn { /// Prepare the provided SQLite handle for use as a Mentat store. Creates tables but /// _does not_ write the bootstrap schema. This constructor should only be used by /// consumers that expect to populate raw transaction data themselves. + pub(crate) fn empty(sqlite: &mut rusqlite::Connection) -> Result { - let (tx, db) = db::create_empty_current_version(sqlite) - .chain_err(|| "Unable to initialize Mentat store")?; + let (tx, db) = db::create_empty_current_version(sqlite)?; tx.commit()?; Ok(Conn::new(db.partition_map, db.schema)) } pub fn connect(sqlite: &mut rusqlite::Connection) -> Result { - let db = db::ensure_current_version(sqlite) - .chain_err(|| "Unable to initialize Mentat store")?; + let db = db::ensure_current_version(sqlite)?; Ok(Conn::new(db.partition_map, db.schema)) } @@ -801,7 +803,7 @@ impl Conn { { attribute_entid = metadata.schema .attribute_for_ident(&attribute) - .ok_or_else(|| ErrorKind::UnknownAttribute(attribute.to_string()))?.1.into(); + .ok_or_else(|| MentatError::UnknownAttribute(attribute.to_string()))?.1.into(); } let cache = &mut metadata.attribute_cache; @@ -869,11 +871,11 @@ mod tests { .partition_map[":db.part/user"].index; let t = format!("[[:db/add {} :db.schema/attribute \"tempid\"]]", next + 1); - match conn.transact(&mut sqlite, t.as_str()).unwrap_err() { - Error(ErrorKind::DbError(::mentat_db::errors::ErrorKind::UnrecognizedEntid(e)), _) => { + match conn.transact(&mut sqlite, t.as_str()).expect_err("expected transact error").downcast() { + Ok(::mentat_db::DbError::UnrecognizedEntid(e)) => { assert_eq!(e, next + 1); }, - x => panic!("expected transact error, got {:?}", x), + x => panic!("expected db error, got {:?}", x), } // Transact two more tempids. @@ -896,12 +898,12 @@ mod tests { // we should reject this, because the first ID was provided by the user! let t = format!("[[:db/add {} :db.schema/attribute \"tempid\"]]", next); - match conn.transact(&mut sqlite, t.as_str()).unwrap_err() { - Error(ErrorKind::DbError(::mentat_db::errors::ErrorKind::UnrecognizedEntid(e)), _) => { + match conn.transact(&mut sqlite, t.as_str()).expect_err("expected transact error").downcast() { + Ok(::mentat_db::DbError::UnrecognizedEntid(e)) => { // All this, despite this being the ID we were about to allocate! assert_eq!(e, next); }, - x => panic!("expected transact error, got {:?}", x), + x => panic!("expected db error, got {:?}", x), } // And if we subsequently transact in a way that allocates one ID, we _will_ use that one. @@ -1057,9 +1059,9 @@ mod tests { // Bad EDN: missing closing ']'. let report = conn.transact(&mut sqlite, "[[:db/add \"t\" :db/ident :a/keyword]"); - match report.unwrap_err() { - Error(ErrorKind::EdnParseError(_), _) => { }, - x => panic!("expected EDN parse error, got {:?}", x), + match report.expect_err("expected transact to fail for bad edn").downcast() { + Ok(edn::ParseError { .. }) => { }, + Err(x) => panic!("expected EDN parse error, got {:?}", x), } // Good EDN. @@ -1068,9 +1070,9 @@ mod tests { // Bad transaction data: missing leading :db/add. let report = conn.transact(&mut sqlite, "[[\"t\" :db/ident :b/keyword]]"); - match report.unwrap_err() { - Error(ErrorKind::EdnParseError(_), _) => { }, - x => panic!("expected EDN parse error, got {:?}", x), + match report.expect_err("expected transact error").downcast() { + Ok(edn::ParseError { .. }) => { }, + Err(x) => panic!("expected EDN parse error, got {:?}", x), } // Good transaction data. @@ -1080,8 +1082,8 @@ mod tests { // Bad transaction based on state of store: conflicting upsert. let report = conn.transact(&mut sqlite, "[[:db/add \"u\" :db/ident :a/keyword] [:db/add \"u\" :db/ident :b/keyword]]"); - match report.unwrap_err() { - Error(ErrorKind::DbError(::mentat_db::errors::ErrorKind::SchemaConstraintViolation(_)), _) => { }, + match report.expect_err("expected transact error").downcast() { + Ok(::mentat_db::DbError::SchemaConstraintViolation(_)) => { }, x => panic!("expected schema constraint violation, got {:?}", x), } } @@ -1099,8 +1101,8 @@ mod tests { let kw = kw!(:foo/bat); let schema = conn.current_schema(); let res = conn.cache(&mut sqlite, &schema, &kw, CacheDirection::Forward, CacheAction::Register); - match res.unwrap_err() { - Error(ErrorKind::UnknownAttribute(msg), _) => assert_eq!(msg, ":foo/bat"), + match res.expect_err("expected cache to fail").downcast() { + Ok(MentatError::UnknownAttribute(msg)) => assert_eq!(msg, ":foo/bat"), x => panic!("expected UnknownAttribute error, got {:?}", x), } } diff --git a/src/entity_builder.rs b/src/entity_builder.rs index 263ed604..ffa1f56a 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -85,7 +85,7 @@ use conn::{ }; use errors::{ - ErrorKind, + MentatError, Result, }; @@ -277,12 +277,12 @@ impl<'a, 'c> InProgressBuilder<'a, 'c> { let provided = tv.value_type(); let expected = attr.value_type; if provided != expected { - bail!(ErrorKind::ValueTypeMismatch(provided, expected)); + bail!(MentatError::ValueTypeMismatch(provided, expected)); } } attribute = aa; } else { - bail!(ErrorKind::UnknownAttribute(a.to_string())); + bail!(MentatError::UnknownAttribute(a.to_string())); } Ok((attribute, v)) } @@ -380,11 +380,6 @@ impl FromThing for TypedValueOr { mod testing { extern crate mentat_db; - use errors::{ - Error, - ErrorKind, - }; - // For matching inside a test. use mentat_db::ErrorKind::{ UnrecognizedEntid, @@ -429,7 +424,7 @@ mod testing { let mut in_progress = conn.begin_transaction(&mut sqlite).expect("begun successfully"); // This should fail: unrecognized entid. - if let Err(Error(ErrorKind::DbError(UnrecognizedEntid(e)), _)) = in_progress.transact_terms(terms, tempids) { + if let Err(Error(MentatError::DbError(UnrecognizedEntid(e)), _)) = in_progress.transact_terms(terms, tempids) { assert_eq!(e, 999); } else { panic!("Should have rejected the entid."); diff --git a/src/errors.rs b/src/errors.rs index 126faa00..29622c5c 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -10,102 +10,63 @@ #![allow(dead_code)] -use rusqlite; - -use uuid; +use std; // To refer to std::result::Result. use std::collections::BTreeSet; -use edn; +use failure::Error; + use mentat_core::{ Attribute, ValueType, }; -use mentat_db; + use mentat_query; -use mentat_query_algebrizer; -use mentat_query_projector; -use mentat_query_pull; -use mentat_query_translator; -use mentat_sql; -use mentat_tolstoy; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } +pub type Result = std::result::Result; - foreign_links { - EdnParseError(edn::ParseError); - Rusqlite(rusqlite::Error); - UuidParseError(uuid::ParseError); - IoError(::std::io::Error); - } - - links { - DbError(mentat_db::Error, mentat_db::ErrorKind); - QueryError(mentat_query_algebrizer::Error, mentat_query_algebrizer::ErrorKind); // Let's not leak the term 'algebrizer'. - ProjectorError(mentat_query_projector::errors::Error, mentat_query_projector::errors::ErrorKind); - PullError(mentat_query_pull::errors::Error, mentat_query_pull::errors::ErrorKind); - TranslatorError(mentat_query_translator::Error, mentat_query_translator::ErrorKind); - SqlError(mentat_sql::Error, mentat_sql::ErrorKind); - SyncError(mentat_tolstoy::Error, mentat_tolstoy::ErrorKind); - } - - errors { - PathAlreadyExists(path: String) { - description("path already exists") - display("path {} already exists", path) - } - - UnboundVariables(names: BTreeSet) { - description("unbound variables at query execution time") - display("variables {:?} unbound at query execution time", names) - } - - InvalidArgumentName(name: String) { - description("invalid argument name") - display("invalid argument name: '{}'", name) - } - - UnknownAttribute(name: String) { - description("unknown attribute") - display("unknown attribute: '{}'", name) - } - - InvalidVocabularyVersion { - description("invalid vocabulary version") - display("invalid vocabulary version") - } - - ConflictingAttributeDefinitions(vocabulary: String, version: ::vocabulary::Version, attribute: String, current: Attribute, requested: Attribute) { - description("conflicting attribute definitions") - display("vocabulary {}/{} already has attribute {}, and the requested definition differs", vocabulary, version, attribute) - } - - ExistingVocabularyTooNew(name: String, existing: ::vocabulary::Version, ours: ::vocabulary::Version) { - description("existing vocabulary too new") - display("existing vocabulary too new: wanted {}, got {}", ours, existing) - } - - UnexpectedCoreSchema(version: Option<::vocabulary::Version>) { - description("unexpected core schema version") - display("core schema: wanted {}, got {:?}", mentat_db::CORE_SCHEMA_VERSION, version) - } - - MissingCoreVocabulary(kw: mentat_query::Keyword) { - description("missing core vocabulary") - display("missing core attribute {}", kw) - } - - PreparedQuerySchemaMismatch { - description("schema changed since query was prepared") - display("schema changed since query was prepared") - } - - ValueTypeMismatch(provided: ValueType, expected: ValueType) { - description("provided value doesn't match value type") - display("provided value of type {} doesn't match attribute value type {}", provided, expected) - } - } +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + +#[derive(Debug, Fail)] +pub enum MentatError { + #[fail(display = "path {} already exists", _0)] + PathAlreadyExists(String), + + #[fail(display = "variables {:?} unbound at query execution time", _0)] + UnboundVariables(BTreeSet), + + #[fail(display = "invalid argument name: '{}'", _0)] + InvalidArgumentName(String), + + #[fail(display = "unknown attribute: '{}'", _0)] + UnknownAttribute(String), + + #[fail(display = "invalid vocabulary version")] + InvalidVocabularyVersion, + + #[fail(display = "vocabulary {}/{} already has attribute {}, and the requested definition differs", _0, _1, _2)] + ConflictingAttributeDefinitions(String, ::vocabulary::Version, String, Attribute, Attribute), + + #[fail(display = "existing vocabulary {} too new: wanted {}, got {}", _0, _1, _2)] + ExistingVocabularyTooNew(String, ::vocabulary::Version, ::vocabulary::Version), + + #[fail(display = "core schema: wanted {}, got {:?}", _0, _1)] + UnexpectedCoreSchema(::vocabulary::Version, Option<::vocabulary::Version>), + + #[fail(display = "Lost the transact() race!")] + UnexpectedLostTransactRace, + + #[fail(display = "missing core attribute {}", _0)] + MissingCoreVocabulary(mentat_query::Keyword), + + #[fail(display = "schema changed since query was prepared")] + PreparedQuerySchemaMismatch, + + #[fail(display = "provided value of type {} doesn't match attribute value type {}", _0, _1)] + ValueTypeMismatch(ValueType, ValueType), } diff --git a/src/lib.rs b/src/lib.rs index 3fb6b575..b3a657e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,8 @@ #![recursion_limit="128"] #[macro_use] -extern crate error_chain; +extern crate failure_derive; +extern crate failure; #[macro_use] extern crate lazy_static; @@ -101,9 +102,10 @@ macro_rules! kw { }; } +#[macro_use] +pub mod errors; pub mod conn; pub mod entity_builder; -pub mod errors; pub mod ident; pub mod query; pub mod query_builder; diff --git a/src/query.rs b/src/query.rs index 77322472..16fa3e3f 100644 --- a/src/query.rs +++ b/src/query.rs @@ -74,7 +74,7 @@ pub use mentat_query_projector::{ }; use errors::{ - ErrorKind, + MentatError, Result, }; @@ -178,7 +178,7 @@ fn algebrize_query // If they aren't, the user has made an error -- perhaps writing the wrong variable in `:in`, or // not binding in the `QueryInput`. if !unbound.is_empty() { - bail!(ErrorKind::UnboundVariables(unbound.into_iter().map(|v| v.to_string()).collect())); + bail!(MentatError::UnboundVariables(unbound.into_iter().map(|v| v.to_string()).collect())); } Ok(algebrized) } @@ -211,7 +211,7 @@ fn fetch_values<'sqlite> fn lookup_attribute(schema: &Schema, attribute: &Keyword) -> Result { schema.get_entid(attribute) - .ok_or_else(|| ErrorKind::UnknownAttribute(attribute.name().into()).into()) + .ok_or_else(|| MentatError::UnknownAttribute(attribute.name().into()).into()) } /// Return a single value for the provided entity and attribute. @@ -398,7 +398,7 @@ pub fn q_prepare<'sqlite, 'schema, 'cache, 'query, T> if !unbound.is_empty() { // TODO: Allow binding variables at execution time, not just // preparation time. - bail!(ErrorKind::UnboundVariables(unbound.into_iter().map(|v| v.to_string()).collect())); + bail!(MentatError::UnboundVariables(unbound.into_iter().map(|v| v.to_string()).collect())); } if algebrized.is_known_empty() { diff --git a/src/query_builder.rs b/src/query_builder.rs index bafc59e7..6be1d2e0 100644 --- a/src/query_builder.rs +++ b/src/query_builder.rs @@ -34,7 +34,7 @@ use ::{ }; use errors::{ - ErrorKind, + MentatError, Result, }; @@ -56,7 +56,7 @@ impl<'a> QueryBuilder<'a> { } pub fn bind_ref_from_kw(&mut self, var: &str, value: Keyword) -> Result<&mut Self> { - let entid = self.store.conn().current_schema().get_entid(&value).ok_or(ErrorKind::UnknownAttribute(value.to_string()))?; + let entid = self.store.conn().current_schema().get_entid(&value).ok_or(MentatError::UnknownAttribute(value.to_string()))?; self.values.insert(Variable::from_valid_name(var), TypedValue::Ref(entid.into())); Ok(self) } diff --git a/src/store.rs b/src/store.rs index 0fd106f8..30856683 100644 --- a/src/store.rs +++ b/src/store.rs @@ -84,7 +84,7 @@ impl Store { pub fn open_empty(path: &str) -> Result { if !path.is_empty() { if Path::new(path).exists() { - bail!(ErrorKind::PathAlreadyExists(path.to_string())); + bail!(MentatError::PathAlreadyExists(path.to_string())); } } @@ -124,7 +124,7 @@ impl Store { pub fn open_empty_with_key(path: &str, encryption_key: &str) -> Result { if !path.is_empty() { if Path::new(path).exists() { - bail!(ErrorKind::PathAlreadyExists(path.to_string())); + bail!(MentatError::PathAlreadyExists(path.to_string())); } } diff --git a/src/vocabulary.rs b/src/vocabulary.rs index f17a3812..7e622ee2 100644 --- a/src/vocabulary.rs +++ b/src/vocabulary.rs @@ -121,7 +121,7 @@ use ::conn::{ }; use ::errors::{ - ErrorKind, + MentatError, Result, }; @@ -375,17 +375,17 @@ trait HasCoreSchema { impl HasCoreSchema for T where T: HasSchema { fn core_type(&self, t: ValueType) -> Result { self.entid_for_type(t) - .ok_or_else(|| ErrorKind::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) + .ok_or_else(|| MentatError::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) } fn core_entid(&self, ident: &Keyword) -> Result { self.get_entid(ident) - .ok_or_else(|| ErrorKind::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) + .ok_or_else(|| MentatError::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) } fn core_attribute(&self, ident: &Keyword) -> Result { self.attribute_for_ident(ident) - .ok_or_else(|| ErrorKind::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) + .ok_or_else(|| MentatError::MissingCoreVocabulary(DB_SCHEMA_VERSION.clone()).into()) .map(|(_, e)| e) } } @@ -568,7 +568,7 @@ pub trait VersionedStore: HasVocabularies + HasSchema { // We have two vocabularies with the same name, same version, and // different definitions for an attribute. That's a coding error. // We can't accept this vocabulary. - bail!(ErrorKind::ConflictingAttributeDefinitions( + bail!(MentatError::ConflictingAttributeDefinitions( definition.name.to_string(), definition.version, pair.0.to_string(), @@ -615,13 +615,13 @@ pub trait VersionedStore: HasVocabularies + HasSchema { fn verify_core_schema(&self) -> Result<()> { if let Some(core) = self.read_vocabulary_named(&DB_SCHEMA_CORE)? { if core.version != CORE_SCHEMA_VERSION { - bail!(ErrorKind::UnexpectedCoreSchema(Some(core.version))); + bail!(MentatError::UnexpectedCoreSchema(CORE_SCHEMA_VERSION, Some(core.version))); } // TODO: check things other than the version. } else { // This would be seriously messed up. - bail!(ErrorKind::UnexpectedCoreSchema(None)); + bail!(MentatError::UnexpectedCoreSchema(CORE_SCHEMA_VERSION, None)); } Ok(()) } @@ -682,7 +682,7 @@ impl<'a, 'c> VersionedStore for InProgress<'a, 'c> { VocabularyCheck::NotPresent => self.install_vocabulary(definition), VocabularyCheck::PresentButNeedsUpdate { older_version } => self.upgrade_vocabulary(definition, older_version), VocabularyCheck::PresentButMissingAttributes { attributes } => self.install_attributes_for(definition, attributes), - VocabularyCheck::PresentButTooNew { newer_version } => Err(ErrorKind::ExistingVocabularyTooNew(definition.name.to_string(), newer_version.version, definition.version).into()), + VocabularyCheck::PresentButTooNew { newer_version } => Err(MentatError::ExistingVocabularyTooNew(definition.name.to_string(), newer_version.version, definition.version).into()), } } @@ -701,7 +701,7 @@ impl<'a, 'c> VersionedStore for InProgress<'a, 'c> { out.insert(definition.name.clone(), VocabularyOutcome::Existed); }, VocabularyCheck::PresentButTooNew { newer_version } => { - bail!(ErrorKind::ExistingVocabularyTooNew(definition.name.to_string(), newer_version.version, definition.version)); + bail!(MentatError::ExistingVocabularyTooNew(definition.name.to_string(), newer_version.version, definition.version)); }, c @ VocabularyCheck::NotPresent | @@ -868,7 +868,7 @@ impl HasVocabularies for T where T: HasSchema + Queryable { attributes: attributes, })) }, - Some(_) => bail!(ErrorKind::InvalidVocabularyVersion), + Some(_) => bail!(MentatError::InvalidVocabularyVersion), } } else { Ok(None) diff --git a/tests/query.rs b/tests/query.rs index 6b7d2a48..8b3b012e 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -61,8 +61,7 @@ use mentat::query::q_uncached; use mentat::conn::Conn; use mentat::errors::{ - Error, - ErrorKind, + MentatError, }; #[test] @@ -234,11 +233,11 @@ fn test_unbound_inputs() { let results = q_uncached(&c, &db.schema, "[:find ?i . :in ?e :where [?e :db/ident ?i]]", inputs); - match results { - Result::Err(Error(ErrorKind::UnboundVariables(vars), _)) => { + match results.expect_err("expected unbound variables").downcast().expect("expected specific error") { + MentatError::UnboundVariables(vars) => { assert_eq!(vars, vec!["?e".to_string()].into_iter().collect()); }, - _ => panic!("Expected unbound variables."), + _ => panic!("Expected UnboundVariables variant."), } } @@ -412,8 +411,8 @@ fn test_fulltext() { [?a :foo/term ?term] ]"#; let r = conn.q_once(&mut c, query, None); - match r { - Err(Error(ErrorKind::QueryError(mentat_query_algebrizer::ErrorKind::InvalidArgument(PlainSymbol(s), ty, i)), _)) => { + match r.expect_err("expected query to fail").downcast() { + Ok(mentat_query_algebrizer::AlgebrizerError::InvalidArgument(PlainSymbol(s), ty, i)) => { assert_eq!(s, "fulltext"); assert_eq!(ty, "string"); assert_eq!(i, 2); @@ -427,8 +426,8 @@ fn test_fulltext() { [?a :foo/term ?term] [(fulltext $ :foo/fts ?a) [[?x ?val]]]]"#; let r = conn.q_once(&mut c, query, None); - match r { - Err(Error(ErrorKind::QueryError(mentat_query_algebrizer::ErrorKind::InvalidArgument(PlainSymbol(s), ty, i)), _)) => { + match r.expect_err("expected query to fail").downcast() { + Ok(mentat_query_algebrizer::AlgebrizerError::InvalidArgument(PlainSymbol(s), ty, i)) => { assert_eq!(s, "fulltext"); assert_eq!(ty, "string"); assert_eq!(i, 2); @@ -583,42 +582,25 @@ fn test_aggregates_type_handling() { // No type limits => can't do it. let r = store.q_once(r#"[:find (sum ?v) . :where [_ _ ?v]]"#, None); let all_types = ValueTypeSet::any(); - match r { - Result::Err( - Error( - ErrorKind::TranslatorError( - ::mentat_query_translator::ErrorKind::ProjectorError( - ::mentat_query_projector::errors::ErrorKind::CannotApplyAggregateOperationToTypes( - SimpleAggregationOp::Sum, - types - ), - ) - ), - _)) => { - assert_eq!(types, all_types); - }, - r => panic!("Unexpected: {:?}", r), + match r.expect_err("expected query to fail").downcast() { + Ok(::mentat_query_projector::errors::ProjectorError::CannotApplyAggregateOperationToTypes( + SimpleAggregationOp::Sum, types)) => { + assert_eq!(types, all_types); + }, + e => panic!("Unexpected error type {:?}", e), } // You can't sum instants. let r = store.q_once(r#"[:find (sum ?v) . :where [_ _ ?v] [(type ?v :db.type/instant)]]"#, None); - match r { - Result::Err( - Error( - ErrorKind::TranslatorError( - ::mentat_query_translator::ErrorKind::ProjectorError( - ::mentat_query_projector::errors::ErrorKind::CannotApplyAggregateOperationToTypes( - SimpleAggregationOp::Sum, - types - ), - ) - ), - _)) => { - assert_eq!(types, ValueTypeSet::of_one(ValueType::Instant)); - }, - r => panic!("Unexpected: {:?}", r), + match r.expect_err("expected query to fail").downcast() { + Ok(::mentat_query_projector::errors::ProjectorError::CannotApplyAggregateOperationToTypes( + SimpleAggregationOp::Sum, + types)) => { + assert_eq!(types, ValueTypeSet::of_one(ValueType::Instant)); + }, + e => panic!("Unexpected error type {:?}", e), } // But you can count them. @@ -1354,19 +1336,13 @@ fn test_aggregation_implicit_grouping() { [?person :foo/play ?game] [?person :foo/is-vegetarian true] [?person :foo/name ?name]]"#, None); - match res { - Result::Err( - Error( - ErrorKind::TranslatorError( - ::mentat_query_translator::ErrorKind::ProjectorError( - ::mentat_query_projector::errors::ErrorKind::AmbiguousAggregates(mmc, cc) - ) - ), _)) => { + match res.expect_err("expected query to fail").downcast() { + Ok(::mentat_query_projector::errors::ProjectorError::AmbiguousAggregates(mmc, cc)) => { assert_eq!(mmc, 2); assert_eq!(cc, 1); }, - r => { - panic!("Unexpected result {:?}.", r); + e => { + panic!("Unexpected error type {:?}.", e); }, } diff --git a/tests/vocabulary.rs b/tests/vocabulary.rs index 46dd4c1f..9e4841d2 100644 --- a/tests/vocabulary.rs +++ b/tests/vocabulary.rs @@ -59,10 +59,7 @@ use mentat::entity_builder::{ TermBuilder, }; -use mentat::errors::{ - Error, - ErrorKind, -}; +use mentat::errors::MentatError; lazy_static! { static ref FOO_NAME: Keyword = { @@ -289,8 +286,8 @@ fn test_add_vocab() { // Scoped borrow of `conn`. { let mut in_progress = conn.begin_transaction(&mut sqlite).expect("begun successfully"); - match in_progress.ensure_vocabulary(&foo_v1_malformed) { - Result::Err(Error(ErrorKind::ConflictingAttributeDefinitions(vocab, version, attr, theirs, ours), _)) => { + match in_progress.ensure_vocabulary(&foo_v1_malformed).expect_err("expected vocabulary to fail").downcast() { + Ok(MentatError::ConflictingAttributeDefinitions(vocab, version, attr, theirs, ours)) => { assert_eq!(vocab.as_str(), ":org.mozilla/foo"); assert_eq!(attr.as_str(), ":foo/baz"); assert_eq!(version, 1); From 800f404a23a5102771d8f0d68bcf9bf0f1b9d1ac Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 20:10:46 -0400 Subject: [PATCH 07/11] Convert ffi/ to failure. This is neat, because currently at the FFI boundary we're primarily concerned with verbalizing our errors. It doesn't matter what 'error' that's wrapped by Result is then, as long as it can be displayed. Once we're past the prototyping stage, it might be a good idea to formalize this. --- ffi/src/lib.rs | 5 +++- tools/cli/src/mentat_cli/errors.rs | 37 ------------------------------ 2 files changed, 4 insertions(+), 38 deletions(-) delete mode 100644 tools/cli/src/mentat_cli/errors.rs diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index 7e8eaa49..a665b25a 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -67,9 +67,12 @@ //! propogation. These types have implemented [From](std::convert::From) such that conversion from the Rust type //! to the C type is as painless as possible. +extern crate core; extern crate libc; extern crate mentat; +use core::fmt::Display; + use std::collections::{ BTreeSet, }; @@ -204,7 +207,7 @@ pub struct ExternResult { pub err: *const c_char, } -impl From> for ExternResult where E: std::error::Error { +impl From> for ExternResult where E: Display { fn from(result: Result) -> Self { match result { Ok(value) => { diff --git a/tools/cli/src/mentat_cli/errors.rs b/tools/cli/src/mentat_cli/errors.rs deleted file mode 100644 index 0012c319..00000000 --- a/tools/cli/src/mentat_cli/errors.rs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2016 Mozilla -// -// Licensed under the Apache License, Version 2.0 (the "License"); you may not use -// this file except in compliance with the License. You may obtain a copy of the -// License at http://www.apache.org/licenses/LICENSE-2.0 -// Unless required by applicable law or agreed to in writing, software distributed -// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR -// CONDITIONS OF ANY KIND, either express or implied. See the License for the -// specific language governing permissions and limitations under the License. - -#![allow(dead_code)] - -use rusqlite; - -use mentat::errors as mentat; - -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - foreign_links { - Rusqlite(rusqlite::Error); - IoError(::std::io::Error); - } - - links { - MentatError(mentat::Error, mentat::ErrorKind); - } - - errors { - CommandParse(message: String) { - description("An error occured parsing the entered command") - display("{}", message) - } - } -} From 0adfa6aae623b3988a688b8b6eef14ce96e213c0 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 4 Jun 2018 20:59:27 -0400 Subject: [PATCH 08/11] Convert tools/cli to failure. --- tools/cli/Cargo.toml | 3 +- tools/cli/src/mentat_cli/command_parser.rs | 36 ++++++++++++++-------- tools/cli/src/mentat_cli/input.rs | 4 +-- tools/cli/src/mentat_cli/lib.rs | 10 ++++-- tools/cli/src/mentat_cli/repl.rs | 9 ++++-- 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/tools/cli/Cargo.toml b/tools/cli/Cargo.toml index 081a2d4f..6a6eb088 100644 --- a/tools/cli/Cargo.toml +++ b/tools/cli/Cargo.toml @@ -20,6 +20,8 @@ test = false [dependencies] combine = "2.2.2" env_logger = "0.5" +failure = "0.1.1" +failure_derive = "0.1.1" getopts = "0.2" lazy_static = "0.2" linefeed = "0.4" @@ -28,7 +30,6 @@ tabwriter = "1" tempfile = "1.1" termion = "1" time = "0.1" -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } [dependencies.rusqlite] version = "0.13" diff --git a/tools/cli/src/mentat_cli/command_parser.rs b/tools/cli/src/mentat_cli/command_parser.rs index ec49618d..69de29e3 100644 --- a/tools/cli/src/mentat_cli/command_parser.rs +++ b/tools/cli/src/mentat_cli/command_parser.rs @@ -30,14 +30,26 @@ use combine::combinator::{ try, }; -use errors as cli; +use CliError; use edn; +use failure::{ + Compat, + Error, +}; + use mentat::{ CacheDirection, }; +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + pub static COMMAND_CACHE: &'static str = &"cache"; pub static COMMAND_CLOSE: &'static str = &"close"; pub static COMMAND_EXIT_LONG: &'static str = &"exit"; @@ -188,7 +200,7 @@ impl Command { } } -pub fn command(s: &str) -> Result { +pub fn command(s: &str) -> Result { let path = || many1::(satisfy(|c: char| !c.is_whitespace())); let argument = || many1::(satisfy(|c: char| !c.is_whitespace())); let arguments = || sep_end_by::, _, _>(many1(satisfy(|c: char| !c.is_whitespace())), many1::, _>(space())).expected("arguments"); @@ -202,7 +214,7 @@ pub fn command(s: &str) -> Result { let edn_arg_parser = || spaces() .with(look_ahead(string("[").or(string("{"))) .with(many1::, _>(try(any()))) - .and_then(|args| -> Result { + .and_then(|args| -> Result> { Ok(args.iter().collect()) }) ); @@ -217,10 +229,10 @@ pub fn command(s: &str) -> Result { .with(arguments()) .map(move |args| { if args.len() < num_args { - bail!(cli::ErrorKind::CommandParse("Missing required argument".to_string())); + bail!(CliError::CommandParse("Missing required argument".to_string())); } if args.len() > num_args { - bail!(cli::ErrorKind::CommandParse(format!("Unrecognized argument {:?}", args[num_args]))); + bail!(CliError::CommandParse(format!("Unrecognized argument {:?}", args[num_args]))); } Ok(args) }) @@ -239,7 +251,7 @@ pub fn command(s: &str) -> Result { .with(no_arg_parser()) .map(|args| { if !args.is_empty() { - bail!(cli::ErrorKind::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); + bail!(CliError::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); } Ok(Command::Close) }); @@ -248,7 +260,7 @@ pub fn command(s: &str) -> Result { .with(no_arg_parser()) .map(|args| { if !args.is_empty() { - bail!(cli::ErrorKind::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); + bail!(CliError::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); } Ok(Command::Exit) }); @@ -302,7 +314,7 @@ pub fn command(s: &str) -> Result { .with(no_arg_parser()) .map(|args| { if !args.is_empty() { - bail!(cli::ErrorKind::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); + bail!(CliError::CommandParse(format!("Unrecognized argument {:?}", args[0])) ); } Ok(Command::Schema) }); @@ -312,10 +324,10 @@ pub fn command(s: &str) -> Result { .with(arguments()) .map(|args| { if args.len() < 1 { - bail!(cli::ErrorKind::CommandParse("Missing required argument".to_string())); + bail!(CliError::CommandParse("Missing required argument".to_string())); } if args.len() > 2 { - bail!(cli::ErrorKind::CommandParse(format!("Unrecognized argument {:?}", args[2]))); + bail!(CliError::CommandParse(format!("Unrecognized argument {:?}", args[2]))); } Ok(Command::Sync(args.clone())) }); @@ -335,7 +347,7 @@ pub fn command(s: &str) -> Result { spaces() .skip(token('.')) - .with(choice::<[&mut Parser>; 16], _> + .with(choice::<[&mut Parser>; 16], _> ([&mut try(help_parser), &mut try(import_parser), &mut try(timer_parser), @@ -353,7 +365,7 @@ pub fn command(s: &str) -> Result { &mut try(sync_parser), &mut try(transact_parser)])) .parse(s) - .unwrap_or((Err(cli::ErrorKind::CommandParse(format!("Invalid command {:?}", s)).into()), "")).0 + .unwrap_or((Err(CliError::CommandParse(format!("Invalid command {:?}", s)).into()), "")).0 } #[cfg(test)] diff --git a/tools/cli/src/mentat_cli/input.rs b/tools/cli/src/mentat_cli/input.rs index 007511ce..9c9f3945 100644 --- a/tools/cli/src/mentat_cli/input.rs +++ b/tools/cli/src/mentat_cli/input.rs @@ -28,7 +28,7 @@ use command_parser::{ command, }; -use errors as cli; +use failure::Error; /// Starting prompt const DEFAULT_PROMPT: &'static str = "mentat=> "; @@ -97,7 +97,7 @@ impl InputReader { /// Reads a single command, item, or statement from `stdin`. /// Returns `More` if further input is required for a complete result. /// In this case, the input received so far is buffered internally. - pub fn read_input(&mut self) -> Result { + pub fn read_input(&mut self) -> Result { let prompt = if self.in_process_cmd.is_some() { MORE_PROMPT } else { DEFAULT_PROMPT }; let prompt = format!("{blue}{prompt}{reset}", blue = color::Fg(::BLUE), diff --git a/tools/cli/src/mentat_cli/lib.rs b/tools/cli/src/mentat_cli/lib.rs index 692257c3..bb637e19 100644 --- a/tools/cli/src/mentat_cli/lib.rs +++ b/tools/cli/src/mentat_cli/lib.rs @@ -10,12 +10,13 @@ #![crate_name = "mentat_cli"] +#[macro_use] extern crate failure_derive; #[macro_use] extern crate log; #[macro_use] extern crate lazy_static; -#[macro_use] extern crate error_chain; extern crate combine; extern crate env_logger; +extern crate failure; extern crate getopts; extern crate linefeed; extern crate rusqlite; @@ -41,7 +42,12 @@ static GREEN: color::Rgb = color::Rgb(0x77, 0xFF, 0x99); pub mod command_parser; pub mod input; pub mod repl; -pub mod errors; + +#[derive(Debug, Fail)] +pub enum CliError { + #[fail(display = "{}", _0)] + CommandParse(String), +} pub fn run() -> i32 { env_logger::init(); diff --git a/tools/cli/src/mentat_cli/repl.rs b/tools/cli/src/mentat_cli/repl.rs index ba74dd2c..b4f66197 100644 --- a/tools/cli/src/mentat_cli/repl.rs +++ b/tools/cli/src/mentat_cli/repl.rs @@ -11,6 +11,11 @@ use std::io::Write; use std::process; +use failure::{ + err_msg, + Error, +}; + use tabwriter::TabWriter; use termion::{ @@ -383,7 +388,7 @@ impl Repl { if self.path.is_empty() || path != self.path { let next = match encryption_key { #[cfg(not(feature = "sqlcipher"))] - Some(_) => bail!(".open_encrypted and .empty_encrypted require the sqlcipher Mentat feature"), + Some(_) => return Err(err_msg(".open_encrypted and .empty_encrypted require the sqlcipher Mentat feature")), #[cfg(feature = "sqlcipher")] Some(k) => { if empty { @@ -467,7 +472,7 @@ impl Repl { output.flush().unwrap(); } - fn print_results(&self, query_output: QueryOutput) -> Result<(), ::errors::Error> { + fn print_results(&self, query_output: QueryOutput) -> Result<(), Error> { let stdout = ::std::io::stdout(); let mut output = TabWriter::new(stdout.lock()); From 31de5be64f6eef35551215835bb035d28fe35a51 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Tue, 5 Jun 2018 21:23:59 -0400 Subject: [PATCH 09/11] Convert db/ to failure. --- core/Cargo.toml | 1 + core/src/cache.rs | 7 +- core/src/lib.rs | 1 + db/Cargo.toml | 3 +- db/src/bootstrap.rs | 23 ++-- db/src/cache.rs | 23 ++-- db/src/db.rs | 92 ++++++-------- db/src/errors.rs | 246 +++++++++++++++++++++++------------- db/src/internal_types.rs | 16 +-- db/src/lib.rs | 17 +-- db/src/metadata.rs | 37 +++--- db/src/schema.rs | 56 ++++---- db/src/tx.rs | 44 +++---- db/src/upsert_resolution.rs | 22 ++-- 14 files changed, 326 insertions(+), 262 deletions(-) diff --git a/core/Cargo.toml b/core/Cargo.toml index 7b75353e..4ff68bc9 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -6,6 +6,7 @@ workspace = ".." [dependencies] chrono = { version = "0.4", features = ["serde"] } enum-set = { git = "https://github.com/rnewman/enum-set" } +failure = "0.1.1" indexmap = "1" lazy_static = "0.2" num = "0.1" diff --git a/core/src/cache.rs b/core/src/cache.rs index 32eda719..07e8cea1 100644 --- a/core/src/cache.rs +++ b/core/src/cache.rs @@ -10,6 +10,8 @@ /// Cache traits. +use failure; + use std::collections::{ BTreeSet, }; @@ -33,8 +35,7 @@ pub trait CachedAttributes { fn get_entids_for_value(&self, attribute: Entid, value: &TypedValue) -> Option<&BTreeSet>; } -pub trait UpdateableCache { - type Error; - fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<(), Self::Error> +pub trait UpdateableCache { + fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<(), Error> where I: Iterator; } diff --git a/core/src/lib.rs b/core/src/lib.rs index 5ec155f4..af9c71fe 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -10,6 +10,7 @@ extern crate chrono; extern crate enum_set; +extern crate failure; extern crate indexmap; extern crate ordered_float; extern crate uuid; diff --git a/db/Cargo.toml b/db/Cargo.toml index 6a9a166b..a356fb2c 100644 --- a/db/Cargo.toml +++ b/db/Cargo.toml @@ -8,7 +8,8 @@ default = [] sqlcipher = ["rusqlite/sqlcipher"] [dependencies] -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } +failure = "0.1.1" +failure_derive = "0.1.1" indexmap = "1" itertools = "0.7" lazy_static = "0.2" diff --git a/db/src/bootstrap.rs b/db/src/bootstrap.rs index d133f26f..a5df3578 100644 --- a/db/src/bootstrap.rs +++ b/db/src/bootstrap.rs @@ -11,7 +11,10 @@ #![allow(dead_code)] use edn; -use errors::{ErrorKind, Result}; +use errors::{ + DbError, + Result, +}; use edn::types::Value; use edn::symbols; use entids; @@ -156,7 +159,7 @@ lazy_static! { :db/cardinality :db.cardinality/many}}"#; edn::parse::value(s) .map(|v| v.without_spans()) - .map_err(|_| ErrorKind::BadBootstrapDefinition("Unable to parse V1_SYMBOLIC_SCHEMA".into())) + .map_err(|_| DbError::BadBootstrapDefinition("Unable to parse V1_SYMBOLIC_SCHEMA".into())) .unwrap() }; } @@ -207,14 +210,14 @@ fn symbolic_schema_to_triples(ident_map: &IdentMap, symbolic_schema: &Value) -> for (ident, mp) in m { let ident = match ident { &Value::Keyword(ref ident) => ident, - _ => bail!(ErrorKind::BadBootstrapDefinition(format!("Expected namespaced keyword for ident but got '{:?}'", ident))), + _ => bail!(DbError::BadBootstrapDefinition(format!("Expected namespaced keyword for ident but got '{:?}'", ident))), }; match *mp { Value::Map(ref mpp) => { for (attr, value) in mpp { let attr = match attr { &Value::Keyword(ref attr) => attr, - _ => bail!(ErrorKind::BadBootstrapDefinition(format!("Expected namespaced keyword for attr but got '{:?}'", attr))), + _ => bail!(DbError::BadBootstrapDefinition(format!("Expected namespaced keyword for attr but got '{:?}'", attr))), }; // We have symbolic idents but the transactor handles entids. Ad-hoc @@ -229,20 +232,20 @@ fn symbolic_schema_to_triples(ident_map: &IdentMap, symbolic_schema: &Value) -> Some(TypedValue::Keyword(ref k)) => { ident_map.get(k) .map(|entid| TypedValue::Ref(*entid)) - .ok_or(ErrorKind::UnrecognizedIdent(k.to_string()))? + .ok_or(DbError::UnrecognizedIdent(k.to_string()))? }, Some(v) => v, - _ => bail!(ErrorKind::BadBootstrapDefinition(format!("Expected Mentat typed value for value but got '{:?}'", value))) + _ => bail!(DbError::BadBootstrapDefinition(format!("Expected Mentat typed value for value but got '{:?}'", value))) }; triples.push((ident.clone(), attr.clone(), typed_value)); } }, - _ => bail!(ErrorKind::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into())) + _ => bail!(DbError::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into())) } } }, - _ => bail!(ErrorKind::BadBootstrapDefinition("Expected {...}".into())) + _ => bail!(DbError::BadBootstrapDefinition("Expected {...}".into())) } Ok(triples) } @@ -263,11 +266,11 @@ fn symbolic_schema_to_assertions(symbolic_schema: &Value) -> Result> value.clone()])); } }, - _ => bail!(ErrorKind::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into())) + _ => bail!(DbError::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into())) } } }, - _ => bail!(ErrorKind::BadBootstrapDefinition("Expected {...}".into())) + _ => bail!(DbError::BadBootstrapDefinition("Expected {...}".into())) } Ok(assertions) } diff --git a/db/src/cache.rs b/db/src/cache.rs index dad957bd..52b06ef5 100644 --- a/db/src/cache.rs +++ b/db/src/cache.rs @@ -106,7 +106,7 @@ use db::{ }; use errors::{ - ErrorKind, + DbError, Result, }; @@ -1155,15 +1155,14 @@ impl CachedAttributes for AttributeCaches { } impl UpdateableCache for AttributeCaches { - type Error = ::errors::Error; - fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> ::std::result::Result<(), Self::Error> + fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()> where I: Iterator { self.update_with_fallback(None, schema, retractions, assertions) } } impl AttributeCaches { - fn update_with_fallback(&mut self, fallback: Option<&AttributeCaches>, schema: &Schema, retractions: I, assertions: I) -> ::std::result::Result<(), ::errors::Error> + fn update_with_fallback(&mut self, fallback: Option<&AttributeCaches>, schema: &Schema, retractions: I, assertions: I) -> Result<()> where I: Iterator { let r_aevs = retractions.peekable(); self.accumulate_into_cache(fallback, schema, r_aevs, AccumulationBehavior::Remove)?; @@ -1237,7 +1236,7 @@ impl SQLiteAttributeCache { let a = attribute.into(); // The attribute must exist! - let _ = schema.attribute_for_entid(a).ok_or_else(|| ErrorKind::UnknownAttribute(a))?; + let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?; let caches = self.make_mut(); caches.forward_cached_attributes.insert(a); caches.repopulate(schema, sqlite, a) @@ -1248,7 +1247,7 @@ impl SQLiteAttributeCache { let a = attribute.into(); // The attribute must exist! - let _ = schema.attribute_for_entid(a).ok_or_else(|| ErrorKind::UnknownAttribute(a))?; + let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?; let caches = self.make_mut(); caches.reverse_cached_attributes.insert(a); @@ -1278,8 +1277,7 @@ impl SQLiteAttributeCache { } impl UpdateableCache for SQLiteAttributeCache { - type Error = ::errors::Error; - fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> ::std::result::Result<(), Self::Error> + fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()> where I: Iterator { self.make_mut().update(schema, retractions, assertions) } @@ -1356,7 +1354,7 @@ impl InProgressSQLiteAttributeCache { let a = attribute.into(); // The attribute must exist! - let _ = schema.attribute_for_entid(a).ok_or_else(|| ErrorKind::UnknownAttribute(a))?; + let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?; if self.is_attribute_cached_forward(a) { return Ok(()); @@ -1372,7 +1370,7 @@ impl InProgressSQLiteAttributeCache { let a = attribute.into(); // The attribute must exist! - let _ = schema.attribute_for_entid(a).ok_or_else(|| ErrorKind::UnknownAttribute(a))?; + let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?; if self.is_attribute_cached_reverse(a) { return Ok(()); @@ -1388,7 +1386,7 @@ impl InProgressSQLiteAttributeCache { let a = attribute.into(); // The attribute must exist! - let _ = schema.attribute_for_entid(a).ok_or_else(|| ErrorKind::UnknownAttribute(a))?; + let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?; // TODO: reverse-index unique by default? let reverse_done = self.is_attribute_cached_reverse(a); @@ -1427,8 +1425,7 @@ impl InProgressSQLiteAttributeCache { } impl UpdateableCache for InProgressSQLiteAttributeCache { - type Error = ::errors::Error; - fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> ::std::result::Result<(), Self::Error> + fn update(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()> where I: Iterator { self.overlay.update_with_fallback(Some(&self.inner), schema, retractions, assertions) } diff --git a/db/src/db.rs b/db/src/db.rs index bfde80a9..f675fe17 100644 --- a/db/src/db.rs +++ b/db/src/db.rs @@ -10,6 +10,8 @@ #![allow(dead_code)] +use failure::ResultExt; + use std::borrow::Borrow; use std::collections::HashMap; use std::collections::hash_map::{ @@ -53,7 +55,11 @@ use mentat_core::{ ValueRc, }; -use errors::{ErrorKind, Result, ResultExt}; +use errors::{ + DbError, + Result, + DbSqlErrorKind, +}; use metadata; use schema::{ SchemaBuilding, @@ -251,8 +257,8 @@ lazy_static! { /// documentation](https://www.sqlite.org/pragma.html#pragma_user_version). fn set_user_version(conn: &rusqlite::Connection, version: i32) -> Result<()> { conn.execute(&format!("PRAGMA user_version = {}", version), &[]) - .chain_err(|| "Could not set_user_version") - .map(|_| ()) + .context(DbSqlErrorKind::CouldNotSetVersionPragma)?; + Ok(()) } /// Get the SQLite user version. @@ -260,10 +266,10 @@ fn set_user_version(conn: &rusqlite::Connection, version: i32) -> Result<()> { /// Mentat manages its own SQL schema version using the user version. See the [SQLite /// documentation](https://www.sqlite.org/pragma.html#pragma_user_version). fn get_user_version(conn: &rusqlite::Connection) -> Result { - conn.query_row("PRAGMA user_version", &[], |row| { + let v = conn.query_row("PRAGMA user_version", &[], |row| { row.get(0) - }) - .chain_err(|| "Could not get_user_version") + }).context(DbSqlErrorKind::CouldNotGetVersionPragma)?; + Ok(v) } /// Do just enough work that either `create_current_version` or sync can populate the DB. @@ -303,8 +309,7 @@ pub fn create_current_version(conn: &mut rusqlite::Connection) -> Result { // TODO: validate metadata mutations that aren't schema related, like additional partitions. if let Some(next_schema) = next_schema { if next_schema != db.schema { - // TODO Use custom ErrorKind https://github.com/brson/error-chain/issues/117 - bail!(ErrorKind::NotYetImplemented(format!("Initial bootstrap transaction did not produce expected bootstrap schema"))); + bail!(DbError::NotYetImplemented(format!("Initial bootstrap transaction did not produce expected bootstrap schema"))); } } @@ -326,7 +331,7 @@ pub fn ensure_current_version(conn: &mut rusqlite::Connection) -> Result { CURRENT_VERSION => read_db(conn), // TODO: support updating an existing store. - v => bail!(ErrorKind::NotYetImplemented(format!("Opening databases with Mentat version: {}", v))), + v => bail!(DbError::NotYetImplemented(format!("Opening databases with Mentat version: {}", v))), } } @@ -356,7 +361,7 @@ impl TypedSQLValue for TypedValue { let u = Uuid::from_bytes(x.as_slice()); if u.is_err() { // Rather than exposing Uuid's ParseError… - bail!(ErrorKind::BadSQLValuePair(rusqlite::types::Value::Blob(x), + bail!(DbError::BadSQLValuePair(rusqlite::types::Value::Blob(x), value_type_tag)); } Ok(TypedValue::Uuid(u.unwrap())) @@ -364,7 +369,7 @@ impl TypedSQLValue for TypedValue { (13, rusqlite::types::Value::Text(x)) => { to_namespaced_keyword(&x).map(|k| k.into()) }, - (_, value) => bail!(ErrorKind::BadSQLValuePair(value, value_type_tag)), + (_, value) => bail!(DbError::BadSQLValuePair(value, value_type_tag)), } } @@ -447,12 +452,12 @@ fn read_ident_map(conn: &rusqlite::Connection) -> Result { let v = read_materialized_view(conn, "idents")?; v.into_iter().map(|(e, a, typed_value)| { if a != entids::DB_IDENT { - bail!(ErrorKind::NotYetImplemented(format!("bad idents materialized view: expected :db/ident but got {}", a))); + bail!(DbError::NotYetImplemented(format!("bad idents materialized view: expected :db/ident but got {}", a))); } if let TypedValue::Keyword(keyword) = typed_value { Ok((keyword.as_ref().clone(), e)) } else { - bail!(ErrorKind::NotYetImplemented(format!("bad idents materialized view: expected [entid :db/ident keyword] but got [entid :db/ident {:?}]", typed_value))); + bail!(DbError::NotYetImplemented(format!("bad idents materialized view: expected [entid :db/ident keyword] but got [entid :db/ident {:?}]", typed_value))); } }).collect() } @@ -546,9 +551,8 @@ fn search(conn: &rusqlite::Connection) -> Result<()> { t.a0 = d.a"#; let mut stmt = conn.prepare_cached(s)?; - stmt.execute(&[]) - .map(|_c| ()) - .chain_err(|| "Could not search!") + stmt.execute(&[]).context(DbSqlErrorKind::CouldNotSearch)?; + Ok(()) } /// Insert the new transaction into the `transactions` table. @@ -570,9 +574,7 @@ fn insert_transaction(conn: &rusqlite::Connection, tx: Entid) -> Result<()> { WHERE added0 IS 1 AND ((rid IS NULL) OR ((rid IS NOT NULL) AND (v0 IS NOT v)))"#; let mut stmt = conn.prepare_cached(s)?; - stmt.execute(&[&tx]) - .map(|_c| ()) - .chain_err(|| "Could not insert transaction: failed to add datoms not already present")?; + stmt.execute(&[&tx]).context(DbSqlErrorKind::TxInsertFailedToAddMissingDatoms)?; let s = r#" INSERT INTO transactions (e, a, v, tx, added, value_type_tag) @@ -583,9 +585,7 @@ fn insert_transaction(conn: &rusqlite::Connection, tx: Entid) -> Result<()> { (added0 IS 1 AND search_type IS ':db.cardinality/one' AND v0 IS NOT v))"#; let mut stmt = conn.prepare_cached(s)?; - stmt.execute(&[&tx]) - .map(|_c| ()) - .chain_err(|| "Could not insert transaction: failed to retract datoms already present")?; + stmt.execute(&[&tx]).context(DbSqlErrorKind::TxInsertFailedToRetractDatoms)?; Ok(()) } @@ -607,9 +607,7 @@ fn update_datoms(conn: &rusqlite::Connection, tx: Entid) -> Result<()> { DELETE FROM datoms WHERE rowid IN ids"#; let mut stmt = conn.prepare_cached(s)?; - stmt.execute(&[]) - .map(|_c| ()) - .chain_err(|| "Could not update datoms: failed to retract datoms already present")?; + stmt.execute(&[]).context(DbSqlErrorKind::DatomsUpdateFailedToRetract)?; // Insert datoms that were added and not already present. We also must expand our bitfield into // flags. Since Mentat follows Datomic and treats its input as a set, it is okay to transact @@ -632,10 +630,7 @@ fn update_datoms(conn: &rusqlite::Connection, tx: Entid) -> Result<()> { AttributeBitFlags::UniqueValue as u8); let mut stmt = conn.prepare_cached(&s)?; - stmt.execute(&[&tx]) - .map(|_c| ()) - .chain_err(|| "Could not update datoms: failed to add datoms not already present")?; - + stmt.execute(&[&tx]).context(DbSqlErrorKind::DatomsUpdateFailedToAdd)?; Ok(()) } @@ -762,9 +757,7 @@ impl MentatStoring for rusqlite::Connection { for statement in &statements { let mut stmt = self.prepare_cached(statement)?; - stmt.execute(&[]) - .map(|_c| ()) - .chain_err(|| "Failed to create temporary tables")?; + stmt.execute(&[]).context(DbSqlErrorKind::FailedToCreateTempTables)?; } Ok(()) @@ -827,8 +820,9 @@ impl MentatStoring for rusqlite::Connection { // TODO: consider ensuring we inserted the expected number of rows. let mut stmt = self.prepare_cached(s.as_str())?; stmt.execute(¶ms) + .context(DbSqlErrorKind::NonFtsInsertionIntoTempSearchTableFailed) + .map_err(|e| e.into()) .map(|_c| ()) - .chain_err(|| "Could not insert non-fts one statements into temporary search table!") }).collect::>>(); results.map(|_| ()) @@ -886,7 +880,7 @@ impl MentatStoring for rusqlite::Connection { } }, _ => { - bail!("Cannot transact a fulltext assertion with a typed value that is not :db/valueType :db.type/string"); + bail!(DbError::WrongTypeValueForFtsAssertion); }, } @@ -913,9 +907,7 @@ impl MentatStoring for rusqlite::Connection { // TODO: consider ensuring we inserted the expected number of rows. let mut stmt = self.prepare_cached(fts_s.as_str())?; - stmt.execute(&fts_params) - .map(|_c| ()) - .chain_err(|| "Could not insert fts values into fts table!")?; + stmt.execute(&fts_params).context(DbSqlErrorKind::FtsInsertionFailed)?; // Second, insert searches. // `params` reference computed values in `block`. @@ -943,17 +935,14 @@ impl MentatStoring for rusqlite::Connection { // TODO: consider ensuring we inserted the expected number of rows. let mut stmt = self.prepare_cached(s.as_str())?; - stmt.execute(¶ms) + stmt.execute(¶ms).context(DbSqlErrorKind::FtsInsertionIntoTempSearchTableFailed) + .map_err(|e| e.into()) .map(|_c| ()) - .chain_err(|| "Could not insert FTS statements into temporary search table!") }).collect::>>(); // Finally, clean up temporary searchids. let mut stmt = self.prepare_cached("UPDATE fulltext_values SET searchid = NULL WHERE searchid IS NOT NULL")?; - stmt.execute(&[]) - .map(|_c| ()) - .chain_err(|| "Could not drop FTS search ids!")?; - + stmt.execute(&[]).context(DbSqlErrorKind::FtsFailedToDropSearchIds)?; results.map(|_| ()) } @@ -985,7 +974,7 @@ pub fn update_partition_map(conn: &rusqlite::Connection, partition_map: &Partiti let max_vars = conn.limit(Limit::SQLITE_LIMIT_VARIABLE_NUMBER) as usize; let max_partitions = max_vars / values_per_statement; if partition_map.len() > max_partitions { - bail!(ErrorKind::NotYetImplemented(format!("No more than {} partitions are supported", max_partitions))); + bail!(DbError::NotYetImplemented(format!("No more than {} partitions are supported", max_partitions))); } // Like "UPDATE parts SET idx = CASE WHEN part = ? THEN ? WHEN part = ? THEN ? ELSE idx END". @@ -1001,9 +990,8 @@ pub fn update_partition_map(conn: &rusqlite::Connection, partition_map: &Partiti // supported in the Clojure implementation at all, and might not be supported in Mentat soon, // so this is very low priority. let mut stmt = conn.prepare_cached(s.as_str())?; - stmt.execute(¶ms[..]) - .map(|_c| ()) - .chain_err(|| "Could not update partition map") + stmt.execute(¶ms[..]).context(DbSqlErrorKind::FailedToUpdatePartitionMap)?; + Ok(()) } /// Update the metadata materialized views based on the given metadata report. @@ -1062,8 +1050,8 @@ SELECT EXISTS // error message in this case. if unique_value_stmt.execute(&[to_bool_ref(attribute.unique.is_some()), &entid as &ToSql]).is_err() { match attribute.unique { - Some(attribute::Unique::Value) => bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/value", entid))), - Some(attribute::Unique::Identity) => bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/identity", entid))), + Some(attribute::Unique::Value) => bail!(DbError::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/value", entid))), + Some(attribute::Unique::Identity) => bail!(DbError::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.unique/identity", entid))), None => unreachable!(), // This shouldn't happen, even after we support removing :db/unique. } } @@ -1077,7 +1065,7 @@ SELECT EXISTS if !attribute.multival { let mut rows = cardinality_stmt.query(&[&entid as &ToSql])?; if rows.next().is_some() { - bail!(ErrorKind::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.cardinality/one", entid))); + bail!(DbError::SchemaAlterationFailed(format!("Cannot alter schema attribute {} to be :db.cardinality/one", entid))); } } }, @@ -2684,8 +2672,8 @@ mod tests { let report = conn.transact_simple_terms(terms, InternSet::new()); - match report.unwrap_err() { - errors::Error(ErrorKind::SchemaConstraintViolation(errors::SchemaConstraintViolation::TypeDisagreements { conflicting_datoms }), _) => { + match report.unwrap_err().downcast() { + Ok(DbError::SchemaConstraintViolation(errors::SchemaConstraintViolation::TypeDisagreements { conflicting_datoms })) => { let mut map = BTreeMap::default(); map.insert((100, entids::DB_TX_INSTANT, TypedValue::Long(-1)), ValueType::Instant); map.insert((200, entids::DB_IDENT, TypedValue::typed_string("test")), ValueType::Keyword); diff --git a/db/src/errors.rs b/db/src/errors.rs index d198081b..0f172cee 100644 --- a/db/src/errors.rs +++ b/db/src/errors.rs @@ -10,6 +10,13 @@ #![allow(dead_code)] +use failure::{ + Backtrace, + Context, + Error, + Fail, +}; + use std::collections::{ BTreeMap, BTreeSet, @@ -29,6 +36,16 @@ use types::{ ValueType, }; +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + +pub type Result = ::std::result::Result; + +// TODO Error/ErrorKind pair #[derive(Clone, Debug, Eq, PartialEq)] pub enum CardinalityConflict { /// A cardinality one attribute has multiple assertions `[e a v1], [e a v2], ...`. @@ -46,7 +63,8 @@ pub enum CardinalityConflict { }, } -#[derive(Clone, Debug, Eq, PartialEq)] +// TODO Error/ErrorKind pair +#[derive(Clone, Debug, Eq, PartialEq, Fail)] pub enum SchemaConstraintViolation { /// A transaction tried to assert datoms where one tempid upserts to two (or more) distinct /// entids. @@ -125,95 +143,145 @@ impl ::std::fmt::Display for InputError { } } -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; +#[derive(Debug, Fail)] +pub enum DbError { + /// We're just not done yet. Message that the feature is recognized but not yet + /// implemented. + #[fail(display = "not yet implemented: {}", _0)] + NotYetImplemented(String), + + /// We've been given a value that isn't the correct Mentat type. + #[fail(display = "value '{}' is not the expected Mentat value type {:?}", _0, _1)] + BadValuePair(String, ValueType), + + /// We've got corrupt data in the SQL store: a value and value_type_tag don't line up. + /// TODO _1.data_type() + #[fail(display = "bad SQL (value_type_tag, value) pair: ({:?}, {:?})", _0, _1)] + BadSQLValuePair(rusqlite::types::Value, i32), + + // /// The SQLite store user_version isn't recognized. This could be an old version of Mentat + // /// trying to open a newer version SQLite store; or it could be a corrupt file; or ... + // #[fail(display = "bad SQL store user_version: {}", _0)] + // BadSQLiteStoreVersion(i32), + + /// A bootstrap definition couldn't be parsed or installed. This is a programmer error, not + /// a runtime error. + #[fail(display = "bad bootstrap definition: {}", _0)] + BadBootstrapDefinition(String), + + /// A schema assertion couldn't be parsed. + #[fail(display = "bad schema assertion: {}", _0)] + BadSchemaAssertion(String), + + /// An ident->entid mapping failed. + #[fail(display = "no entid found for ident: {}", _0)] + UnrecognizedIdent(String), + + /// An entid->ident mapping failed. + /// We also use this error if you try to transact an entid that we didn't allocate, + /// in part because we blow the stack in error_chain if we define a new enum! + #[fail(display = "unrecognized or no ident found for entid: {}", _0)] + UnrecognizedEntid(Entid), + + #[fail(display = "unknown attribute for entid: {}", _0)] + UnknownAttribute(Entid), + + #[fail(display = "cannot reverse-cache non-unique attribute: {}", _0)] + CannotCacheNonUniqueAttributeInReverse(Entid), + + #[fail(display = "schema alteration failed: {}", _0)] + SchemaAlterationFailed(String), + + /// A transaction tried to violate a constraint of the schema of the Mentat store. + #[fail(display = "schema constraint violation: {}", _0)] + SchemaConstraintViolation(SchemaConstraintViolation), + + /// The transaction was malformed in some way (that was not recognized at parse time; for + /// example, in a way that is schema-dependent). + #[fail(display = "transaction input error: {}", _0)] + InputError(InputError), + + #[fail(display = "Cannot transact a fulltext assertion with a typed value that is not :db/valueType :db.type/string")] + WrongTypeValueForFtsAssertion, +} + +#[derive(Debug)] +pub struct DbSqlError { + inner: Context, +} + +impl Fail for DbSqlError { + fn cause(&self) -> Option<&Fail> { + self.inner.cause() } - foreign_links { - Rusqlite(rusqlite::Error); - } - - errors { - /// We're just not done yet. Message that the feature is recognized but not yet - /// implemented. - NotYetImplemented(t: String) { - description("not yet implemented") - display("not yet implemented: {}", t) - } - - /// We've been given a value that isn't the correct Mentat type. - BadValuePair(value: String, value_type: ValueType) { - description("value is not the expected Mentat value type") - display("value '{}' is not the expected Mentat value type {:?}", value, value_type) - } - - /// We've got corrupt data in the SQL store: a value and value_type_tag don't line up. - BadSQLValuePair(value: rusqlite::types::Value, value_type_tag: i32) { - description("bad SQL (value_type_tag, value) pair") - display("bad SQL (value_type_tag, value) pair: ({}, {:?})", value_type_tag, value.data_type()) - } - - // /// The SQLite store user_version isn't recognized. This could be an old version of Mentat - // /// trying to open a newer version SQLite store; or it could be a corrupt file; or ... - // BadSQLiteStoreVersion(version: i32) { - // description("bad SQL store user_version") - // display("bad SQL store user_version: {}", version) - // } - - /// A bootstrap definition couldn't be parsed or installed. This is a programmer error, not - /// a runtime error. - BadBootstrapDefinition(t: String) { - description("bad bootstrap definition") - display("bad bootstrap definition: {}", t) - } - - /// A schema assertion couldn't be parsed. - BadSchemaAssertion(t: String) { - description("bad schema assertion") - display("bad schema assertion: {}", t) - } - - /// An ident->entid mapping failed. - UnrecognizedIdent(ident: String) { - description("no entid found for ident") - display("no entid found for ident: {}", ident) - } - - /// An entid->ident mapping failed. - /// We also use this error if you try to transact an entid that we didn't allocate, - /// in part because we blow the stack in error_chain if we define a new enum! - UnrecognizedEntid(entid: Entid) { - description("unrecognized or no ident found for entid") - display("unrecognized or no ident found for entid: {}", entid) - } - - UnknownAttribute(attr: Entid) { - description("unknown attribute") - display("unknown attribute for entid: {}", attr) - } - - CannotCacheNonUniqueAttributeInReverse(attr: Entid) { - description("cannot reverse-cache non-unique attribute") - display("cannot reverse-cache non-unique attribute: {}", attr) - } - - SchemaAlterationFailed(t: String) { - description("schema alteration failed") - display("schema alteration failed: {}", t) - } - - /// A transaction tried to violate a constraint of the schema of the Mentat store. - SchemaConstraintViolation(violation: SchemaConstraintViolation) { - description("schema constraint violation") - display("schema constraint violation: {}", violation) - } - - /// The transaction was malformed in some way (that was not recognized at parse time; for - /// example, in a way that is schema-dependent). - InputError(error: InputError) { - description("transaction input error") - display("transaction input error: {}", error) - } + fn backtrace(&self) -> Option<&Backtrace> { + self.inner.backtrace() } } + +impl ::std::fmt::Display for DbSqlError { + fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result { + ::std::fmt::Display::fmt(&self.inner, f) + } +} + +impl DbSqlError { + pub fn kind(&self) -> DbSqlErrorKind { + *self.inner.get_context() + } +} + +impl From for DbSqlError { + fn from(kind: DbSqlErrorKind) -> DbSqlError { + DbSqlError { inner: Context::new(kind) } + } +} + +impl From> for DbSqlError { + fn from(inner: Context) -> DbSqlError { + DbSqlError { inner: inner } + } +} + +#[derive(Copy, Clone, Eq, PartialEq, Debug, Fail)] +pub enum DbSqlErrorKind { + #[fail(display = "Could not set_user_version")] + CouldNotSetVersionPragma, + + #[fail(display = "Could not get_user_version")] + CouldNotGetVersionPragma, + + #[fail(display = "Could not search!")] + CouldNotSearch, + + #[fail(display = "Could not insert transaction: failed to add datoms not already present")] + TxInsertFailedToAddMissingDatoms, + + #[fail(display = "Could not insert transaction: failed to retract datoms already present")] + TxInsertFailedToRetractDatoms, + + #[fail(display = "Could not update datoms: failed to retract datoms already present")] + DatomsUpdateFailedToRetract, + + #[fail(display = "Could not update datoms: failed to add datoms not already present")] + DatomsUpdateFailedToAdd, + + #[fail(display = "Failed to create temporary tables")] + FailedToCreateTempTables, + + #[fail(display = "Could not insert non-fts one statements into temporary search table!")] + NonFtsInsertionIntoTempSearchTableFailed, + + #[fail(display = "Could not insert fts values into fts table!")] + FtsInsertionFailed, + + #[fail(display = "Could not insert FTS statements into temporary search table!")] + FtsInsertionIntoTempSearchTableFailed, + + #[fail(display = "Could not drop FTS search ids!")] + FtsFailedToDropSearchIds, + + #[fail(display = "Could not update partition map")] + FailedToUpdatePartitionMap, +} diff --git a/db/src/internal_types.rs b/db/src/internal_types.rs index 34f73d09..b61add26 100644 --- a/db/src/internal_types.rs +++ b/db/src/internal_types.rs @@ -31,7 +31,7 @@ use edn::{ use errors; use errors::{ - ErrorKind, + DbError, Result, }; use schema::{ @@ -69,7 +69,7 @@ impl TransactableValue for ValueAndSpan { Ok(EntityPlace::Entid(entities::Entid::Ident(v))) } else { // We only allow namespaced idents. - bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)) + bail!(DbError::InputError(errors::InputError::BadEntityPlace)) } }, Text(v) => Ok(EntityPlace::TempId(TempId::External(v))), @@ -86,10 +86,10 @@ impl TransactableValue for ValueAndSpan { EntityPlace::Entid(a) => Ok(EntityPlace::LookupRef(entities::LookupRef { a: entities::AttributePlace::Entid(a), v: v.clone() })), EntityPlace::TempId(_) | EntityPlace::TxFunction(_) | - EntityPlace::LookupRef(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + EntityPlace::LookupRef(_) => bail!(DbError::InputError(errors::InputError::BadEntityPlace)), } }, - _ => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + _ => bail!(DbError::InputError(errors::InputError::BadEntityPlace)), } }, Nil | @@ -102,7 +102,7 @@ impl TransactableValue for ValueAndSpan { NamespacedSymbol(_) | Vector(_) | Set(_) | - Map(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + Map(_) => bail!(DbError::InputError(errors::InputError::BadEntityPlace)), } } @@ -114,7 +114,7 @@ impl TransactableValue for ValueAndSpan { impl TransactableValue for TypedValue { fn into_typed_value(self, _schema: &Schema, value_type: ValueType) -> Result { if self.value_type() != value_type { - bail!(ErrorKind::BadValuePair(format!("{:?}", self), value_type)); + bail!(DbError::BadValuePair(format!("{:?}", self), value_type)); } Ok(self) } @@ -128,7 +128,7 @@ impl TransactableValue for TypedValue { TypedValue::Long(_) | TypedValue::Double(_) | TypedValue::Instant(_) | - TypedValue::Uuid(_) => bail!(ErrorKind::InputError(errors::InputError::BadEntityPlace)), + TypedValue::Uuid(_) => bail!(DbError::InputError(errors::InputError::BadEntityPlace)), } } @@ -199,7 +199,7 @@ pub fn replace_lookup_ref(lookup_map: &AVMap, desired_or: Either lookup_map.get(&*av) .map(|x| lift(*x)).map(Left) // XXX TODO: fix this error kind! - .ok_or_else(|| ErrorKind::UnrecognizedIdent(format!("couldn't lookup [a v]: {:?}", (*av).clone())).into()), + .ok_or_else(|| DbError::UnrecognizedIdent(format!("couldn't lookup [a v]: {:?}", (*av).clone())).into()), } } } diff --git a/db/src/lib.rs b/db/src/lib.rs index 87813828..a2a8088b 100644 --- a/db/src/lib.rs +++ b/db/src/lib.rs @@ -8,10 +8,8 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -// Oh, error_chain. -#![recursion_limit="128"] - -#[macro_use] extern crate error_chain; +extern crate failure; +#[macro_use] extern crate failure_derive; extern crate indexmap; extern crate itertools; #[macro_use] extern crate lazy_static; @@ -31,7 +29,12 @@ use std::iter::repeat; use itertools::Itertools; -pub use errors::{Error, ErrorKind, ResultExt, Result}; +pub use errors::{ + DbError, + Result, + SchemaConstraintViolation, +}; +#[macro_use] pub mod errors; mod add_retract_alter_set; pub mod cache; @@ -39,7 +42,6 @@ pub mod db; mod bootstrap; pub mod debug; pub mod entids; -pub mod errors; pub mod internal_types; // pub because we need them for building entities programmatically. mod metadata; mod schema; @@ -114,8 +116,7 @@ pub fn to_namespaced_keyword(s: &str) -> Result { _ => None, }; - // TODO Use custom ErrorKind https://github.com/brson/error-chain/issues/117 - nsk.ok_or(ErrorKind::NotYetImplemented(format!("InvalidKeyword: {}", s)).into()) + nsk.ok_or(DbError::NotYetImplemented(format!("InvalidKeyword: {}", s)).into()) } /// Prepare an SQL `VALUES` block, like (?, ?, ?), (?, ?, ?). diff --git a/db/src/metadata.rs b/db/src/metadata.rs index 9ac02067..24d678c3 100644 --- a/db/src/metadata.rs +++ b/db/src/metadata.rs @@ -24,6 +24,8 @@ //! //! This module recognizes, validates, applies, and reports on these mutations. +use failure::ResultExt; + use std::collections::{BTreeMap, BTreeSet}; use std::collections::btree_map::Entry; @@ -33,9 +35,8 @@ use add_retract_alter_set::{ use edn::symbols; use entids; use errors::{ - ErrorKind, + DbError, Result, - ResultExt, }; use mentat_core::{ attribute, @@ -131,7 +132,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu builder.component(false); }, v => { - bail!(ErrorKind::BadSchemaAssertion(format!("Attempted to retract :db/isComponent with the wrong value {:?}.", v))); + bail!(DbError::BadSchemaAssertion(format!("Attempted to retract :db/isComponent with the wrong value {:?}.", v))); }, } }, @@ -146,15 +147,15 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu builder.non_unique(); }, v => { - bail!(ErrorKind::BadSchemaAssertion(format!("Attempted to retract :db/unique with the wrong value {}.", v))); + bail!(DbError::BadSchemaAssertion(format!("Attempted to retract :db/unique with the wrong value {}.", v))); }, } }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [:db/retract _ :db/unique :db.unique/_] but got [:db/retract {} :db/unique {:?}]", entid, value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [:db/retract _ :db/unique :db.unique/_] but got [:db/retract {} :db/unique {:?}]", entid, value))) } }, _ => { - bail!(ErrorKind::BadSchemaAssertion(format!("Retracting attribute {} for entity {} not permitted.", attr, entid))); + bail!(DbError::BadSchemaAssertion(format!("Retracting attribute {} for entity {} not permitted.", attr, entid))); }, } } @@ -168,7 +169,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu entids::DB_DOC => { match *value { TypedValue::String(_) => {}, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/doc \"string value\"] but got [... :db/doc {:?}] for entid {} and attribute {}", value, entid, attr))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/doc \"string value\"] but got [... :db/doc {:?}] for entid {} and attribute {}", value, entid, attr))) } }, @@ -182,7 +183,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu TypedValue::Ref(entids::DB_TYPE_REF) => { builder.value_type(ValueType::Ref); }, TypedValue::Ref(entids::DB_TYPE_STRING) => { builder.value_type(ValueType::String); }, TypedValue::Ref(entids::DB_TYPE_UUID) => { builder.value_type(ValueType::Uuid); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/valueType :db.type/*] but got [... :db/valueType {:?}] for entid {} and attribute {}", value, entid, attr))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/valueType :db.type/*] but got [... :db/valueType {:?}] for entid {} and attribute {}", value, entid, attr))) } }, @@ -190,7 +191,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu match *value { TypedValue::Ref(entids::DB_CARDINALITY_MANY) => { builder.multival(true); }, TypedValue::Ref(entids::DB_CARDINALITY_ONE) => { builder.multival(false); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/cardinality :db.cardinality/many|:db.cardinality/one] but got [... :db/cardinality {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/cardinality :db.cardinality/many|:db.cardinality/one] but got [... :db/cardinality {:?}]", value))) } }, @@ -198,40 +199,40 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu match *value { TypedValue::Ref(entids::DB_UNIQUE_VALUE) => { builder.unique(attribute::Unique::Value); }, TypedValue::Ref(entids::DB_UNIQUE_IDENTITY) => { builder.unique(attribute::Unique::Identity); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/unique :db.unique/value|:db.unique/identity] but got [... :db/unique {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/unique :db.unique/value|:db.unique/identity] but got [... :db/unique {:?}]", value))) } }, entids::DB_INDEX => { match *value { TypedValue::Boolean(x) => { builder.index(x); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/index true|false] but got [... :db/index {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/index true|false] but got [... :db/index {:?}]", value))) } }, entids::DB_FULLTEXT => { match *value { TypedValue::Boolean(x) => { builder.fulltext(x); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/fulltext true|false] but got [... :db/fulltext {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/fulltext true|false] but got [... :db/fulltext {:?}]", value))) } }, entids::DB_IS_COMPONENT => { match *value { TypedValue::Boolean(x) => { builder.component(x); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/isComponent true|false] but got [... :db/isComponent {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/isComponent true|false] but got [... :db/isComponent {:?}]", value))) } }, entids::DB_NO_HISTORY => { match *value { TypedValue::Boolean(x) => { builder.no_history(x); }, - _ => bail!(ErrorKind::BadSchemaAssertion(format!("Expected [... :db/noHistory true|false] but got [... :db/noHistory {:?}]", value))) + _ => bail!(DbError::BadSchemaAssertion(format!("Expected [... :db/noHistory true|false] but got [... :db/noHistory {:?}]", value))) } }, _ => { - bail!(ErrorKind::BadSchemaAssertion(format!("Do not recognize attribute {} for entid {}", attr, entid))) + bail!(DbError::BadSchemaAssertion(format!("Do not recognize attribute {} for entid {}", attr, entid))) } } }; @@ -243,8 +244,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu match attribute_map.entry(entid) { Entry::Vacant(entry) => { // Validate once… - builder.validate_install_attribute() - .chain_err(|| ErrorKind::BadSchemaAssertion(format!("Schema alteration for new attribute with entid {} is not valid", entid)))?; + builder.validate_install_attribute().context(DbError::BadSchemaAssertion(format!("Schema alteration for new attribute with entid {} is not valid", entid)))?; // … and twice, now we have the Attribute. let a = builder.build(); @@ -254,8 +254,7 @@ pub fn update_attribute_map_from_entid_triples(attribute_map: &mut Attribu }, Entry::Occupied(mut entry) => { - builder.validate_alter_attribute() - .chain_err(|| ErrorKind::BadSchemaAssertion(format!("Schema alteration for existing attribute with entid {} is not valid", entid)))?; + builder.validate_alter_attribute().context(DbError::BadSchemaAssertion(format!("Schema alteration for existing attribute with entid {} is not valid", entid)))?; let mutations = builder.mutate(entry.get_mut()); attributes_altered.insert(entid, mutations); }, diff --git a/db/src/schema.rs b/db/src/schema.rs index 7f6d1c32..90f32f97 100644 --- a/db/src/schema.rs +++ b/db/src/schema.rs @@ -12,7 +12,10 @@ use db::TypedSQLValue; use edn; -use errors::{ErrorKind, Result}; +use errors::{ + DbError, + Result, +}; use edn::symbols; use mentat_core::{ attribute, @@ -39,19 +42,19 @@ pub trait AttributeValidation { impl AttributeValidation for Attribute { fn validate(&self, ident: F) -> Result<()> where F: Fn() -> String { if self.unique == Some(attribute::Unique::Value) && !self.index { - bail!(ErrorKind::BadSchemaAssertion(format!(":db/unique :db/unique_value without :db/index true for entid: {}", ident()))) + bail!(DbError::BadSchemaAssertion(format!(":db/unique :db/unique_value without :db/index true for entid: {}", ident()))) } if self.unique == Some(attribute::Unique::Identity) && !self.index { - bail!(ErrorKind::BadSchemaAssertion(format!(":db/unique :db/unique_identity without :db/index true for entid: {}", ident()))) + bail!(DbError::BadSchemaAssertion(format!(":db/unique :db/unique_identity without :db/index true for entid: {}", ident()))) } if self.fulltext && self.value_type != ValueType::String { - bail!(ErrorKind::BadSchemaAssertion(format!(":db/fulltext true without :db/valueType :db.type/string for entid: {}", ident()))) + bail!(DbError::BadSchemaAssertion(format!(":db/fulltext true without :db/valueType :db.type/string for entid: {}", ident()))) } if self.fulltext && !self.index { - bail!(ErrorKind::BadSchemaAssertion(format!(":db/fulltext true without :db/index true for entid: {}", ident()))) + bail!(DbError::BadSchemaAssertion(format!(":db/fulltext true without :db/index true for entid: {}", ident()))) } if self.component && self.value_type != ValueType::Ref { - bail!(ErrorKind::BadSchemaAssertion(format!(":db/isComponent true without :db/valueType :db.type/ref for entid: {}", ident()))) + bail!(DbError::BadSchemaAssertion(format!(":db/isComponent true without :db/valueType :db.type/ref for entid: {}", ident()))) } // TODO: consider warning if we have :db/index true for :db/valueType :db.type/string, // since this may be inefficient. More generally, we should try to drive complex @@ -150,17 +153,17 @@ impl AttributeBuilder { pub fn validate_install_attribute(&self) -> Result<()> { if self.value_type.is_none() { - bail!(ErrorKind::BadSchemaAssertion("Schema attribute for new attribute does not set :db/valueType".into())); + bail!(DbError::BadSchemaAssertion("Schema attribute for new attribute does not set :db/valueType".into())); } Ok(()) } pub fn validate_alter_attribute(&self) -> Result<()> { if self.value_type.is_some() { - bail!(ErrorKind::BadSchemaAssertion("Schema alteration must not set :db/valueType".into())); + bail!(DbError::BadSchemaAssertion("Schema alteration must not set :db/valueType".into())); } if self.fulltext.is_some() { - bail!(ErrorKind::BadSchemaAssertion("Schema alteration must not set :db/fulltext".into())); + bail!(DbError::BadSchemaAssertion("Schema alteration must not set :db/fulltext".into())); } Ok(()) } @@ -247,15 +250,15 @@ pub trait SchemaBuilding { impl SchemaBuilding for Schema { fn require_ident(&self, entid: Entid) -> Result<&symbols::Keyword> { - self.get_ident(entid).ok_or(ErrorKind::UnrecognizedEntid(entid).into()) + self.get_ident(entid).ok_or(DbError::UnrecognizedEntid(entid).into()) } fn require_entid(&self, ident: &symbols::Keyword) -> Result { - self.get_entid(&ident).ok_or(ErrorKind::UnrecognizedIdent(ident.to_string()).into()) + self.get_entid(&ident).ok_or(DbError::UnrecognizedIdent(ident.to_string()).into()) } fn require_attribute_for_entid(&self, entid: Entid) -> Result<&Attribute> { - self.attribute_for_entid(entid).ok_or(ErrorKind::UnrecognizedEntid(entid).into()) + self.attribute_for_entid(entid).ok_or(DbError::UnrecognizedEntid(entid).into()) } /// Create a valid `Schema` from the constituent maps. @@ -271,8 +274,8 @@ impl SchemaBuilding for Schema { where U: IntoIterator{ let entid_assertions: Result> = assertions.into_iter().map(|(symbolic_ident, symbolic_attr, value)| { - let ident: i64 = *ident_map.get(&symbolic_ident).ok_or(ErrorKind::UnrecognizedIdent(symbolic_ident.to_string()))?; - let attr: i64 = *ident_map.get(&symbolic_attr).ok_or(ErrorKind::UnrecognizedIdent(symbolic_attr.to_string()))?; + let ident: i64 = *ident_map.get(&symbolic_ident).ok_or(DbError::UnrecognizedIdent(symbolic_ident.to_string()))?; + let attr: i64 = *ident_map.get(&symbolic_attr).ok_or(DbError::UnrecognizedIdent(symbolic_attr.to_string()))?; Ok((ident, attr, value)) }).collect(); @@ -305,7 +308,7 @@ impl SchemaTypeChecking for Schema { // wrapper function. match TypedValue::from_edn_value(&value.clone().without_spans()) { // We don't recognize this EDN at all. Get out! - None => bail!(ErrorKind::BadValuePair(format!("{}", value), value_type)), + None => bail!(DbError::BadValuePair(format!("{}", value), value_type)), Some(typed_value) => match (value_type, typed_value) { // Most types don't coerce at all. (ValueType::Boolean, tv @ TypedValue::Boolean(_)) => Ok(tv), @@ -331,7 +334,7 @@ impl SchemaTypeChecking for Schema { (vt @ ValueType::Instant, _) | (vt @ ValueType::Keyword, _) | (vt @ ValueType::Ref, _) - => bail!(ErrorKind::BadValuePair(format!("{}", value), vt)), + => bail!(DbError::BadValuePair(format!("{}", value), vt)), } } } @@ -343,7 +346,6 @@ impl SchemaTypeChecking for Schema { mod test { use super::*; use self::edn::Keyword; - use errors::Error; fn add_attribute(schema: &mut Schema, ident: Keyword, @@ -435,8 +437,8 @@ mod test { let err = validate_attribute_map(&schema.entid_map, &schema.attribute_map).err(); assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::BadSchemaAssertion(message), _) => { assert_eq!(message, ":db/unique :db/unique_value without :db/index true for entid: :foo/bar"); }, + match err.unwrap().downcast() { + Ok(DbError::BadSchemaAssertion(message)) => { assert_eq!(message, ":db/unique :db/unique_value without :db/index true for entid: :foo/bar"); }, x => panic!("expected Bad Schema Assertion error, got {:?}", x), } } @@ -458,8 +460,8 @@ mod test { let err = validate_attribute_map(&schema.entid_map, &schema.attribute_map).err(); assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::BadSchemaAssertion(message), _) => { assert_eq!(message, ":db/unique :db/unique_identity without :db/index true for entid: :foo/bar"); }, + match err.unwrap().downcast() { + Ok(DbError::BadSchemaAssertion(message)) => { assert_eq!(message, ":db/unique :db/unique_identity without :db/index true for entid: :foo/bar"); }, x => panic!("expected Bad Schema Assertion error, got {:?}", x), } } @@ -481,8 +483,8 @@ mod test { let err = validate_attribute_map(&schema.entid_map, &schema.attribute_map).err(); assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::BadSchemaAssertion(message), _) => { assert_eq!(message, ":db/isComponent true without :db/valueType :db.type/ref for entid: :foo/bar"); }, + match err.unwrap().downcast() { + Ok(DbError::BadSchemaAssertion(message)) => { assert_eq!(message, ":db/isComponent true without :db/valueType :db.type/ref for entid: :foo/bar"); }, x => panic!("expected Bad Schema Assertion error, got {:?}", x), } } @@ -504,8 +506,8 @@ mod test { let err = validate_attribute_map(&schema.entid_map, &schema.attribute_map).err(); assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::BadSchemaAssertion(message), _) => { assert_eq!(message, ":db/fulltext true without :db/index true for entid: :foo/bar"); }, + match err.unwrap().downcast() { + Ok(DbError::BadSchemaAssertion(message)) => { assert_eq!(message, ":db/fulltext true without :db/index true for entid: :foo/bar"); }, x => panic!("expected Bad Schema Assertion error, got {:?}", x), } } @@ -526,8 +528,8 @@ mod test { let err = validate_attribute_map(&schema.entid_map, &schema.attribute_map).err(); assert!(err.is_some()); - match err.unwrap() { - Error(ErrorKind::BadSchemaAssertion(message), _) => { assert_eq!(message, ":db/fulltext true without :db/valueType :db.type/string for entid: :foo/bar"); }, + match err.unwrap().downcast() { + Ok(DbError::BadSchemaAssertion(message)) => { assert_eq!(message, ":db/fulltext true without :db/valueType :db.type/string for entid: :foo/bar"); }, x => panic!("expected Bad Schema Assertion error, got {:?}", x), } } diff --git a/db/src/tx.rs b/db/src/tx.rs index 028e0652..ad7f1746 100644 --- a/db/src/tx.rs +++ b/db/src/tx.rs @@ -71,7 +71,7 @@ use edn::{ use entids; use errors; use errors::{ - ErrorKind, + DbError, Result, }; use internal_types::{ @@ -179,7 +179,7 @@ pub fn remove_db_id(map: &mut entmod::MapNotation) -> R entmod::ValuePlace::Atom(v) => Some(v.into_entity_place()?), entmod::ValuePlace::Vector(_) | entmod::ValuePlace::MapNotation(_) => { - bail!(ErrorKind::InputError(errors::InputError::BadDbId)) + bail!(DbError::InputError(errors::InputError::BadDbId)) }, } } else { @@ -244,7 +244,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } if !conflicting_upserts.is_empty() { - bail!(ErrorKind::SchemaConstraintViolation(errors::SchemaConstraintViolation::ConflictingUpserts { conflicting_upserts })); + bail!(DbError::SchemaConstraintViolation(errors::SchemaConstraintViolation::ConflictingUpserts { conflicting_upserts })); } Ok(tempids) @@ -281,7 +281,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { if self.partition_map.contains_entid(e) { Ok(KnownEntid(e)) } else { - bail!(ErrorKind::UnrecognizedEntid(e)) + bail!(DbError::UnrecognizedEntid(e)) } } @@ -298,7 +298,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { let lr_typed_value: TypedValue = lookup_ref.v.clone().into_typed_value(&self.schema, lr_attribute.value_type)?; if lr_attribute.unique.is_none() { - bail!(ErrorKind::NotYetImplemented(format!("Cannot resolve (lookup-ref {} {:?}) with attribute that is not :db/unique", lr_a, lr_typed_value))) + bail!(DbError::NotYetImplemented(format!("Cannot resolve (lookup-ref {} {:?}) with attribute that is not :db/unique", lr_a, lr_typed_value))) } Ok(self.lookup_refs.intern((lr_a, lr_typed_value))) @@ -336,7 +336,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { entmod::EntityPlace::TxFunction(ref tx_function) => { match tx_function.op.0.as_str() { "transaction-tx" => Ok(Either::Left(self.tx_id)), - unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), + unknown @ _ => bail!(DbError::NotYetImplemented(format!("Unknown transaction function {}", unknown))), } }, } @@ -357,13 +357,13 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { fn entity_v_into_term_e(&mut self, x: entmod::ValuePlace, backward_a: &entmod::Entid) -> Result> { match backward_a.unreversed() { None => { - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for forward attribute"))); + bail!(DbError::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for forward attribute"))); }, Some(forward_a) => { let forward_a = self.entity_a_into_term_a(forward_a)?; let forward_attribute = self.schema.require_attribute_for_entid(forward_a)?; if forward_attribute.value_type != ValueType::Ref { - bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} that is not :db/valueType :db.type/ref", forward_a))) + bail!(DbError::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} that is not :db/valueType :db.type/ref", forward_a))) } match x { @@ -378,7 +378,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { Ok(Either::Left(KnownEntid(entid))) } else { // The given value is expected to be :db.type/ref, so this shouldn't happen. - bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} with value that is not :db.valueType :db.type/ref", forward_a))) + bail!(DbError::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} with value that is not :db.valueType :db.type/ref", forward_a))) } } } @@ -396,15 +396,15 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { entmod::ValuePlace::TxFunction(ref tx_function) => { match tx_function.op.0.as_str() { "transaction-tx" => Ok(Either::Left(KnownEntid(self.tx_id.0))), - unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), + unknown @ _ => bail!(DbError::NotYetImplemented(format!("Unknown transaction function {}", unknown))), } }, entmod::ValuePlace::Vector(_) => - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", forward_a))), + bail!(DbError::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", forward_a))), entmod::ValuePlace::MapNotation(_) => - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", forward_a))), + bail!(DbError::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", forward_a))), } }, } @@ -475,7 +475,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { entmod::ValuePlace::LookupRef(ref lookup_ref) => { if attribute.value_type != ValueType::Ref { - bail!(ErrorKind::NotYetImplemented(format!("Cannot resolve value lookup ref for attribute {} that is not :db/valueType :db.type/ref", a))) + bail!(DbError::NotYetImplemented(format!("Cannot resolve value lookup ref for attribute {} that is not :db/valueType :db.type/ref", a))) } Either::Right(LookupRefOrTempId::LookupRef(in_process.intern_lookup_ref(lookup_ref)?)) @@ -484,7 +484,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { entmod::ValuePlace::TxFunction(ref tx_function) => { let typed_value = match tx_function.op.0.as_str() { "transaction-tx" => TypedValue::Ref(self.tx_id), - unknown @ _ => bail!(ErrorKind::NotYetImplemented(format!("Unknown transaction function {}", unknown))), + unknown @ _ => bail!(DbError::NotYetImplemented(format!("Unknown transaction function {}", unknown))), }; // Here we do schema-aware typechecking: we assert that the computed @@ -494,7 +494,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // value can be used where a double is expected. See also // `SchemaTypeChecking.to_typed_value(...)`. if attribute.value_type != typed_value.value_type() { - bail!(ErrorKind::NotYetImplemented(format!("Transaction function {} produced value of type {} but expected type {}", + bail!(DbError::NotYetImplemented(format!("Transaction function {} produced value of type {} but expected type {}", tx_function.op.0.as_str(), typed_value.value_type(), attribute.value_type))); } @@ -503,7 +503,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { entmod::ValuePlace::Vector(vs) => { if !attribute.multival { - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value for attribute {} that is not :db.cardinality :db.cardinality/many", a))); + bail!(DbError::NotYetImplemented(format!("Cannot explode vector value for attribute {} that is not :db.cardinality :db.cardinality/many", a))); } for vv in vs { @@ -523,11 +523,11 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { // AddOrRetract, which proliferates types and code, or only handling // nested maps rather than map values, like Datomic does. if op != OpType::Add { - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode nested map value in :db/retract for attribute {}", a))); + bail!(DbError::NotYetImplemented(format!("Cannot explode nested map value in :db/retract for attribute {}", a))); } if attribute.value_type != ValueType::Ref { - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode nested map value for attribute {} that is not :db/valueType :db.type/ref", a))) + bail!(DbError::NotYetImplemented(format!("Cannot explode nested map value for attribute {} that is not :db/valueType :db.type/ref", a))) } // :db/id is optional; if it's not given, we generate a special internal tempid @@ -580,7 +580,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } if dangling { - bail!(ErrorKind::NotYetImplemented(format!("Cannot explode nested map value that would lead to dangling entity for attribute {}", a))); + bail!(DbError::NotYetImplemented(format!("Cannot explode nested map value that would lead to dangling entity for attribute {}", a))); } in_process.entity_e_into_term_v(db_id)? @@ -668,7 +668,7 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { } if !conflicting_upserts.is_empty() { - bail!(ErrorKind::SchemaConstraintViolation(errors::SchemaConstraintViolation::ConflictingUpserts { conflicting_upserts })); + bail!(DbError::SchemaConstraintViolation(errors::SchemaConstraintViolation::ConflictingUpserts { conflicting_upserts })); } debug!("tempids {:?}", tempids); @@ -742,12 +742,12 @@ impl<'conn, 'a, W> Tx<'conn, 'a, W> where W: TransactWatcher { let errors = tx_checking::type_disagreements(&aev_trie); if !errors.is_empty() { - bail!(ErrorKind::SchemaConstraintViolation(errors::SchemaConstraintViolation::TypeDisagreements { conflicting_datoms: errors })); + bail!(DbError::SchemaConstraintViolation(errors::SchemaConstraintViolation::TypeDisagreements { conflicting_datoms: errors })); } let errors = tx_checking::cardinality_conflicts(&aev_trie); if !errors.is_empty() { - bail!(ErrorKind::SchemaConstraintViolation(errors::SchemaConstraintViolation::CardinalityConflicts { conflicts: errors })); + bail!(DbError::SchemaConstraintViolation(errors::SchemaConstraintViolation::CardinalityConflicts { conflicts: errors })); } // Pipeline stage 4: final terms (after rewriting) -> DB insertions. diff --git a/db/src/upsert_resolution.rs b/db/src/upsert_resolution.rs index 5f90821c..bd0b854e 100644 --- a/db/src/upsert_resolution.rs +++ b/db/src/upsert_resolution.rs @@ -21,8 +21,10 @@ use std::collections::{ use indexmap; use petgraph::unionfind; -use errors; -use errors::ErrorKind; +use errors::{ + DbError, + Result, +}; use types::{ AVPair, }; @@ -100,11 +102,11 @@ pub(crate) struct FinalPopulations { impl Generation { /// Split entities into a generation of populations that need to evolve to have their tempids /// resolved or allocated, and a population of inert entities that do not reference tempids. - pub(crate) fn from(terms: I, schema: &Schema) -> errors::Result<(Generation, Population)> where I: IntoIterator { + pub(crate) fn from(terms: I, schema: &Schema) -> Result<(Generation, Population)> where I: IntoIterator { let mut generation = Generation::default(); let mut inert = vec![]; - let is_unique = |a: Entid| -> errors::Result { + let is_unique = |a: Entid| -> Result { let attribute: &Attribute = schema.require_attribute_for_entid(a)?; Ok(attribute.unique == Some(attribute::Unique::Identity)) }; @@ -223,7 +225,7 @@ impl Generation { } /// Evolve potential upserts that haven't resolved into allocations. - pub(crate) fn allocate_unresolved_upserts(&mut self) -> errors::Result<()> { + pub(crate) fn allocate_unresolved_upserts(&mut self) -> Result<()> { let mut upserts_ev = vec![]; ::std::mem::swap(&mut self.upserts_ev, &mut upserts_ev); @@ -236,7 +238,7 @@ impl Generation { /// /// Some of the tempids may be identified, so we also provide a map from tempid to a dense set /// of contiguous integer labels. - pub(crate) fn temp_ids_in_allocations(&self, schema: &Schema) -> errors::Result> { + pub(crate) fn temp_ids_in_allocations(&self, schema: &Schema) -> Result> { assert!(self.upserts_e.is_empty(), "All upserts should have been upserted, resolved, or moved to the allocated population!"); assert!(self.upserts_ev.is_empty(), "All upserts should have been upserted, resolved, or moved to the allocated population!"); @@ -313,7 +315,7 @@ impl Generation { /// After evolution is complete, use the provided allocated entids to segment `self` into /// populations, each with no references to tempids. - pub(crate) fn into_final_populations(self, temp_id_map: &TempIdMap) -> errors::Result { + pub(crate) fn into_final_populations(self, temp_id_map: &TempIdMap) -> Result { assert!(self.upserts_e.is_empty()); assert!(self.upserts_ev.is_empty()); @@ -329,21 +331,21 @@ impl Generation { match (op, temp_id_map.get(&*t1), temp_id_map.get(&*t2)) { (op, Some(&n1), Some(&n2)) => Term::AddOrRetract(op, n1, a, TypedValue::Ref(n2.0)), (OpType::Add, _, _) => unreachable!(), // This is a coding error -- every tempid in a :db/add entity should resolve or be allocated. - (OpType::Retract, _, _) => bail!(ErrorKind::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: one of {}, {}", t1, t2))), + (OpType::Retract, _, _) => bail!(DbError::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: one of {}, {}", t1, t2))), } }, Term::AddOrRetract(op, Right(t), a, Left(v)) => { match (op, temp_id_map.get(&*t)) { (op, Some(&n)) => Term::AddOrRetract(op, n, a, v), (OpType::Add, _) => unreachable!(), // This is a coding error. - (OpType::Retract, _) => bail!(ErrorKind::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: {}", t))), + (OpType::Retract, _) => bail!(DbError::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: {}", t))), } }, Term::AddOrRetract(op, Left(e), a, Right(t)) => { match (op, temp_id_map.get(&*t)) { (op, Some(&n)) => Term::AddOrRetract(op, e, a, TypedValue::Ref(n.0)), (OpType::Add, _) => unreachable!(), // This is a coding error. - (OpType::Retract, _) => bail!(ErrorKind::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: {}", t))), + (OpType::Retract, _) => bail!(DbError::NotYetImplemented(format!("[:db/retract ...] entity referenced tempid that did not upsert: {}", t))), } }, Term::AddOrRetract(_, Left(_), _, Left(_)) => unreachable!(), // This is a coding error -- these should not be in allocations. From 4e46adeba1785f17d957a09da0c4b235adb251e5 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Thu, 7 Jun 2018 14:28:46 -0400 Subject: [PATCH 10/11] Convert tolstoy/ to failure. --- src/entity_builder.rs | 6 +-- tolstoy/Cargo.toml | 4 +- tolstoy/src/errors.rs | 87 ++++++++++++++-------------------------- tolstoy/src/lib.rs | 13 +++--- tolstoy/src/metadata.rs | 4 +- tolstoy/src/syncer.rs | 10 ++--- tolstoy/src/tx_mapper.rs | 6 +-- 7 files changed, 48 insertions(+), 82 deletions(-) diff --git a/src/entity_builder.rs b/src/entity_builder.rs index ffa1f56a..524b32b3 100644 --- a/src/entity_builder.rs +++ b/src/entity_builder.rs @@ -381,9 +381,7 @@ mod testing { extern crate mentat_db; // For matching inside a test. - use mentat_db::ErrorKind::{ - UnrecognizedEntid, - }; + use mentat_db::DbError; use ::{ Conn, @@ -424,7 +422,7 @@ mod testing { let mut in_progress = conn.begin_transaction(&mut sqlite).expect("begun successfully"); // This should fail: unrecognized entid. - if let Err(Error(MentatError::DbError(UnrecognizedEntid(e)), _)) = in_progress.transact_terms(terms, tempids) { + if let Ok(DbError::UnrecognizedEntid(e)) = in_progress.transact_terms(terms, tempids).expect_err("expected transact to fail").downcast() { assert_eq!(e, 999); } else { panic!("Should have rejected the entid."); diff --git a/tolstoy/Cargo.toml b/tolstoy/Cargo.toml index 119078e3..d864ef7d 100644 --- a/tolstoy/Cargo.toml +++ b/tolstoy/Cargo.toml @@ -5,6 +5,8 @@ workspace = ".." authors = ["Grisha Kruglov "] [dependencies] +failure = "0.1.1" +failure_derive = "0.1.1" futures = "0.1" hyper = "0.11" tokio-core = "0.1" @@ -15,8 +17,6 @@ serde_derive = "1.0" lazy_static = "0.2" uuid = { version = "0.5", features = ["v4", "serde"] } -error-chain = { git = "https://github.com/rnewman/error-chain", branch = "rnewman/sync" } - [dependencies.mentat_core] path = "../core" diff --git a/tolstoy/src/errors.rs b/tolstoy/src/errors.rs index de5dfdba..3b0a2e3c 100644 --- a/tolstoy/src/errors.rs +++ b/tolstoy/src/errors.rs @@ -10,63 +10,34 @@ #![allow(dead_code)] -use std; -use hyper; -use rusqlite; -use uuid; -use mentat_db; -use serde_cbor; -use serde_json; +use failure::Error; -error_chain! { - types { - Error, ErrorKind, ResultExt, Result; - } - - foreign_links { - IOError(std::io::Error); - HttpError(hyper::Error); - HyperUriError(hyper::error::UriError); - SqlError(rusqlite::Error); - UuidParseError(uuid::ParseError); - Utf8Error(std::str::Utf8Error); - JsonError(serde_json::Error); - CborError(serde_cbor::error::Error); - } - - links { - DbError(mentat_db::Error, mentat_db::ErrorKind); - } - - errors { - TxIncorrectlyMapped(n: usize) { - description("encountered more than one uuid mapping for tx") - display("expected one, found {} uuid mappings for tx", n) - } - - UnexpectedState(t: String) { - description("encountered unexpected state") - display("encountered unexpected state: {}", t) - } - - NotYetImplemented(t: String) { - description("not yet implemented") - display("not yet implemented: {}", t) - } - - DuplicateMetadata(k: String) { - description("encountered more than one metadata value for key") - display("encountered more than one metadata value for key: {}", k) - } - - TxProcessorUnfinished { - description("Tx processor couldn't finish") - display("Tx processor couldn't finish") - } - - BadServerResponse(s: String) { - description("Received bad response from the server") - display("Received bad response from the server: {}", s) - } - } +#[macro_export] +macro_rules! bail { + ($e:expr) => ( + return Err($e.into()); + ) +} + +pub type Result = ::std::result::Result; + +#[derive(Debug, Fail)] +pub enum TolstoyError { + #[fail(display = "Received bad response from the server: {}", _0)] + BadServerResponse(String), + + #[fail(display = "encountered more than one metadata value for key: {}", _0)] + DuplicateMetadata(String), + + #[fail(display = "transaction processor didn't say it was done")] + TxProcessorUnfinished, + + #[fail(display = "expected one, found {} uuid mappings for tx", _0)] + TxIncorrectlyMapped(usize), + + #[fail(display = "encountered unexpected state: {}", _0)] + UnexpectedState(String), + + #[fail(display = "not yet implemented: {}", _0)] + NotYetImplemented(String), } diff --git a/tolstoy/src/lib.rs b/tolstoy/src/lib.rs index 51306f2b..d4293c64 100644 --- a/tolstoy/src/lib.rs +++ b/tolstoy/src/lib.rs @@ -8,11 +8,9 @@ // CONDITIONS OF ANY KIND, either express or implied. See the License for the // specific language governing permissions and limitations under the License. -// For error_chain: -#![recursion_limit="128"] - +extern crate failure; #[macro_use] -extern crate error_chain; +extern crate failure_derive; #[macro_use] extern crate lazy_static; @@ -33,16 +31,15 @@ extern crate mentat_core; extern crate rusqlite; extern crate uuid; +#[macro_use] +pub mod errors; pub mod schema; pub mod metadata; pub mod tx_processor; -pub mod errors; pub mod syncer; pub mod tx_mapper; pub use syncer::Syncer; pub use errors::{ - Error, - ErrorKind, + TolstoyError, Result, - ResultExt, }; diff --git a/tolstoy/src/metadata.rs b/tolstoy/src/metadata.rs index b8196964..8c9b6579 100644 --- a/tolstoy/src/metadata.rs +++ b/tolstoy/src/metadata.rs @@ -15,7 +15,7 @@ use uuid::Uuid; use schema; use errors::{ - ErrorKind, + TolstoyError, Result, }; @@ -42,7 +42,7 @@ impl HeadTrackable for SyncMetadataClient { let updated = tx.execute("UPDATE tolstoy_metadata SET value = ? WHERE key = ?", &[&uuid_bytes, &schema::REMOTE_HEAD_KEY])?; if updated != 1 { - bail!(ErrorKind::DuplicateMetadata(schema::REMOTE_HEAD_KEY.into())); + bail!(TolstoyError::DuplicateMetadata(schema::REMOTE_HEAD_KEY.into())); } Ok(()) } diff --git a/tolstoy/src/syncer.rs b/tolstoy/src/syncer.rs index c4a77515..7abc296d 100644 --- a/tolstoy/src/syncer.rs +++ b/tolstoy/src/syncer.rs @@ -31,7 +31,7 @@ use metadata::HeadTrackable; use schema::ensure_current_version; use errors::{ - ErrorKind, + TolstoyError, Result, }; @@ -179,7 +179,7 @@ impl Syncer { let mut uploader = UploadingTxReceiver::new(remote_client, remote_head); Processor::process(db_tx, from_tx, &mut uploader)?; if !uploader.is_done { - bail!(ErrorKind::TxProcessorUnfinished); + bail!(TolstoyError::TxProcessorUnfinished); } // Last tx uuid uploaded by the tx receiver. // It's going to be our new head. @@ -222,7 +222,7 @@ impl Syncer { // without walking the table at all, and use the tx index. Processor::process(&db_tx, None, &mut inquiring_tx_receiver)?; if !inquiring_tx_receiver.is_done { - bail!(ErrorKind::TxProcessorUnfinished); + bail!(TolstoyError::TxProcessorUnfinished); } let have_local_changes = match inquiring_tx_receiver.last_tx { Some(tx) => { @@ -257,7 +257,7 @@ impl Syncer { Syncer::upload_ours(&mut db_tx, Some(upload_from_tx), &remote_client, &remote_head)?; } else { d(&format!("Unable to fast-forward the server; missing local tx mapping")); - bail!(ErrorKind::TxIncorrectlyMapped(0)); + bail!(TolstoyError::TxIncorrectlyMapped(0)); } // We diverged from the server. @@ -265,7 +265,7 @@ impl Syncer { } else { d(&format!("server changed since last sync.")); - bail!(ErrorKind::NotYetImplemented( + bail!(TolstoyError::NotYetImplemented( format!("Can't yet sync against changed server. Local head {:?}, remote head {:?}", locally_known_remote_head, remote_head) )); } diff --git a/tolstoy/src/tx_mapper.rs b/tolstoy/src/tx_mapper.rs index a5a84405..1ad8521b 100644 --- a/tolstoy/src/tx_mapper.rs +++ b/tolstoy/src/tx_mapper.rs @@ -14,7 +14,7 @@ use uuid::Uuid; use mentat_core::Entid; use errors::{ - ErrorKind, + TolstoyError, Result, }; @@ -59,7 +59,7 @@ impl TxMapper { if txs.len() == 0 { return Ok(None); } else if txs.len() > 1 { - bail!(ErrorKind::TxIncorrectlyMapped(txs.len())); + bail!(TolstoyError::TxIncorrectlyMapped(txs.len())); } Ok(Some(txs.remove(0)?)) } @@ -79,7 +79,7 @@ impl TxMapper { if uuids.len() == 0 { return Ok(None); } else if uuids.len() > 1 { - bail!(ErrorKind::TxIncorrectlyMapped(uuids.len())); + bail!(TolstoyError::TxIncorrectlyMapped(uuids.len())); } Ok(Some(uuids.remove(0)?)) } From 3760f84da81663981ceb9feb53706e93d0bd3227 Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Wed, 20 Jun 2018 14:39:29 -0700 Subject: [PATCH 11/11] Post: Fix comment referring to error-chain. --- parser-utils/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/parser-utils/src/lib.rs b/parser-utils/src/lib.rs index ca85bc64..c0408f72 100644 --- a/parser-utils/src/lib.rs +++ b/parser-utils/src/lib.rs @@ -13,8 +13,7 @@ extern crate edn; extern crate itertools; /// A `ValueParseError` is a `combine::primitives::ParseError`-alike that implements the `Debug`, -/// `Display`, and `std::error::Error` traits. In addition, it doesn't capture references, making -/// it possible to store `ValueParseError` instances in local links with the `error-chain` crate. +/// `Display`, and `std::error::Error` traits. In addition, it doesn't capture references. /// /// This is achieved by wrapping slices of type `&'a [edn::Value]` in an owning type that implements /// `Display`; rather than introducing a newtype like `DisplayVec`, we re-use `edn::Value::Vector`.