redirect and error reform

src/client.rs

@@ -16,7 +16,7 @@ use serde_json;
 use serde_urlencoded;
 
 use ::body::{self, Body};
-use ::redirect::{RedirectPolicy, check_redirect};
+use ::redirect::{self, RedirectPolicy, check_redirect};
 use ::response::Response;
 
 static DEFAULT_USER_AGENT: &'static str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"));
@@ -36,7 +36,7 @@ pub struct Client {
 impl Client {
     /// Constructs a new `Client`.
     pub fn new() -> ::Result<Client> {
-        let mut client = try!(new_hyper_client());
+        let mut client = try_!(new_hyper_client());
         client.set_redirect_policy(::hyper::client::RedirectPolicy::FollowNone);
         Ok(Client {
             inner: Arc::new(ClientRef {
@@ -133,7 +133,7 @@ fn new_hyper_client() -> ::Result<::hyper::Client> {
         ::hyper::client::Pool::with_connector(
             Default::default(),
             ::hyper::net::HttpsConnector::new(
-                try!(NativeTlsClient::new()
+                try_!(NativeTlsClient::new()
                      .map_err(|e| ::hyper::Error::Ssl(Box::new(e)))))
         )
     ))
@@ -198,7 +198,7 @@ impl RequestBuilder {
     ///     .send();
     /// ```
     pub fn form<T: Serialize>(mut self, form: &T) -> RequestBuilder {
-        let body = serde_urlencoded::to_string(form).map_err(::Error::from);
+        let body = serde_urlencoded::to_string(form).map_err(::error::from);
         self.headers.set(ContentType::form_url_encoded());
         self.body = Some(body.map(|b| b.into()));
         self
@@ -242,10 +242,10 @@ impl RequestBuilder {
         }
         let client = self.client;
         let mut method = self.method;
-        let mut url = try!(self.url);
+        let mut url = try_!(self.url);
         let mut headers = self.headers;
         let mut body = match self.body {
-            Some(b) => Some(try!(b)),
+            Some(b) => Some(try_!(b)),
             None => None,
         };
 
@@ -263,7 +263,7 @@ impl RequestBuilder {
                     req = req.body(body);
                 }
 
-                try!(req.send())
+                try_!(req.send(), &url)
             };
 
             let should_redirect = match res.status {
@@ -304,12 +304,20 @@ impl RequestBuilder {
                     Ok(loc) => {
                         headers.set(Referer(url.to_string()));
                         urls.push(url);
-                        if check_redirect(&client.redirect_policy.lock().unwrap(), &loc, &urls)? {
-                            loc
-                        } else {
-                            debug!("redirect_policy disallowed redirection to '{}'", loc);
-
-                            return Ok(::response::new(res, client.auto_ungzip.load(Ordering::Relaxed)));
+                        let action = check_redirect(&client.redirect_policy.lock().unwrap(), &loc, &urls);
+                        match action {
+                            redirect::Action::Follow => loc,
+                            redirect::Action::Stop => {
+                                debug!("redirect_policy disallowed redirection to '{}'", loc);
+
+                                return Ok(::response::new(res, client.auto_ungzip.load(Ordering::Relaxed)));
+                            },
+                            redirect::Action::LoopDetected => {
+                                return Err(::error::loop_detected(res.url.clone()));
+                            },
+                            redirect::Action::TooManyRedirects => {
+                                return Err(::error::too_many_redirects(res.url.clone()));
+                            }
                         }
                     },
                     Err(e) => {

src/error.rs

@@ -1,87 +1,191 @@
 use std::error::Error as StdError;
 use std::fmt;
 
+use ::Url;
+
 /// The Errors that may occur when processing a `Request`.
 #[derive(Debug)]
-pub enum Error {
-    /// An HTTP error from the `hyper` crate.
-    Http(::hyper::Error),
-    /// An error trying to serialize a value.
-    ///
-    /// This may be serializing a value that is illegal in JSON or
-    /// form-url-encoded bodies.
-    Serialize(Box<StdError + Send + Sync>),
-    /// A request tried to redirect too many times.
-    TooManyRedirects,
-    /// An infinite redirect loop was detected.
-    RedirectLoop,
-    #[doc(hidden)]
-    __DontMatchMe,
+pub struct Error {
+    kind: Kind,
+    url: Option<Url>,
+}
+
+/// A `Result` alias where the `Err` case is `reqwest::Error`.
+pub type Result<T> = ::std::result::Result<T, Error>;
+
+impl Error {
+    /// Returns a possible URL related to this error.
+    #[inline]
+    pub fn url(&self) -> Option<&Url> {
+        self.url.as_ref()
+    }
+
+    /// Returns true if the error is related to HTTP.
+    #[inline]
+    pub fn is_http(&self) -> bool {
+        match self.kind {
+            Kind::Http(_) => true,
+            _ => false,
+        }
+    }
+
+    /// Returns true if the error is serialization related.
+    #[inline]
+    pub fn is_serialization(&self) -> bool {
+        match self.kind {
+            Kind::Json(_) |
+            Kind::UrlEncoded(_) => true,
+            _ => false,
+        }
+    }
+
+    /// Returns true if the error is from a `RedirectPolicy`.
+    #[inline]
+    pub fn is_redirect(&self) -> bool {
+        match self.kind {
+            Kind::TooManyRedirects |
+            Kind::RedirectLoop => true,
+            _ => false,
+        }
+    }
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            Error::Http(ref e) => fmt::Display::fmt(e, f),
-            Error::Serialize(ref e) => fmt::Display::fmt(e, f),
-            Error::TooManyRedirects => f.pad("Too many redirects"),
-            Error::RedirectLoop => f.pad("Infinite redirect loop"),
-            Error::__DontMatchMe => unreachable!()
+        if let Some(ref url) = self.url {
+            try!(fmt::Display::fmt(url, f));
+            try!(f.write_str(": "));
+        }
+        match self.kind {
+            Kind::Http(ref e) => fmt::Display::fmt(e, f),
+            Kind::UrlEncoded(ref e) => fmt::Display::fmt(e, f),
+            Kind::Json(ref e) => fmt::Display::fmt(e, f),
+            Kind::TooManyRedirects => f.write_str("Too many redirects"),
+            Kind::RedirectLoop => f.write_str("Infinite redirect loop"),
         }
     }
 }
 
 impl StdError for Error {
     fn description(&self) -> &str {
-        match *self {
-            Error::Http(ref e) => e.description(),
-            Error::Serialize(ref e) => e.description(),
-            Error::TooManyRedirects => "Too many redirects",
-            Error::RedirectLoop => "Infinite redirect loop",
-            Error::__DontMatchMe => unreachable!()
+        match self.kind {
+            Kind::Http(ref e) => e.description(),
+            Kind::UrlEncoded(ref e) => e.description(),
+            Kind::Json(ref e) => e.description(),
+            Kind::TooManyRedirects => "Too many redirects",
+            Kind::RedirectLoop => "Infinite redirect loop",
         }
     }
 
     fn cause(&self) -> Option<&StdError> {
-        match *self {
-            Error::Http(ref e) => Some(e),
-            Error::Serialize(ref e) => Some(&**e),
-            Error::TooManyRedirects |
-            Error::RedirectLoop => None,
-            Error::__DontMatchMe => unreachable!()
+        match self.kind {
+            Kind::Http(ref e) => Some(e),
+            Kind::UrlEncoded(ref e) => Some(e),
+            Kind::Json(ref e) => Some(e),
+            Kind::TooManyRedirects |
+            Kind::RedirectLoop => None,
         }
     }
 }
 
-fn _assert_types() {
-    fn _assert_send<T: Send>() {
+// pub(crate)
+
+#[derive(Debug)]
+pub enum Kind {
+    Http(::hyper::Error),
+    UrlEncoded(::serde_urlencoded::ser::Error),
+    Json(::serde_json::Error),
+    TooManyRedirects,
+    RedirectLoop,
+}
+
+
+impl From<::hyper::Error> for Kind {
+    #[inline]
+    fn from(err: ::hyper::Error) -> Kind {
+        Kind::Http(err)
+    }
+}
+
+impl From<::url::ParseError> for Kind {
+    #[inline]
+    fn from(err: ::url::ParseError) -> Kind {
+        Kind::Http(::hyper::Error::Uri(err))
     }
-    _assert_send::<Error>();
 }
 
-impl From<::hyper::Error> for Error {
-    fn from(err: ::hyper::Error) -> Error {
-        Error::Http(err)
+impl From<::serde_urlencoded::ser::Error> for Kind {
+    #[inline]
+    fn from(err: ::serde_urlencoded::ser::Error) -> Kind {
+        Kind::UrlEncoded(err)
     }
 }
 
-impl From<::url::ParseError> for Error {
-    fn from(err: ::url::ParseError) -> Error {
-        Error::Http(::hyper::Error::Uri(err))
+impl From<::serde_json::Error> for Kind {
+    #[inline]
+    fn from(err: ::serde_json::Error) -> Kind {
+        Kind::Json(err)
     }
 }
 
-impl From<::serde_urlencoded::ser::Error> for Error {
-    fn from(err: ::serde_urlencoded::ser::Error) -> Error {
-        Error::Serialize(Box::new(err))
+pub struct InternalFrom<T>(pub T, pub Option<Url>);
+
+impl From<InternalFrom<Error>> for Error {
+    #[inline]
+    fn from(other: InternalFrom<Error>) -> Error {
+        other.0
     }
 }
 
-impl From<::serde_json::Error> for Error {
-    fn from(err: ::serde_json::Error) -> Error {
-        Error::Serialize(Box::new(err))
+impl<T> From<InternalFrom<T>> for Error
+where T: Into<Kind> {
+    #[inline]
+    fn from(other: InternalFrom<T>) -> Error {
+         Error {
+            kind: other.0.into(),
+            url: other.1,
+        }
     }
 }
 
-/// A `Result` alias where the `Err` case is `reqwest::Error`.
-pub type Result<T> = ::std::result::Result<T, Error>;
+#[inline]
+pub fn from<T>(err: T) -> Error
+where T: Into<Kind> {
+    InternalFrom(err, None).into()
+}
+
+#[inline]
+pub fn loop_detected(url: Url) -> Error {
+    Error {
+        kind: Kind::RedirectLoop,
+        url: Some(url),
+    }
+}
+
+#[inline]
+pub fn too_many_redirects(url: Url) -> Error {
+    Error {
+        kind: Kind::TooManyRedirects,
+        url: Some(url),
+    }
+}
+
+#[macro_export]
+macro_rules! try_ {
+    ($e:expr) => (
+        match $e {
+            Ok(v) => v,
+            Err(err) => {
+                return Err(::Error::from(::error::InternalFrom(err, None)));
+            }
+        }
+    );
+    ($e:expr, $url:expr) => (
+        match $e {
+            Ok(v) => v,
+            Err(err) => {
+                return Err(::Error::from(::error::InternalFrom(err, Some($url.clone()))));
+            }
+        }
+    )
+}

src/lib.rs

@@ -124,9 +124,9 @@ pub use self::body::Body;
 pub use self::redirect::RedirectPolicy;
 pub use self::response::Response;
 
+#[macro_use] mod error;
 mod body;
 mod client;
-mod error;
 mod redirect;
 mod response;
 
@@ -161,4 +161,8 @@ fn _assert_impls() {
 
     assert_send::<RequestBuilder>();
     assert_send::<Response>();
+
+
+    assert_send::<Error>();
+    assert_sync::<Error>();
 }