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

Make db/ use DbErrorKind. #770

Merged
merged 7 commits into from
Jun 27, 2018
Merged

Conversation

ncalexan
Copy link
Member

@grigoryk, does this seem like a decent way to achieve #769? It took me a while to figure it out but it's not a ton of work.

Copy link
Contributor

@grigoryk grigoryk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like what the initial conversion should have been. I should try using this stuff in some consumer code to understand it better from the other side of the equation.

src/conn.rs Outdated
@@ -872,8 +872,8 @@ mod tests {
let t = format!("[[:db/add {} :db.schema/attribute \"tempid\"]]", next + 1);

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);
Ok(e @ ::mentat_db::DbError { .. }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooff, that's a mouthful - but I suppose this works, and most consumers won't care about the kind anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and this got better as I iterated further.

}
}

impl From<rusqlite::Error> for DbError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

db/src/errors.rs Outdated
}

#[derive(Clone, PartialEq, Debug, Fail)]
// #[derive(Debug, Fail)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out statement

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out statement

I fixed this.

I elected to keep Tolstoy using `failure::Error`, because Tolstoy
looks rather more like a high-level application (and will continue to
do so for a while) than a production-ready mid- or low-level API.
We're not exposing a uniform API with `mentat::Result` yet, meaning
that early consumers (e.g., the logins work for Mozilla Lockbox) need
to wrap errors from all over the Mentat crate hierarchy.
@ncalexan ncalexan merged commit ae42784 into mozilla:master Jun 27, 2018
@ncalexan ncalexan deleted the failure-errorkinds branch June 27, 2018 22:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants