[k8s] iotedge-proxy: refactor error handling (Azure#1639)

* Refactor error handling for iotedge-proxy crate

edgelet/iotedge-proxy/src/error.rs

@@ -2,16 +2,13 @@
 
 use std::fmt;
 use std::fmt::Display;
-use std::io::Error as IoError;
 
 use failure::{Backtrace, Context, Fail};
-use hyper::http::uri::InvalidUri;
-use hyper::{header, Body, Error as HyperError, Response, StatusCode};
-use native_tls::Error as NativeTlsError;
+use hyper::{header, Body, Response, StatusCode};
 use serde_json::json;
-use url::ParseError as UrlParseError;
 
 use crate::IntoResponse;
+use url::Url;
 
 #[derive(Debug)]
 pub struct Error {
@@ -20,46 +17,64 @@ pub struct Error {
 
 #[derive(Debug, Fail, PartialEq)]
 pub enum ErrorKind {
-    #[fail(display = "Could not load settings")]
-    LoadSettings,
-
-    #[fail(display = "Unsupported url schema {:?}", _0)]
-    UnsupportedSchema(String),
-
-    #[fail(display = "Could not initialize tokio runtime")]
-    Tokio,
-
-    #[fail(display = "Invalid URL {:?}", _0)]
-    InvalidUrl(String),
-
-    #[fail(display = "Invalid URL {:?}: {}", _0, _1)]
-    InvalidUrlWithReason(String, String),
+    #[fail(display = "The proxy could not start up successfully: {}", _0)]
+    Initialize(InitializeErrorReason),
 
     #[fail(display = "HTTP connection error")]
     Hyper,
 
-    #[fail(display = "A native TLS error occurred")]
-    NativeTls,
+    #[fail(
+        display = "Could not form well-formed URL by joining {:?} with {:?}",
+        _0, _1
+    )]
+    UrlJoin(Url, String),
 
-    #[fail(display = "Parse error url error")]
-    Parse,
-
-    #[fail(display = "Invalid URI to parse")]
-    Uri,
+    #[fail(display = "Invalid URI to parse: {:?}", _0)]
+    Uri(Url),
 
     #[fail(display = "Invalid HTTP header value {:?}", _0)]
     HeaderValue(String),
 
-    #[fail(display = "An IO error occurred")]
-    Io,
-
-    #[fail(display = "An IO error occurred {:?}", _0)]
-    File(String),
-
+    #[cfg(test)]
     #[fail(display = "Error")]
     Generic,
 }
 
+#[derive(Clone, Debug, PartialEq)]
+pub enum InitializeErrorReason {
+    LoadSettings,
+    LoadSettingsUnsupportedSchema(String),
+    InvalidUrl(Url),
+    InvalidUrlWithReason(Url, String),
+    ClientConfig,
+    ClientConfigReadFile(String),
+    Tokio,
+}
+
+impl Display for InitializeErrorReason {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            InitializeErrorReason::LoadSettings => write!(f, "Could not load settings"),
+            InitializeErrorReason::LoadSettingsUnsupportedSchema(x) => {
+                write!(f, "Unsupported URL schema: {}", x)
+            }
+            InitializeErrorReason::InvalidUrl(x) => {
+                write!(f, "Could not resolve address for given URL: {}", x)
+            }
+            InitializeErrorReason::InvalidUrlWithReason(url, reason) => write!(
+                f,
+                "Could not resolve address for given URL: {}. {}",
+                url, reason
+            ),
+            InitializeErrorReason::ClientConfig => write!(f, "Could not load proxy client config"),
+            InitializeErrorReason::ClientConfigReadFile(x) => {
+                write!(f, "Could not read file for proxy client config: {}", x)
+            }
+            InitializeErrorReason::Tokio => write!(f, "Could not initialize tokio runtime"),
+        }
+    }
+}
+
 impl Error {
     pub fn kind(&self) -> &ErrorKind {
         self.inner.get_context()
@@ -123,43 +138,3 @@ impl From<ErrorKind> for Error {
         }
     }
 }
-
-impl From<HyperError> for Error {
-    fn from(error: HyperError) -> Self {
-        Error {
-            inner: error.context(ErrorKind::Hyper),
-        }
-    }
-}
-
-impl From<NativeTlsError> for Error {
-    fn from(error: NativeTlsError) -> Self {
-        Error {
-            inner: error.context(ErrorKind::NativeTls),
-        }
-    }
-}
-
-impl From<UrlParseError> for Error {
-    fn from(error: UrlParseError) -> Self {
-        Error {
-            inner: error.context(ErrorKind::Parse),
-        }
-    }
-}
-
-impl From<InvalidUri> for Error {
-    fn from(error: InvalidUri) -> Self {
-        Error {
-            inner: error.context(ErrorKind::Uri),
-        }
-    }
-}
-
-impl From<IoError> for Error {
-    fn from(error: IoError) -> Self {
-        Error {
-            inner: error.context(ErrorKind::Io),
-        }
-    }
-}

edgelet/iotedge-proxy/src/lib.rs

@@ -18,7 +18,7 @@ mod routine;
 mod settings;
 pub mod signal;
 
-pub use error::{Error, ErrorKind};
+pub use error::{Error, ErrorKind, InitializeErrorReason};
 pub use routine::Routine;
 pub use settings::{ApiSettings, ServiceSettings, Settings};
 

edgelet/iotedge-proxy/src/proxy/client.rs

@@ -1,11 +1,11 @@
 // Copyright (c) Microsoft. All rights reserved.
 
-use failure::ResultExt;
+use failure::{Fail, ResultExt};
 use futures::{Future, IntoFuture};
 use hyper::client::connect::Connect;
 use hyper::client::HttpConnector;
 use hyper::header::HeaderValue;
-use hyper::{header, Body, Client as HyperClient, Request, Response};
+use hyper::{header, Body, Client as HyperClient, Request, Response, Uri};
 use hyper_tls::HttpsConnector;
 use log::info;
 
@@ -57,13 +57,21 @@ where
         &self,
         mut req: Request<Body>,
     ) -> impl Future<Item = Response<Body>, Error = Error> {
+        let path = req
+            .uri()
+            .path_and_query()
+            .map_or("", |p| p.as_str())
+            .to_string();
+
         self.config
             .host()
-            .join(req.uri().path_and_query().map_or("", |p| p.as_str()))
-            .map_err(Error::from)
+            .join(&path)
+            .map_err(|err| {
+                Error::from(err.context(ErrorKind::UrlJoin(self.config.host().clone(), path)))
+            })
             .and_then(|url| {
                 // set a full URL to redirect request to
-                *req.uri_mut() = url.as_str().parse()?;
+                *req.uri_mut() = url.as_str().parse::<Uri>().context(ErrorKind::Uri(url))?;
 
                 // set host value in request header
                 if let Ok(host) = req.uri().host().unwrap_or_default().parse() {
@@ -95,17 +103,21 @@ where
     fn request(&self, req: Request<Body>) -> ResponseFuture {
         let request = format!("{} {} {:?}", req.method(), req.uri(), req.version());
 
-        let fut = self.0.request(req).map_err(Error::from).map(move |res| {
-            let body_length = res
-                .headers()
-                .get(header::CONTENT_LENGTH)
-                .and_then(|length| length.to_str().ok().map(ToString::to_string))
-                .unwrap_or_else(|| "-".to_string());
+        let fut = self
+            .0
+            .request(req)
+            .map_err(|err| Error::from(err.context(ErrorKind::Hyper)))
+            .map(move |res| {
+                let body_length = res
+                    .headers()
+                    .get(header::CONTENT_LENGTH)
+                    .and_then(|length| length.to_str().ok().map(ToString::to_string))
+                    .unwrap_or_else(|| "-".to_string());
 
-            info!("\"{}\" {} {}", request, res.status(), body_length);
+                info!("\"{}\" {} {}", request, res.status(), body_length);
 
-            res
-        });
+                res
+            });
 
         Box::new(fut)
     }
@@ -140,7 +152,7 @@ mod tests {
         let task = client.request(req).and_then(|res| {
             let status = res.status();
             res.into_body()
-                .map_err(Error::from)
+                .map_err(|_| Error::from(ErrorKind::Generic))
                 .concat2()
                 .map(move |body| (status, body.into_bytes()))
         });

edgelet/iotedge-proxy/src/proxy/config.rs

@@ -6,7 +6,7 @@ use failure::ResultExt;
 use native_tls::{Certificate, TlsConnector};
 use url::Url;
 
-use crate::{Error, ErrorKind, ServiceSettings};
+use crate::{Error, ErrorKind, InitializeErrorReason, ServiceSettings};
 
 #[derive(Clone)]
 pub struct Config<T>
@@ -40,22 +40,28 @@ where
 }
 
 pub fn get_config(settings: &ServiceSettings) -> Result<Config<ValueToken>, Error> {
-    let token = fs::read_to_string(settings.token())
-        .context(ErrorKind::File(settings.token().display().to_string()))?;
+    let token = fs::read_to_string(settings.token()).context(ErrorKind::Initialize(
+        InitializeErrorReason::ClientConfigReadFile(settings.token().display().to_string()),
+    ))?;
 
     let mut tls = TlsConnector::builder();
 
     if let Some(path) = settings.certificate() {
-        let file = fs::read_to_string(path).context(ErrorKind::File(path.display().to_string()))?;
-        let cert = Certificate::from_pem(file.as_bytes())?;
+        let file = fs::read_to_string(path).context(ErrorKind::Initialize(
+            InitializeErrorReason::ClientConfigReadFile(path.display().to_string()),
+        ))?;
+
+        let cert = Certificate::from_pem(file.as_bytes())
+            .context(ErrorKind::Initialize(InitializeErrorReason::ClientConfig))?;
 
         tls.add_root_certificate(cert);
     }
 
     Ok(Config::new(
         settings.backend().clone(),
         ValueToken(Some(token)),
-        tls.build()?,
+        tls.build()
+            .context(ErrorKind::Initialize(InitializeErrorReason::ClientConfig))?,
     ))
 }
 
@@ -81,7 +87,7 @@ mod tests {
 
     use crate::proxy::{get_config, TokenSource};
     use crate::tls::CertGenerator;
-    use crate::{ErrorKind, ServiceSettings};
+    use crate::{ErrorKind, InitializeErrorReason, ServiceSettings};
 
     #[test]
     fn it_loads_config_from_filesystem() {
@@ -127,7 +133,12 @@ mod tests {
 
         let err = get_config(&settings).err().unwrap();
 
-        assert_eq!(err.kind(), &ErrorKind::File(token.display().to_string()));
+        assert_eq!(
+            err.kind(),
+            &ErrorKind::Initialize(InitializeErrorReason::ClientConfigReadFile(
+                token.display().to_string()
+            ))
+        );
     }
 
     #[test]
@@ -149,7 +160,12 @@ mod tests {
 
         let err = get_config(&settings).err().unwrap();
 
-        assert_eq!(err.kind(), &ErrorKind::File(cert.display().to_string()));
+        assert_eq!(
+            err.kind(),
+            &ErrorKind::Initialize(InitializeErrorReason::ClientConfigReadFile(
+                cert.display().to_string()
+            ))
+        );
     }
 
     #[test]
@@ -172,7 +188,10 @@ mod tests {
 
         let err = get_config(&settings).err().unwrap();
 
-        assert_eq!(err.kind(), &ErrorKind::NativeTls);
+        assert_eq!(
+            err.kind(),
+            &ErrorKind::Initialize(InitializeErrorReason::ClientConfig)
+        );
     }
 
 }

edgelet/iotedge-proxy/src/proxy/service.rs

@@ -119,7 +119,7 @@ mod tests {
             res.into_body()
                 .concat2()
                 .map(Chunk::into_bytes)
-                .map_err(Error::from)
+                .map_err(|_| Error::from(ErrorKind::Generic))
         });
 
         let mut runtime = Runtime::new().unwrap();
@@ -146,7 +146,7 @@ mod tests {
                             .to_string(),
                     )
                 })
-                .map_err(Error::from)
+                .map_err(|_| Error::from(ErrorKind::Generic))
         });
 
         let mut runtime = Runtime::new().unwrap();
@@ -175,7 +175,7 @@ mod tests {
                             .to_string(),
                     )
                 })
-                .map_err(Error::from)
+                .map_err(|_| Error::from(ErrorKind::Generic))
         });
 
         let mut runtime = Runtime::new().unwrap();