Skip to content
This repository was archived by the owner on Sep 12, 2018. It is now read-only.

Commit 5fe4f12

Browse files
ncalexanncalexan
committed
Use concrete Mentat error types rather than failure::Error. (#769) r=grisha
In the language of https://github.com/withoutboats/failure/blob/868273409c826755812dbff1b67cc0ac3fa702f7/book/src/error-errorkind.md Mentat is a mid-level API, not an application, and therefore we should prefer our own error types. Writing an initial consumer of Mentat (a Rust logins API targeting Mozilla Lockbox), I have found this to be true: my consumer wants to consume concrete Mentat error types. This doesn't go "all the way" and convert all sub-crates to the Error/ErrorKind, but it does go part of the way.
2 parents 72a9b30 + ae42784 commit 5fe4f12

35 files changed

Lines changed: 447 additions & 405 deletions

File tree

‎core/src/cache.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010

1111
/// Cache traits.
1212
13-
use failure;
14-
1513
use std::collections::{
1614
BTreeSet,
1715
};
@@ -35,7 +33,7 @@ pub trait CachedAttributes {
3533
fn get_entids_for_value(&self, attribute: Entid, value: &TypedValue) -> Option<&BTreeSet<Entid>>;
3634
}
3735

38-
pub trait UpdateableCache<Error=failure::Error> {
39-
fn update<I>(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<(), Error>
36+
pub trait UpdateableCache<E> {
37+
fn update<I>(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<(), E>
4038
where I: Iterator<Item=(Entid, Entid, TypedValue)>;
4139
}

‎db/src/bootstrap.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
use edn;
1414
use errors::{
15-
DbError,
15+
DbErrorKind,
1616
Result,
1717
};
1818
use edn::types::Value;
@@ -159,7 +159,7 @@ lazy_static! {
159159
:db/cardinality :db.cardinality/many}}"#;
160160
edn::parse::value(s)
161161
.map(|v| v.without_spans())
162-
.map_err(|_| DbError::BadBootstrapDefinition("Unable to parse V1_SYMBOLIC_SCHEMA".into()))
162+
.map_err(|_| DbErrorKind::BadBootstrapDefinition("Unable to parse V1_SYMBOLIC_SCHEMA".into()))
163163
.unwrap()
164164
};
165165
}
@@ -210,14 +210,14 @@ fn symbolic_schema_to_triples(ident_map: &IdentMap, symbolic_schema: &Value) ->
210210
for (ident, mp) in m {
211211
let ident = match ident {
212212
&Value::Keyword(ref ident) => ident,
213-
_ => bail!(DbError::BadBootstrapDefinition(format!("Expected namespaced keyword for ident but got '{:?}'", ident))),
213+
_ => bail!(DbErrorKind::BadBootstrapDefinition(format!("Expected namespaced keyword for ident but got '{:?}'", ident))),
214214
};
215215
match *mp {
216216
Value::Map(ref mpp) => {
217217
for (attr, value) in mpp {
218218
let attr = match attr {
219219
&Value::Keyword(ref attr) => attr,
220-
_ => bail!(DbError::BadBootstrapDefinition(format!("Expected namespaced keyword for attr but got '{:?}'", attr))),
220+
_ => bail!(DbErrorKind::BadBootstrapDefinition(format!("Expected namespaced keyword for attr but got '{:?}'", attr))),
221221
};
222222

223223
// We have symbolic idents but the transactor handles entids. Ad-hoc
@@ -232,20 +232,20 @@ fn symbolic_schema_to_triples(ident_map: &IdentMap, symbolic_schema: &Value) ->
232232
Some(TypedValue::Keyword(ref k)) => {
233233
ident_map.get(k)
234234
.map(|entid| TypedValue::Ref(*entid))
235-
.ok_or(DbError::UnrecognizedIdent(k.to_string()))?
235+
.ok_or(DbErrorKind::UnrecognizedIdent(k.to_string()))?
236236
},
237237
Some(v) => v,
238-
_ => bail!(DbError::BadBootstrapDefinition(format!("Expected Mentat typed value for value but got '{:?}'", value)))
238+
_ => bail!(DbErrorKind::BadBootstrapDefinition(format!("Expected Mentat typed value for value but got '{:?}'", value)))
239239
};
240240

241241
triples.push((ident.clone(), attr.clone(), typed_value));
242242
}
243243
},
244-
_ => bail!(DbError::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into()))
244+
_ => bail!(DbErrorKind::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into()))
245245
}
246246
}
247247
},
248-
_ => bail!(DbError::BadBootstrapDefinition("Expected {...}".into()))
248+
_ => bail!(DbErrorKind::BadBootstrapDefinition("Expected {...}".into()))
249249
}
250250
Ok(triples)
251251
}
@@ -266,11 +266,11 @@ fn symbolic_schema_to_assertions(symbolic_schema: &Value) -> Result<Vec<Value>>
266266
value.clone()]));
267267
}
268268
},
269-
_ => bail!(DbError::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into()))
269+
_ => bail!(DbErrorKind::BadBootstrapDefinition("Expected {:db/ident {:db/attr value ...} ...}".into()))
270270
}
271271
}
272272
},
273-
_ => bail!(DbError::BadBootstrapDefinition("Expected {...}".into()))
273+
_ => bail!(DbErrorKind::BadBootstrapDefinition("Expected {...}".into()))
274274
}
275275
Ok(assertions)
276276
}

‎db/src/cache.rs‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ use std::sync::Arc;
7272

7373
use std::iter::Peekable;
7474

75+
use failure::{
76+
ResultExt,
77+
};
78+
7579
use num;
7680

7781
use rusqlite;
@@ -107,6 +111,7 @@ use db::{
107111

108112
use errors::{
109113
DbError,
114+
DbErrorKind,
110115
Result,
111116
};
112117

@@ -895,7 +900,7 @@ impl AttributeCaches {
895900
let table = if is_fulltext { "fulltext_datoms" } else { "datoms" };
896901
let sql = format!("SELECT a, e, v, value_type_tag FROM {} WHERE a = ? ORDER BY a ASC, e ASC", table);
897902
let args: Vec<&rusqlite::types::ToSql> = vec![&attribute];
898-
let mut stmt = sqlite.prepare(&sql)?;
903+
let mut stmt = sqlite.prepare(&sql).context(DbErrorKind::CacheUpdateFailed)?;
899904
let replacing = true;
900905
self.repopulate_from_aevt(schema, &mut stmt, args, replacing)
901906
}
@@ -1154,7 +1159,7 @@ impl CachedAttributes for AttributeCaches {
11541159
}
11551160
}
11561161

1157-
impl UpdateableCache for AttributeCaches {
1162+
impl UpdateableCache<DbError> for AttributeCaches {
11581163
fn update<I>(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()>
11591164
where I: Iterator<Item=(Entid, Entid, TypedValue)> {
11601165
self.update_with_fallback(None, schema, retractions, assertions)
@@ -1236,7 +1241,7 @@ impl SQLiteAttributeCache {
12361241
let a = attribute.into();
12371242

12381243
// The attribute must exist!
1239-
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?;
1244+
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbErrorKind::UnknownAttribute(a))?;
12401245
let caches = self.make_mut();
12411246
caches.forward_cached_attributes.insert(a);
12421247
caches.repopulate(schema, sqlite, a)
@@ -1247,7 +1252,7 @@ impl SQLiteAttributeCache {
12471252
let a = attribute.into();
12481253

12491254
// The attribute must exist!
1250-
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?;
1255+
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbErrorKind::UnknownAttribute(a))?;
12511256

12521257
let caches = self.make_mut();
12531258
caches.reverse_cached_attributes.insert(a);
@@ -1276,7 +1281,7 @@ impl SQLiteAttributeCache {
12761281
}
12771282
}
12781283

1279-
impl UpdateableCache for SQLiteAttributeCache {
1284+
impl UpdateableCache<DbError> for SQLiteAttributeCache {
12801285
fn update<I>(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()>
12811286
where I: Iterator<Item=(Entid, Entid, TypedValue)> {
12821287
self.make_mut().update(schema, retractions, assertions)
@@ -1354,7 +1359,7 @@ impl InProgressSQLiteAttributeCache {
13541359
let a = attribute.into();
13551360

13561361
// The attribute must exist!
1357-
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?;
1362+
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbErrorKind::UnknownAttribute(a))?;
13581363

13591364
if self.is_attribute_cached_forward(a) {
13601365
return Ok(());
@@ -1370,7 +1375,7 @@ impl InProgressSQLiteAttributeCache {
13701375
let a = attribute.into();
13711376

13721377
// The attribute must exist!
1373-
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?;
1378+
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbErrorKind::UnknownAttribute(a))?;
13741379

13751380
if self.is_attribute_cached_reverse(a) {
13761381
return Ok(());
@@ -1386,7 +1391,7 @@ impl InProgressSQLiteAttributeCache {
13861391
let a = attribute.into();
13871392

13881393
// The attribute must exist!
1389-
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbError::UnknownAttribute(a))?;
1394+
let _ = schema.attribute_for_entid(a).ok_or_else(|| DbErrorKind::UnknownAttribute(a))?;
13901395

13911396
// TODO: reverse-index unique by default?
13921397
let reverse_done = self.is_attribute_cached_reverse(a);
@@ -1424,7 +1429,7 @@ impl InProgressSQLiteAttributeCache {
14241429
}
14251430
}
14261431

1427-
impl UpdateableCache for InProgressSQLiteAttributeCache {
1432+
impl UpdateableCache<DbError> for InProgressSQLiteAttributeCache {
14281433
fn update<I>(&mut self, schema: &Schema, retractions: I, assertions: I) -> Result<()>
14291434
where I: Iterator<Item=(Entid, Entid, TypedValue)> {
14301435
self.overlay.update_with_fallback(Some(&self.inner), schema, retractions, assertions)

0 commit comments

Comments
 (0)