HUGE refactor of error types (to use failure)

1 files changed, 193 insertions, 50 deletions

diff --git a/rust/src/errors.rs b/rust/src/errors.rs
index 0b966e93..80535fef 100644
--- a/rust/src/errors.rs
+++ b/rust/src/errors.rs
@@ -1,55 +1,198 @@
-//! Crate-specific Result, Error, and ErrorKind types (using `error_chain`)
-
-error_chain! {
-    foreign_links { Fmt(::std::fmt::Error);
-                    Diesel(::diesel::result::Error);
-                    R2d2(::diesel::r2d2::Error);
-                    Uuid(::uuid::ParseError);
-                    Io(::std::io::Error) #[cfg(unix)];
-                    Serde(::serde_json::Error);
-                    Utf8Decode(::std::string::FromUtf8Error);
-                    StringDecode(::data_encoding::DecodeError);
-    }
-    errors {
-        InvalidFatcatId(id: String) {
-            description("invalid fatcat identifier syntax")
-            display("invalid fatcat identifier (expect 26-char base32 encoded): {}", id)
-        }
-        MalformedExternalId(id: String) {
-            description("external identifier doesn't match required pattern")
-            display("external identifier doesn't match required pattern: {}", id)
-        }
-        MalformedChecksum(hash: String) {
-            description("checksum doesn't match required pattern (hex encoding)")
-            display("checksum doesn't match required pattern (hex encoding): {}", hash)
-        }
-        NotInControlledVocabulary(word: String) {
-            description("word or type not correct for controlled vocabulary")
-            display("word or type not correct for controlled vocabulary")
-        }
-        EditgroupAlreadyAccepted(id: String) {
-            description("editgroup was already accepted")
-            display("attempted to accept or mutate an editgroup which was already accepted: {}", id)
-        }
-        MissingOrMultipleExternalId(message: String) {
-            description("external identifiers missing or multiple specified")
-            display("external identifiers missing or multiple specified; please supply exactly one")
-        }
-        InvalidEntityStateTransform(message: String) {
-            description("Invalid Entity State Transform")
-            display("tried to mutate an entity which was not in an appropriate state: {}", message)
-        }
-        InvalidCredentials(message: String) {
-            description("auth token was missing, expired, revoked, or corrupt")
-            display("auth token was missing, expired, revoked, or corrupt: {}", message)
+//! Crate-specific errors (using `failure`)
+//!
+//! The justification for this complexity is that we need to return correct HTTP error types as
+//! well as helpful error messages in API responses.
+
+// Design goals:
+// - be able to call '?' on random things and collect them in endpoint handlers
+// - be able to 'bail!()' and 'ensure!()' and have those end up as InternalErrors
+// - do conversion into ErrorResponse model in this file, not endpoint handlers, and ErrorReponse
+//   should have good context about the error
+//
+// Plan:
+// - use .map_err() to convert to the correct type
+// - map to
+
+pub use failure::Error;
+use failure::Fail;
+use fatcat_api_spec::models;
+use std::result;
+
+/// A type alias for handling errors throughout this crate
+pub type Result<T> = result::Result<T, Error>;
+
+#[derive(Debug, Fail, Clone)]
+pub enum FatcatError {
+    #[fail(display = "no such {} found: {}", _0, _1)]
+    NotFound(String, String),
+
+    #[fail(
+        display = "invalid fatcat identifier (expect 26-char base32 encoded): {}",
+        _0
+    )]
+    InvalidFatcatId(String),
+
+    #[fail(
+        display = "external identifier doesn't match required pattern for a {}: {}",
+        _0, _1
+    )]
+    MalformedExternalId(String, String),
+
+    #[fail(
+        display = "checksum doesn't match required pattern ({} in hex encoding): {}",
+        _0, _1
+    )]
+    MalformedChecksum(String, String),
+
+    #[fail(display = "not a valid UUID: {}", _0)]
+    MalformedUuid(String),
+
+    #[fail(display = "'{}' is not a an known/acceptable '{}' value", _1, _0)]
+    NotInControlledVocabulary(String, String), // vocab, word
+
+    #[fail(
+        display = "attempted to accept or mutate an editgroup which was already accepted: {}",
+        _0
+    )]
+    EditgroupAlreadyAccepted(String),
+
+    #[fail(
+        display = "external identifiers missing or multiple specified; please supply exactly one: {}",
+        _0
+    )]
+    MissingOrMultipleExternalId(String),
+
+    #[fail(display = "tried to mutate an entity into impossible state: {}", _0)]
+    InvalidEntityStateTransform(String),
+
+    #[fail(
+        display = "auth token was missing, expired, revoked, or corrupt: {}",
+        _0
+    )]
+    InvalidCredentials(String),
+
+    #[fail(display = "editor account doesn't have authorization: {}", _0)]
+    InsufficientPrivileges(String),
+
+    #[fail(
+        display = "broke a constraint or made an otherwise invalid request: {}",
+        _0
+    )]
+    // Utf8Decode, StringDecode, Uuid
+    BadRequest(String),
+
+    #[fail(display = "unexpected database error: {}", _0)]
+    // Diesel constraint that we think is a user error
+    ConstraintViolation(String),
+
+    // TODO: can these hold context instead of Inner?
+    #[fail(display = "unexpected database error: {}", _0)]
+    // other Diesel, R2d2 errors which we don't think are user errors (eg, connection failure)
+    DatabaseError(String),
+
+    #[fail(display = "unexpected internal error: {}", _0)]
+    // Fmt, Io, Serde,
+    InternalError(String),
+
+    // TODO: backwards compat; need to refactor out
+    #[fail(display = "unexpected internal error: {}", _0)]
+    OtherBadRequest(String), // NOTE: this enum is not exhaustive and shouldn't be matched over!
+}
+
+impl Into<models::ErrorResponse> for FatcatError {
+    /// Format an error as an API response (ErrorResponse model, used by all HTTP 4xx and 5xx
+    /// responses)
+    fn into(self) -> models::ErrorResponse {
+        // TODO: something more complex? context?
+        models::ErrorResponse {
+            success: false,
+            error: self.name().unwrap_or("other").to_string(),
+            message: self.to_string(),
         }
-        InsufficientPrivileges(message: String) {
-            description("editor account doesn't have authorization")
-            display("editor account doesn't have authorization: {}", message)
+    }
+}
+
+impl From<diesel::result::Error> for FatcatError {
+    /// The "not found" case should be handled in user code
+    fn from(inner: diesel::result::Error) -> FatcatError {
+        FatcatError::DatabaseError(inner.to_string())
+    }
+}
+
+impl From<std::fmt::Error> for FatcatError {
+    fn from(inner: std::fmt::Error) -> FatcatError {
+        FatcatError::InternalError(inner.to_string())
+    }
+}
+
+impl From<diesel::r2d2::Error> for FatcatError {
+    fn from(inner: diesel::r2d2::Error) -> FatcatError {
+        FatcatError::InternalError(inner.to_string())
+    }
+}
+
+impl From<uuid::ParseError> for FatcatError {
+    fn from(inner: uuid::ParseError) -> FatcatError {
+        FatcatError::MalformedUuid(inner.to_string())
+    }
+}
+
+impl From<serde_json::Error> for FatcatError {
+    fn from(inner: serde_json::Error) -> FatcatError {
+        FatcatError::InternalError(inner.to_string())
+    }
+}
+
+impl From<std::string::FromUtf8Error> for FatcatError {
+    fn from(inner: std::string::FromUtf8Error) -> FatcatError {
+        FatcatError::InternalError(inner.to_string())
+    }
+}
+
+impl From<data_encoding::DecodeError> for FatcatError {
+    fn from(inner: data_encoding::DecodeError) -> FatcatError {
+        FatcatError::InternalError(inner.to_string())
+    }
+}
+
+// A big catchall!
+impl From<failure::Error> for FatcatError {
+    fn from(error: failure::Error) -> FatcatError {
+        // TODO: I think it should be possible to match here? regardless, this is *super* janky
+        if let Some(_) = error.downcast_ref::<FatcatError>() {
+            return error.downcast::<FatcatError>().unwrap();
         }
-        OtherBadRequest(message: String) {
-            description("catch-all error for bad or unallowed requests")
-            display("broke a constraint or made an otherwise invalid request: {}", message)
+        if let Some(_) = error.downcast_ref::<std::fmt::Error>() {
+            return error.downcast::<std::fmt::Error>().unwrap().into();
         }
+        // TODO: more downcast catching?
+        FatcatError::InternalError(error.to_string())
     }
 }
+
+/*
+// Allows adding more context via a String
+impl From<Context<String>> for FatcatError {
+    fn from(inner: Context<String>) -> FatcatError {
+        FatcatError::InternalError(inner.context())
+    }
+}
+
+// Allows adding more context via a &str
+impl From<Context<&'static str>> for FatcatError {
+    fn from(inner: Context<&'static str>) -> FatcatError {
+        FatcatError::InternalError(inner.context().to_string())
+    }
+}
+*/
+
+/* XXX:
+Fmt(::std::fmt::Error);
+Diesel(::diesel::result::Error);
+R2d2(::diesel::r2d2::Error);
+Uuid(::uuid::ParseError);
+Io(::std::io::Error) #[cfg(unix)];
+Serde(::serde_json::Error);
+Utf8Decode(::std::string::FromUtf8Error);
+StringDecode(::data_encoding::DecodeError);
+*/