[Proposal] Error Handling (#625)

* Add new error type

* Use error type in ListDatabases

* Use error type in ListDatabasesResponse

* Add some docs

* Treat Error type like wrapper when wrapping an error

* Add helper macros

* Make Error generic over operation specific errors:

* Move CreateDatabase to the new error

* Move pipeline to use new error type

* Add way to downcast to error types directly

* Add missing error type

* PR feedback

* Get mock framework working

* Get generated services working

* Introduce error code

* Fix broken mock test

* Format code

* Seal the ResultExt trait

Author: rylev

sdk/core/src/error/azure_core_errors.rs

@@ -0,0 +1,41 @@
+use super::{Error, ErrorKind};
+
+impl From<crate::errors::Error> for Error {
+    fn from(err: crate::errors::Error) -> Error {
+        match err {
+            crate::errors::Error::Json(e) => e.into(),
+            crate::errors::Error::Header(e) => Error::new(ErrorKind::DataConversion, e),
+            crate::errors::Error::Parse(e) => Error::new(ErrorKind::DataConversion, e),
+            crate::errors::Error::HeadersNotFound(hs) => Error::with_message(
+                ErrorKind::DataConversion,
+                format!("headers not found: {}", hs.join(", ")),
+            ),
+            crate::errors::Error::HeaderNotFound(h) => Error::with_message(
+                ErrorKind::DataConversion,
+                format!("header not found: {}", h),
+            ),
+            crate::errors::Error::Http(e) => e.into(),
+            crate::errors::Error::Stream(e) => Error::new(ErrorKind::Io, e),
+            crate::errors::Error::GetToken(e) => Error::new(ErrorKind::Credential, e),
+            crate::errors::Error::HttpPrepare(e) => e.into(),
+            crate::errors::Error::Other(e) => Error::new(ErrorKind::Other, e),
+        }
+    }
+}
+
+impl From<crate::errors::HttpError> for Error {
+    fn from(err: crate::errors::HttpError) -> Error {
+        match err {
+            crate::HttpError::StatusCode { status, ref body } => Error::new(
+                ErrorKind::http_response_from_body(status.as_u16(), body),
+                err,
+            ),
+            crate::HttpError::ExecuteRequest(e) => Error::new(ErrorKind::Io, e),
+            crate::HttpError::ReadBytes(e) => Error::new(ErrorKind::Io, e),
+            crate::HttpError::StreamReset(e) => Error::new(ErrorKind::Io, e),
+            crate::HttpError::Utf8(e) => Error::new(ErrorKind::DataConversion, e),
+            crate::HttpError::BuildResponse(e) => Error::new(ErrorKind::DataConversion, e),
+            crate::HttpError::BuildClientRequest(e) => Error::new(ErrorKind::Other, e),
+        }
+    }
+}

sdk/core/src/error/http_error.rs

@@ -0,0 +1,96 @@
+use crate::Response;
+
+/// An unsuccessful HTTP response
+#[derive(Debug)]
+pub struct HttpError {
+    status: u16,
+    error_code: Option<String>,
+    headers: std::collections::HashMap<String, String>,
+    body: String,
+}
+
+impl HttpError {
+    /// Create an error from an http response.
+    ///
+    /// This does not check whether the response was a success and should only be used with unsuccessul responses.
+    pub async fn new(response: Response) -> Self {
+        let status = response.status();
+        let mut error_code = get_error_code_from_header(&response);
+        let headers = response
+            .headers()
+            .into_iter()
+            // TODO: the following will panic if a non-UTF8 header value is sent back
+            // We should not panic but instead handle this gracefully
+            .map(|(n, v)| (n.as_str().to_owned(), v.to_str().unwrap().to_owned()))
+            .collect();
+        let body = response.into_body_string().await;
+        error_code = error_code.or_else(|| get_error_code_from_body(&body));
+        HttpError {
+            status: status.as_u16(),
+            headers,
+            error_code,
+            body,
+        }
+    }
+
+    /// Get a reference to the http error's status.
+    pub fn status(&self) -> u16 {
+        self.status
+    }
+
+    /// Get a reference to the http error's error code.
+    pub fn error_code(&self) -> Option<&str> {
+        self.error_code.as_deref()
+    }
+}
+
+impl std::fmt::Display for HttpError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "HttpError")?;
+        writeln!(f, "\tStatus: {}", self.status)?;
+        writeln!(
+            f,
+            "\tError Code: {}",
+            self.error_code.as_deref().unwrap_or("unknown")
+        )?;
+        // TODO: sanitize body
+        writeln!(f, "\tBody: \"{}\"", self.body)?;
+        writeln!(f, "\tHeaders:")?;
+        // TODO: sanitize headers
+        for (k, v) in &self.headers {
+            writeln!(f, "\t\t{}:{}", k, v)?;
+        }
+
+        Ok(())
+    }
+}
+
+impl std::error::Error for HttpError {}
+
+/// Gets the error code if it's present in the headers
+///
+/// For more info, see [here](https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#handling-errors)
+fn get_error_code_from_header(response: &Response) -> Option<String> {
+    Some(
+        response
+            .headers()
+            .get(http::header::HeaderName::from_static("x-ms-error-code"))?
+            .to_str()
+            .ok()?
+            .to_owned(),
+    )
+}
+
+/// Gets the error code if it's present in the body
+///
+/// For more info, see [here](https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#handling-errors)
+pub(crate) fn get_error_code_from_body(body: &str) -> Option<String> {
+    Some(
+        serde_json::from_str::<serde_json::Value>(body)
+            .ok()?
+            .get("error")?
+            .get("code")?
+            .as_str()?
+            .to_owned(),
+    )
+}

sdk/core/src/error/hyperium_http.rs

@@ -0,0 +1,48 @@
+use super::{Error, ErrorKind};
+
+use http::header::{InvalidHeaderName, InvalidHeaderValue};
+use http::method::InvalidMethod;
+use http::status::InvalidStatusCode;
+use http::uri::{InvalidUri, InvalidUriParts};
+
+impl From<http::Error> for super::Error {
+    fn from(err: http::Error) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidHeaderName> for super::Error {
+    fn from(err: InvalidHeaderName) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidHeaderValue> for super::Error {
+    fn from(err: InvalidHeaderValue) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidMethod> for super::Error {
+    fn from(err: InvalidMethod) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidStatusCode> for super::Error {
+    fn from(err: InvalidStatusCode) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidUri> for super::Error {
+    fn from(err: InvalidUri) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}
+
+impl From<InvalidUriParts> for super::Error {
+    fn from(err: InvalidUriParts) -> super::Error {
+        Error::new(ErrorKind::DataConversion, err)
+    }
+}

sdk/core/src/error/macros.rs

@@ -0,0 +1,119 @@
+/// A convenient way to create a new error using the normal formatting infrastructure
+#[macro_export]
+macro_rules! format_err {
+    ($kind:expr, $msg:literal $(,)?) => {{
+        // Handle $:literal as a special case to make cargo-expanded code more
+        // concise in the common case.
+        $crate::error::Error::new($kind, $msg)
+    }};
+    ($kind:expr, $msg:expr $(,)?) => {{
+        $crate::error::Error::new($kind, $msg)
+    }};
+    ($kind:expr, $msg:expr, $($arg:tt)*) => {{
+        $crate::error::Error::new($kind, format!($msg, $($arg)*))
+    }};
+}
+
+/// Return early with an error if a condition is not satisfied.
+#[macro_export]
+macro_rules! ensure {
+    ($cond:expr, $kind:expr, $msg:literal $(,)?) => {
+        if !$cond {
+            return ::std::result::Result::Err($crate::format_err!($kind, $msg));
+        }
+    };
+    ($cond:expr, $kind:expr, dicate $msg:expr $(,)?) => {
+        if !$cond {
+            return ::std::result::Result::Err($crate::format_err!($kind, $msg));
+        }
+    };
+    ($cond:expr, $kind:expr, dicate $msg:expr, $($arg:tt)*) => {
+        if !$cond {
+            return ::std::result::Result::Err($crate::format_err!($kind, $msg, $($arg)*));
+        }
+    };
+}
+
+/// Return early with an error if two expressions are not equal to each other.
+#[macro_export]
+macro_rules! ensure_eq {
+    ($left:expr, $right:expr, $kind:expr, $msg:literal $(,)?) => {
+        $crate::ensure!($left == $right, $kind, $msg);
+    };
+    ($left:expr, $right:expr, $kind:expr, dicate $msg:expr $(,)?) => {
+        $crate::ensure!($left == $right, $kind, $msg);
+    };
+    ($left:expr, $right:expr, $kind:expr, dicate $msg:expr, $($arg:tt)*) => {
+        $crate::ensure!($left == $right, $kind, $msg, $($arg)*);
+    };
+}
+
+/// Return early with an error if two expressions are equal to each other.
+#[macro_export]
+macro_rules! ensure_ne {
+    ($left:expr, $right:expr, $kind:expr, $msg:literal $(,)?) => {
+        $crate::ensure!($left != $right, $kind, $msg);
+    };
+    ($left:expr, $right:expr, $kind:expr, dicate $msg:expr $(,)?) => {
+        $crate::ensure!($left != $right, $kind, $msg);
+    };
+    ($left:expr, $right:expr, $kind:expr, dicate $msg:expr, $($arg:tt)*) => {
+        $crate::ensure!($left != $right, $kind, $msg, $($arg)*);
+    };
+}
+
+#[cfg(test)]
+mod tests {
+    use super::super::*;
+
+    #[derive(Debug, PartialEq, Copy, Clone)]
+    struct OperationError;
+
+    impl Display for OperationError {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            write!(f, "OperationError")
+        }
+    }
+
+    #[derive(thiserror::Error, Debug)]
+    enum IntermediateError {
+        #[error("second error")]
+        Io(#[from] std::io::Error),
+    }
+
+    #[test]
+    fn ensure_works() {
+        fn test_ensure(predicate: bool) -> Result<()> {
+            ensure!(predicate, ErrorKind::Other, "predicate failed");
+            Ok(())
+        }
+
+        fn test_ensure_eq(item1: &str, item2: &str) -> Result<()> {
+            ensure_eq!(item1, item2, ErrorKind::Other, "predicate failed");
+            Ok(())
+        }
+
+        fn test_ensure_ne(item1: &str, item2: &str) -> Result<()> {
+            ensure_ne!(item1, item2, ErrorKind::Other, "predicate failed");
+            Ok(())
+        }
+
+        let err = test_ensure(false).unwrap_err();
+        assert_eq!(format!("{}", err), "predicate failed");
+        assert_eq!(err.kind(), &ErrorKind::Other);
+
+        assert!(test_ensure(true).is_ok());
+
+        let err = test_ensure_eq("foo", "bar").unwrap_err();
+        assert_eq!(format!("{}", err), "predicate failed");
+        assert_eq!(err.kind(), &ErrorKind::Other);
+
+        assert!(test_ensure_eq("foo", "foo").is_ok());
+
+        let err = test_ensure_ne("foo", "foo").unwrap_err();
+        assert_eq!(format!("{}", err), "predicate failed");
+        assert_eq!(err.kind(), &ErrorKind::Other);
+
+        assert!(test_ensure_ne("foo", "bar").is_ok());
+    }
+}