Refactor errors

This commit is a start of refactoring of errors within the client.
Remove From impl for Reqwest's Response type to Error
as it isn't useful at this stage; a problem with a response is
better handled inside of transport's async send() fn, where an Err could
potentially be returned for a response deemed to be an error.

For now, all HTTP status code responses from Elasticsearch return
an Ok() result. Opened #28 to discuss.

Author: russcam

elasticsearch/src/auth.rs

@@ -7,8 +7,13 @@ pub enum Credentials {
     Basic(String, String),
     /// An access_token to use for Bearer authentication
     Bearer(String),
-    /// Bytes of a PKCS#12 archive and password to use for PKI (Client Certificate) authentication
+    /// Bytes of a DER-formatted PKCS#12 archive and password to use for
+    /// PKI (Client Certificate) authentication.
+    ///
+    /// The archive should contain a leaf certificate and its private key, as well any intermediate
+    /// certificates that allow clients to build a chain to a trusted root. The chain certificates
+    /// should be in order from the leaf certificate towards the root.
     Cert(Vec<u8>, String),
-    /// An id an api_key to use for API key authentication
+    /// An id and api_key to use for API key authentication
     ApiKey(String, String),
 }

elasticsearch/src/error.rs

@@ -1,4 +1,4 @@
-/* Error type based on error type from es-rs:
+/* Error type based on the error type from es-rs:
  *
  * Copyright 2015-2018 Ben Ashford
  *
@@ -13,7 +13,6 @@
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
- *
  */
 use crate::http::transport::BuildError;
 use serde_json;
@@ -33,9 +32,6 @@ pub enum Error {
     /// A general error from this library
     Lib(String),
 
-    /// An error reported in a JSON response from Elasticsearch
-    Server(String),
-
     /// HTTP library error
     Http(reqwest::Error),
 
@@ -76,20 +72,11 @@ impl From<BuildError> for Error {
     }
 }
 
-impl<'a> From<&'a mut reqwest::Response> for Error {
-    fn from(err: &'a mut reqwest::Response) -> Error {
-        // TODO: figure out how to read the response body synchronously
-        //let body = err.text().await?;
-        Error::Server(format!("{} status code received", err.status()))
-    }
-}
-
 impl error::Error for Error {
     fn description(&self) -> &str {
         match *self {
             Error::Build(ref err) => err.description(),
             Error::Lib(ref err) => err,
-            Error::Server(ref err) => err,
             Error::Http(ref err) => err.description(),
             Error::Io(ref err) => err.description(),
             Error::Json(ref err) => err.description(),
@@ -100,7 +87,6 @@ impl error::Error for Error {
         match *self {
             Error::Build(ref err) => Some(err as &dyn error::Error),
             Error::Lib(_) => None,
-            Error::Server(_) => None,
             Error::Http(ref err) => Some(err as &dyn error::Error),
             Error::Io(ref err) => Some(err as &dyn error::Error),
             Error::Json(ref err) => Some(err as &dyn error::Error),
@@ -113,7 +99,6 @@ impl fmt::Display for Error {
         match *self {
             Error::Build(ref err) => fmt::Display::fmt(err, f),
             Error::Lib(ref s) => fmt::Display::fmt(s, f),
-            Error::Server(ref s) => fmt::Display::fmt(s, f),
             Error::Http(ref err) => fmt::Display::fmt(err, f),
             Error::Io(ref err) => fmt::Display::fmt(err, f),
             Error::Json(ref err) => fmt::Display::fmt(err, f),

elasticsearch/src/http/transport.rs

@@ -260,8 +260,17 @@ impl Transport {
             }
         }
 
-        let response = request_builder.send().await?;
-        Ok(Response::new(response))
+        let response = request_builder.send().await;
+        match response {
+            Ok(r) => Ok(Response::new(r)),
+            Err(e) => {
+                if e.is_timeout() {
+                    Err(Error::Lib(format!("Request timed out to {:?}", e.url())))
+                } else {
+                    Err(Error::Http(e))
+                }
+            }
+        }
     }
 }
 
@@ -434,9 +443,20 @@ impl ConnectionPool for CloudConnectionPool {
 
 #[cfg(test)]
 pub mod tests {
-    use crate::http::transport::CloudId;
+    use crate::auth::Credentials;
+    use crate::http::transport::{CloudId, TransportBuilder, SingleNodeConnectionPool};
     use url::Url;
 
+    #[test]
+    fn invalid_cert_credentials() {
+        let conn_pool = SingleNodeConnectionPool::default();
+        let builder = TransportBuilder::new(conn_pool)
+            .auth(Credentials::Cert(b"Nonsense".to_vec(), "".into()));
+
+        let res = builder.build();
+        assert!(res.is_err());
+    }
+
     #[test]
     fn can_parse_cloud_id() {
         let base64 = base64::encode("cloud-endpoint.example$3dadf823f05388497ea684236d918a1a$3f26e1609cf54a0f80137a80de560da4");

elasticsearch/tests/error.rs

@@ -0,0 +1,26 @@
+pub mod common;
+use common::*;
+
+use reqwest::StatusCode;
+use serde_json::{json, Value};
+use elasticsearch::ExplainParts;
+
+/// Responses in the range 400-599 return Response body
+#[tokio::test]
+async fn bad_request_returns_response() -> Result<(), failure::Error> {
+    let client = client::create_default();
+    let response = client
+        .explain(ExplainParts::IndexId("non_existent_index", "id"))
+        .body(json!({}))
+        .send()
+        .await?;
+
+    assert_eq!(response.status_code(), StatusCode::BAD_REQUEST);
+
+    let error = response.read_body::<Value>().await?;
+    assert_eq!(error["status"].as_i64(), Some(400));
+    assert_eq!(error["error"]["type"].as_str(), Some("action_request_validation_exception"));
+    assert_eq!(error["error"]["reason"].as_str(), Some("Validation Failed: 1: query is missing;"));
+
+    Ok(())
+}