Replace reliance on From<Context<_>> with an extension trait and constructor

Summary:
Edenapi uses a group of impls like the following as the canonical way to construct ApiError.

```
impl From<Context<ApiErrorKind>> for ApiError {
```

Downstream code would write:

```
use failure::ResultExt;

let stream_trees = config
    .get_or_default("edenapi", "streamtrees")
    .context(ApiErrorKind::BadConfig("edenapi.streamtrees"))?;
```

This relies on the way that ResultExt::context returns a unique type that is different from failure::Error.

This diff introduces a dedicated extension trait to allow the same code in the caller to continue to work even without a failure::Context<T> type, which will be required for dropping our dependency on failure::Fail and failure.

```
pub trait ApiErrorContext<T> {
    fn context(self, kind: ApiErrorKind) -> ApiResult<T>;
}
```

We also introduce a public constructor for ApiError and replace indirect construction that used to look like this:

```
error.context(ApiErrorKind::Curl).into()
```

with this instead:

```
ApiError::new(ApiErrorKind::Curl, error)
```

which is the same number of characters but clearer. The argument order matches [std::io::Error::new](https://doc.rust-lang.org/std/io/struct.Error.html#method.new).

Reviewed By: kulshrax

Differential Revision: D18574668

fbshipit-source-id: 0a56297bb942a26d75a62ca39fc16abeb4486345

eden/scm/edenscmnative/bindings/modules/pyedenapi/src/lib.rs

@@ -106,7 +106,7 @@ fn add_data(
         };
         store
             .add(&delta, &metadata)
-            .map_err(|e| into_exception(py, e.context(ApiErrorKind::Store).into()))?;
+            .map_err(|e| into_exception(py, ApiError::new(ApiErrorKind::Store, e)))?;
     }
     Ok(())
 }
@@ -201,7 +201,7 @@ py_class!(class client |py| {
         }).map_err(|e| into_exception(py, e))?;
 
         for entry in data_iter {
-            store.add_entry(&entry).map_err(|e| into_exception(py, e.context(ApiErrorKind::Store).into()))?;
+            store.add_entry(&entry).map_err(|e| into_exception(py, ApiError::new(ApiErrorKind::Store, e)))?;
         }
 
         downloadstats::create_instance(py, stats)

eden/scm/lib/edenapi/src/config.rs

@@ -7,13 +7,12 @@
 
 use std::path::{Path, PathBuf};
 
-use failure::ResultExt;
 use url::Url;
 
 use auth::AuthConfig;
 use configparser::{config::ConfigSet, hg::ConfigSetHgExt};
 
-use crate::errors::{ApiErrorKind, ApiResult};
+use crate::errors::{ApiErrorContext, ApiErrorKind, ApiResult};
 
 #[derive(Default)]
 pub struct Config {

eden/scm/lib/edenapi/src/curl.rs

@@ -13,7 +13,7 @@ use curl::{
     easy::{Easy2, Handler, HttpVersion, List},
     multi::Multi,
 };
-use failure::{format_err, Fallible, ResultExt};
+use failure::{format_err, Fallible};
 use itertools::Itertools;
 use parking_lot::{Mutex, MutexGuard};
 use serde::{de::DeserializeOwned, Serialize};
@@ -29,7 +29,7 @@ use types::{
 
 use crate::api::EdenApi;
 use crate::config::{ClientCreds, Config};
-use crate::errors::{ApiError, ApiErrorKind, ApiResult};
+use crate::errors::{ApiError, ApiErrorContext, ApiErrorKind, ApiResult};
 use crate::progress::{ProgressFn, ProgressReporter};
 use crate::stats::DownloadStats;
 
@@ -136,7 +136,8 @@ impl EdenApi for EdenApiCurlClient {
         }
 
         if msg != "I_AM_ALIVE" {
-            Err(format_err!("Unexpected response: {:?}", &msg).context(ApiErrorKind::BadResponse))?;
+            Err(format_err!("Unexpected response: {:?}", &msg))
+                .context(ApiErrorKind::BadResponse)?;
         }
 
         Ok(())

eden/scm/lib/edenapi/src/errors.rs

@@ -11,7 +11,7 @@ use std::{
     path::PathBuf,
 };
 
-use failure::{Backtrace, Context, Fail};
+use failure::{Backtrace, Error, Fail};
 use http::StatusCode;
 use thiserror::Error;
 
@@ -19,21 +19,29 @@ pub type ApiResult<T> = Result<T, ApiError>;
 
 #[derive(Debug)]
 pub struct ApiError {
-    context: Context<ApiErrorKind>,
+    cause: Option<Error>,
+    kind: ApiErrorKind,
 }
 
 impl ApiError {
+    pub fn new(kind: ApiErrorKind, cause: impl Into<Error>) -> Self {
+        ApiError {
+            cause: Some(cause.into()),
+            kind,
+        }
+    }
+
     pub fn kind(&self) -> &ApiErrorKind {
-        &*self.context.get_context()
+        &self.kind
     }
 
     pub(crate) fn from_http(code: u32, msg: impl ToString) -> Self {
         let code = match code.try_into() {
             Ok(code) => match StatusCode::from_u16(code) {
                 Ok(code) => code,
-                Err(e) => return e.context(ApiErrorKind::BadResponse).into(),
+                Err(e) => return ApiError::new(ApiErrorKind::BadResponse, e),
             },
-            Err(e) => return e.context(ApiErrorKind::BadResponse).into(),
+            Err(e) => return ApiError::new(ApiErrorKind::BadResponse, e),
         };
         let msg = msg.to_string();
 
@@ -53,20 +61,20 @@ impl ApiError {
 
 impl Fail for ApiError {
     fn cause(&self) -> Option<&dyn Fail> {
-        self.context.cause()
+        self.cause.as_ref().map(Error::as_fail)
     }
 
     fn backtrace(&self) -> Option<&Backtrace> {
-        self.context.backtrace()
+        self.cause.as_ref().map(Error::backtrace)
     }
 }
 
 impl Display for ApiError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if let Some(error) = self.context.cause() {
-            write!(f, "{}: {}", &self.context, &error)
+        if let Some(cause) = &self.cause {
+            write!(f, "{}: {}", self.kind, cause)
         } else {
-            write!(f, "{}", &self.context)
+            write!(f, "{}", self.kind)
         }
     }
 }
@@ -147,27 +155,19 @@ pub enum ApiErrorKind {
     Other(String),
 }
 
-impl From<Context<ApiErrorKind>> for ApiError {
-    fn from(context: Context<ApiErrorKind>) -> Self {
-        Self { context }
+pub trait ApiErrorContext<T> {
+    fn context(self, kind: ApiErrorKind) -> ApiResult<T>;
+}
+
+impl<T, E: Into<Error>> ApiErrorContext<T> for Result<T, E> {
+    fn context(self, kind: ApiErrorKind) -> ApiResult<T> {
+        self.map_err(|e| ApiError::new(kind, e))
     }
 }
 
 impl From<ApiErrorKind> for ApiError {
     fn from(kind: ApiErrorKind) -> Self {
-        Context::new(kind).into()
-    }
-}
-
-impl From<Context<String>> for ApiError {
-    fn from(context: Context<String>) -> Self {
-        context.map(ApiErrorKind::Other).into()
-    }
-}
-
-impl From<Context<&str>> for ApiError {
-    fn from(context: Context<&str>) -> Self {
-        context.map(String::from).into()
+        ApiError { cause: None, kind }
     }
 }
 
@@ -186,27 +186,27 @@ impl From<&str> for ApiError {
 impl From<curl::Error> for ApiError {
     fn from(error: curl::Error) -> Self {
         if error.is_ssl_connect_error() {
-            error.context(ApiErrorKind::Tls).into()
+            ApiError::new(ApiErrorKind::Tls, error)
         } else {
-            error.context(ApiErrorKind::Curl).into()
+            ApiError::new(ApiErrorKind::Curl, error)
         }
     }
 }
 
 impl From<curl::MultiError> for ApiError {
     fn from(error: curl::MultiError) -> Self {
-        error.context(ApiErrorKind::Curl).into()
+        ApiError::new(ApiErrorKind::Curl, error)
     }
 }
 
 impl From<serde_cbor::error::Error> for ApiError {
     fn from(error: serde_cbor::error::Error) -> Self {
-        error.context(ApiErrorKind::Serialization).into()
+        ApiError::new(ApiErrorKind::Serialization, error)
     }
 }
 
 impl From<url::ParseError> for ApiError {
     fn from(error: url::ParseError) -> Self {
-        error.context(ApiErrorKind::Url).into()
+        ApiError::new(ApiErrorKind::Url, error)
     }
 }