Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved Error Handling #625

Merged
merged 18 commits into from
Feb 21, 2022
Merged
41 changes: 41 additions & 0 deletions sdk/core/src/error/azure_core_errors.rs
Original file line number Diff line number Diff line change
@@ -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 {
rylev marked this conversation as resolved.
Show resolved Hide resolved
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),
}
}
}
96 changes: 96 additions & 0 deletions sdk/core/src/error/http_error.rs
Original file line number Diff line number Diff line change
@@ -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(),
)
}
48 changes: 48 additions & 0 deletions sdk/core/src/error/hyperium_http.rs
Original file line number Diff line number Diff line change
@@ -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)
}
}
119 changes: 119 additions & 0 deletions sdk/core/src/error/macros.rs
Original file line number Diff line number Diff line change
@@ -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());
}
}
Loading