Repository: oli-obk/cargo_metadata (rust)

Issue:
Consider switching to a error type which is `Sync`
Consider switching to a error type which is `Sync` (so that it can be turned into a `Failure` error).

`Fail` requires `Send`+`Sync`+`'static` but error chain errors are not `Sync` (due to the way they handle backtraces if I remember correctly).



Pull request:
Use failure based error instead of error_chain.
Fix #67

Now Error is `'statis + Send + Sync` before it
was not `Sync` which could cause problems.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index f11b495..f36dfd6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,7 +8,7 @@ license = "MIT"
 readme = "README.md"
 
 [dependencies]
-error-chain = {version = "0.12.0", default-features = false}
+failure = "0.1.5"
 serde = "1.0.79"
 serde_derive = "1.0.79"
 serde_json = "1.0.1"
@@ -18,8 +18,7 @@ features = ["serde"]
 version = "0.9"
 
 [features]
-default = ["backtrace"]
-backtrace = ["error-chain/backtrace"]
+default = []
 
 [dev-dependencies]
 clap = "2.26.0"
diff --git a/src/errors.rs b/src/errors.rs
index d9c77d9..a66639a 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -1,19 +1,75 @@
-#![allow(deprecated)] // .cause has been changed to .source
+use std::io;
+use std::str::Utf8Error;
+use std::string::FromUtf8Error;
 
-//! Create the `Error`, `ErrorKind`, `ResultExt`, and `Result` types
+/// Custom result type for `cargo_metadata::Error`
+pub type Result<T> = ::std::result::Result<T, Error>;
 
-error_chain! {
-    errors {
-        /// Error during execution of `cargo metadata`
-        CargoMetadata(stderr: String) {
-            description("execution of `cargo metadata` failed")
-            display("error during execution of `cargo metadata`: {}", stderr)
-        }
+/// Error returned when executing/parsing `cargo metadata` fails.
+///
+/// # Note about Backtraces
+///
+/// This error type does not contain backtraces, but each error variant
+/// comes from _one_ specific place, so it's not really needed for the
+/// inside of this crate. If you need a backtrace down to, but not inside
+/// of, a failed call of `cargo_metadata` you can do one of multiple thinks:
+///
+/// 1. Convert it to a `failure::Error` (possible using the `?` operator),
+///    which is similar to a `Box<::std::error::Error + 'static + Send  + Sync>`.
+/// 2. Have appropriate variants in your own error type. E.g. you could wrap
+///    a `failure::Context<Error>` or add a `failure::Backtrace` field (which
+///    is empty if `RUST_BACKTRACE` is not set, so it's simple to use).
+/// 3. You still can place a failure based error into a `error_chain` if you
+///    really want to. (Either through foreign_links or by making it a field
+///    value of a `ErrorKind` variant).
+///
+#[derive(Debug, Fail)]
+pub enum Error {
+
+    /// Error during execution of `cargo metadata`
+    #[fail(display = "Error during execution of `cargo metadata`: {}", stderr)]
+    CargoMetadata {
+        /// stderr returned by the `cargo metadata` command
+        stderr: String
+    },
+
+    /// IO Error during execution of `cargo metadata`
+    #[fail(display = "{}", 0)]
+    Io(io::Error),
+
+    /// Output of `cargo metadata` was not valid utf8
+    #[fail(display = "Cannot convert the stdout of `cargo metadata`: {}", 0)]
+    Utf8(Utf8Error),
+
+    /// Error output of `cargo metadata` was not valid utf8
+    #[fail(display = "Cannot convert the stderr of `cargo metadata`: {}", 0)]
+    ErrUtf8(FromUtf8Error),
+
+    /// Deserialization error (structure of json did not match expected structure)
+    #[fail(display = "Failed to interpret `cargo metadata`'s json: {}", 0)]
+    Json(::serde_json::Error)
+}
+
+impl From<io::Error> for Error {
+    fn from(v: io::Error) -> Self {
+        Error::Io(v)
     }
-    foreign_links {
-        Io(::std::io::Error) #[doc = "IO Error during execution of `cargo metadata`"];
-        Utf8(::std::str::Utf8Error) #[doc = "Output of `cargo metadata` was not valid utf8"];
-        ErrUtf8(::std::string::FromUtf8Error) #[doc = "Error output of `cargo metadata` was not valid utf8"];
-        Json(::serde_json::Error) #[doc = "Deserialization error (structure of json did not match expected structure)"];
+}
+
+impl From<Utf8Error> for Error {
+    fn from(v: Utf8Error) -> Self {
+        Error::Utf8(v)
     }
 }
+
+impl From<FromUtf8Error> for Error {
+    fn from(v: FromUtf8Error) -> Self {
+        Error::ErrUtf8(v)
+    }
+}
+
+impl From<::serde_json::Error> for Error {
+    fn from(v: ::serde_json::Error) -> Self {
+        Error::Json(v)
+    }
+}
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index 508ebbf..0f454e0 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -157,7 +157,7 @@
 //! ```
 
 #[macro_use]
-extern crate error_chain;
+extern crate failure;
 extern crate semver;
 extern crate serde;
 #[macro_use]
@@ -175,7 +175,7 @@ use semver::Version;
 
 pub use dependency::{Dependency, DependencyKind};
 use diagnostic::Diagnostic;
-pub use errors::{Error, ErrorKind, Result};
+pub use errors::{Error, Result};
 pub use messages::{
     parse_messages, Artifact, ArtifactProfile, BuildScript, CompilerMessage, Message,
 };
@@ -466,7 +466,7 @@ impl MetadataCommand {
         self.features = Some(features);
         self
     }
-    /// Arbitrary command line flags to pass to `cargo`.  These will be added 
+    /// Arbitrary command line flags to pass to `cargo`.  These will be added
     /// to the end of the command line invocation.
     pub fn other_options(&mut self, options: impl AsRef<[String]>) -> &mut MetadataCommand {
         self.other_options = options.as_ref().to_vec();
@@ -500,7 +500,7 @@ impl MetadataCommand {
         cmd.args(&self.other_options);
         let output = cmd.output()?;
         if !output.status.success() {
-            return Err(ErrorKind::CargoMetadata(String::from_utf8(output.stderr)?).into());
+            return Err(Error::CargoMetadata { stderr: String::from_utf8(output.stderr)? });
         }
         let stdout = from_utf8(&output.stdout)?;
         let meta = serde_json::from_str(stdout)?;


Test patch:
diff --git a/tests/selftest.rs b/tests/selftest.rs
index 6dab1ea..28d4bf4 100644
--- a/tests/selftest.rs
+++ b/tests/selftest.rs
@@ -10,7 +10,7 @@ use std::path::{Path, PathBuf};
 
 use semver::Version;
 
-use cargo_metadata::{CargoOpt, Error, ErrorKind, MetadataCommand};
+use cargo_metadata::{CargoOpt, Error, MetadataCommand};
 
 #[derive(Debug, PartialEq, Eq, Deserialize)]
 struct TestPackageMetadata {
@@ -95,8 +95,8 @@ fn builder_interface() {
 #[test]
 fn error1() {
     match MetadataCommand::new().manifest_path("foo").exec() {
-        Err(Error(ErrorKind::CargoMetadata(s), _)) => assert_eq!(
-            s.trim(),
+        Err(Error::CargoMetadata { stderr }) => assert_eq!(
+            stderr.trim(),
             "error: the manifest-path must be a path to a Cargo.toml file"
         ),
         _ => unreachable!(),
@@ -109,8 +109,8 @@ fn error2() {
         .manifest_path("foo/Cargo.toml")
         .exec()
     {
-        Err(Error(ErrorKind::CargoMetadata(s), _)) => assert_eq!(
-            s.trim(),
+        Err(Error::CargoMetadata { stderr }) => assert_eq!(
+            stderr.trim(),
             "error: manifest path `foo/Cargo.toml` does not exist"
         ),
         _ => unreachable!(),
@@ -123,7 +123,7 @@ fn cargo_path() {
         .cargo_path("this does not exist")
         .exec()
     {
-        Err(Error(ErrorKind::Io(e), _)) => assert_eq!(
+        Err(Error::Io(e)) => assert_eq!(
             e.kind(), std::io::ErrorKind::NotFound
         ),
         _ => unreachable!(),