provide more detailed error info and align messages stylistically (#63)

* Capture io::Error

* Remove unused WebPushError::{SslError, TlsError}

* Provide more detailed error responses

* Align error messages stylistically

* fix warning about elidable lifetime

src/clients/hyper_client.rs

@@ -87,8 +87,12 @@ impl WebPushClient for HyperWebPushClient {
 
         debug!("Response: {:?}", response);
 
-        if let Err(WebPushError::ServerError(None)) = response {
-            Err(WebPushError::ServerError(retry_after))
+        if let Err(WebPushError::ServerError {
+            retry_after: None,
+            info,
+        }) = response
+        {
+            Err(WebPushError::ServerError { retry_after, info })
         } else {
             Ok(response?)
         }

src/clients/isahc_client.rs

@@ -90,8 +90,12 @@ impl WebPushClient for IsahcWebPushClient {
 
         trace!("Response: {:?}", response);
 
-        if let Err(WebPushError::ServerError(None)) = response {
-            Err(WebPushError::ServerError(retry_after))
+        if let Err(WebPushError::ServerError {
+            retry_after: None,
+            info,
+        }) = response
+        {
+            Err(WebPushError::ServerError { retry_after, info })
         } else {
             Ok(response?)
         }

src/clients/request_builder.rs

@@ -4,15 +4,7 @@
 use http::header::{CONTENT_ENCODING, CONTENT_LENGTH, CONTENT_TYPE};
 use http::{Request, StatusCode};
 
-use crate::{error::WebPushError, message::WebPushMessage};
-
-#[derive(Deserialize, Serialize, Debug, PartialEq)]
-struct ErrorInfo {
-    code: u16,
-    errno: u16,
-    error: String,
-    message: String,
-}
+use crate::{error::ErrorInfo, error::WebPushError, message::WebPushMessage};
 
 /// Builds the request to send to the push service.
 ///
@@ -72,25 +64,28 @@ where
 
 /// Parses the response from the push service, and will return `Err` if the request was bad.
 pub fn parse_response(response_status: StatusCode, body: Vec<u8>) -> Result<(), WebPushError> {
-    match response_status {
-        status if status.is_success() => Ok(()),
-        status if status.is_server_error() => Err(WebPushError::ServerError(None)),
-
-        StatusCode::UNAUTHORIZED => Err(WebPushError::Unauthorized),
-        StatusCode::GONE => Err(WebPushError::EndpointNotValid),
-        StatusCode::NOT_FOUND => Err(WebPushError::EndpointNotFound),
-        StatusCode::PAYLOAD_TOO_LARGE => Err(WebPushError::PayloadTooLarge),
+    if response_status.is_success() {
+        return Ok(());
+    }
 
-        StatusCode::BAD_REQUEST => match String::from_utf8(body) {
-            Err(_) => Err(WebPushError::BadRequest(None)),
-            Ok(body_str) => match serde_json::from_str::<ErrorInfo>(&body_str) {
-                Ok(error_info) => Err(WebPushError::BadRequest(Some(error_info.error))),
-                Err(_) if body_str.is_empty() => Err(WebPushError::BadRequest(None)),
-                Err(_) => Err(WebPushError::BadRequest(Some(body_str))),
-            },
-        },
+    let info: ErrorInfo = serde_json::from_slice(&body).unwrap_or_else(|_| ErrorInfo {
+        code: response_status.as_u16(),
+        errno: 999,
+        error: "unknown error".into(),
+        message: String::from_utf8(body).unwrap_or_else(|_| "-".into()),
+    });
 
-        e => Err(WebPushError::Other(format!("{:?}", e))),
+    match response_status {
+        StatusCode::UNAUTHORIZED => Err(WebPushError::Unauthorized(info)),
+        StatusCode::GONE => Err(WebPushError::EndpointNotValid(info)),
+        StatusCode::NOT_FOUND => Err(WebPushError::EndpointNotFound(info)),
+        StatusCode::PAYLOAD_TOO_LARGE => Err(WebPushError::PayloadTooLarge),
+        StatusCode::BAD_REQUEST => Err(WebPushError::BadRequest(info)),
+        status if status.is_server_error() => Err(WebPushError::ServerError {
+            retry_after: None,
+            info,
+        }),
+        _ => Err(WebPushError::Other(info)),
     }
 }
 
@@ -164,71 +159,76 @@ mod tests {
 
     #[test]
     fn parses_a_successful_response_correctly() {
-        assert_eq!(Ok(()), parse_response(StatusCode::OK, vec![]))
+        assert!(matches!(parse_response(StatusCode::OK, vec![]), Ok(())));
     }
 
     #[test]
     fn parses_an_unauthorized_response_correctly() {
-        assert_eq!(
-            Err(WebPushError::Unauthorized),
-            parse_response(StatusCode::UNAUTHORIZED, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::UNAUTHORIZED, vec![]),
+            Err(WebPushError::Unauthorized(_))
+        ));
     }
 
     #[test]
     fn parses_a_gone_response_correctly() {
-        assert_eq!(
-            Err(WebPushError::EndpointNotValid),
-            parse_response(StatusCode::GONE, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::GONE, vec![]),
+            Err(WebPushError::EndpointNotValid(_))
+        ));
     }
 
     #[test]
     fn parses_a_not_found_response_correctly() {
-        assert_eq!(
-            Err(WebPushError::EndpointNotFound),
-            parse_response(StatusCode::NOT_FOUND, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::NOT_FOUND, vec![]),
+            Err(WebPushError::EndpointNotFound(_))
+        ));
     }
 
     #[test]
     fn parses_a_payload_too_large_response_correctly() {
-        assert_eq!(
-            Err(WebPushError::PayloadTooLarge),
-            parse_response(StatusCode::PAYLOAD_TOO_LARGE, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::PAYLOAD_TOO_LARGE, vec![]),
+            Err(WebPushError::PayloadTooLarge)
+        ));
     }
 
     #[test]
     fn parses_a_server_error_response_correctly() {
-        assert_eq!(
-            Err(WebPushError::ServerError(None)),
-            parse_response(StatusCode::INTERNAL_SERVER_ERROR, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::INTERNAL_SERVER_ERROR, vec![]),
+            Err(WebPushError::ServerError { .. })
+        ));
     }
 
     #[test]
     fn parses_a_bad_request_response_with_no_body_correctly() {
-        assert_eq!(
-            Err(WebPushError::BadRequest(None)),
-            parse_response(StatusCode::BAD_REQUEST, vec![])
-        )
+        assert!(matches!(
+            parse_response(StatusCode::BAD_REQUEST, vec![]),
+            Err(WebPushError::BadRequest(_))
+        ));
     }
 
     #[test]
     fn parses_a_bad_request_response_with_body_correctly() {
         let json = r#"
             {
-                "code": 404,
+                "code": 400,
                 "errno": 103,
                 "error": "FooBar",
                 "message": "No message found"
             }
         "#;
 
-        assert_eq!(
-            Err(WebPushError::BadRequest(Some(String::from("FooBar")))),
-            parse_response(StatusCode::BAD_REQUEST, json.as_bytes().to_vec())
-        )
+        assert!(matches!(
+            parse_response(StatusCode::BAD_REQUEST, json.as_bytes().to_vec()),
+            Err(WebPushError::BadRequest(ErrorInfo {
+                code: 400,
+                errno: 103,
+                error: _,
+                message: _,
+            })),
+        ));
     }
 }

src/error.rs

@@ -6,32 +6,49 @@ use base64::DecodeError;
 use http::uri::InvalidUri;
 use serde_json::error::Error as JsonError;
 
-#[derive(PartialEq, Debug, Clone, Ord, PartialOrd, Eq, Deserialize, Serialize, Hash)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ErrorInfo {
+    pub code: u16,
+    pub errno: u16,
+    pub error: String,
+    pub message: String,
+}
+
+impl fmt::Display for ErrorInfo {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "code {}, errno {}: {} ({})",
+            self.code, self.errno, self.error, self.message
+        )
+    }
+}
+
+#[derive(Debug)]
 pub enum WebPushError {
-    /// An unknown error happened encrypting the message,
+    /// An unknown error happened while encrypting or sending the message
     Unspecified,
     /// Please provide valid credentials to send the notification
-    Unauthorized,
+    Unauthorized(ErrorInfo),
     /// Request was badly formed
-    BadRequest(Option<String>),
+    BadRequest(ErrorInfo),
     /// Contains an optional `Duration`, until the user can retry the request
-    ServerError(Option<Duration>),
+    ServerError {
+        retry_after: Option<Duration>,
+        info: ErrorInfo,
+    },
     /// The feature is not implemented yet
-    NotImplemented,
+    NotImplemented(ErrorInfo),
     /// The provided URI is invalid
     InvalidUri,
     /// The URL specified is no longer valid and should no longer be used
-    EndpointNotValid,
+    EndpointNotValid(ErrorInfo),
     /// The URL specified is invalid and should not be used again
-    EndpointNotFound,
+    EndpointNotFound(ErrorInfo),
     /// Maximum allowed payload size is 3800 characters
     PayloadTooLarge,
-    /// Could not initialize a TLS connection
-    TlsError,
-    /// Error in SSL signing
-    SslError,
     /// Error in reading a file
-    IoError,
+    Io(IoError),
     /// Make sure the message was addressed to a registration token whose
     /// package name matches the value passed in the request (Google).
     InvalidPackageName,
@@ -47,7 +64,7 @@ pub enum WebPushError {
     InvalidResponse,
     /// A claim had invalid data
     InvalidClaims,
-    Other(String),
+    Other(ErrorInfo),
 }
 
 impl Error for WebPushError {}
@@ -85,8 +102,8 @@ impl From<isahc::Error> for WebPushError {
 }
 
 impl From<IoError> for WebPushError {
-    fn from(_: IoError) -> WebPushError {
-        WebPushError::IoError
+    fn from(err: IoError) -> WebPushError {
+        WebPushError::Io(err)
     }
 }
 
@@ -100,23 +117,21 @@ impl WebPushError {
     pub fn short_description(&self) -> &'static str {
         match *self {
             WebPushError::Unspecified => "unspecified",
-            WebPushError::Unauthorized => "unauthorized",
+            WebPushError::Unauthorized(_) => "unauthorized",
             WebPushError::BadRequest(_) => "bad_request",
-            WebPushError::ServerError(_) => "server_error",
-            WebPushError::NotImplemented => "not_implemented",
+            WebPushError::ServerError { .. } => "server_error",
+            WebPushError::NotImplemented(_) => "not_implemented",
             WebPushError::InvalidUri => "invalid_uri",
-            WebPushError::EndpointNotValid => "endpoint_not_valid",
-            WebPushError::EndpointNotFound => "endpoint_not_found",
+            WebPushError::EndpointNotValid(_) => "endpoint_not_valid",
+            WebPushError::EndpointNotFound(_) => "endpoint_not_found",
             WebPushError::PayloadTooLarge => "payload_too_large",
-            WebPushError::TlsError => "tls_error",
             WebPushError::InvalidPackageName => "invalid_package_name",
             WebPushError::InvalidTtl => "invalid_ttl",
             WebPushError::InvalidTopic => "invalid_topic",
             WebPushError::InvalidResponse => "invalid_response",
             WebPushError::MissingCryptoKeys => "missing_crypto_keys",
             WebPushError::InvalidCryptoKeys => "invalid_crypto_keys",
-            WebPushError::SslError => "ssl_error",
-            WebPushError::IoError => "io_error",
+            WebPushError::Io(_) => "io_error",
             WebPushError::Other(_) => "other",
             WebPushError::InvalidClaims => "invalidClaims",
         }
@@ -125,40 +140,28 @@ impl WebPushError {
 
 impl fmt::Display for WebPushError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            WebPushError::Unspecified =>
-                write!(f, "An unknown error happened encrypting the message"),
-            WebPushError::Unauthorized =>
-                write!(f, "Please provide valid credentials to send the notification"),
-            WebPushError::BadRequest(_) =>
-                write!(f, "Request was badly formed"),
-            WebPushError::ServerError(_) =>
-                write!(f, "Server was unable to process the request, please try again later"),
-            WebPushError::PayloadTooLarge =>
-                write!(f, "Maximum allowed payload size is 3070 characters"),
-            WebPushError::InvalidUri =>
-                write!(f, "The provided URI is invalid"),
-            WebPushError::NotImplemented =>
-                write!(f, "The feature is not implemented yet"),
-            WebPushError::EndpointNotValid =>
-                write!(f, "The URL specified is no longer valid and should no longer be used"),
-            WebPushError::EndpointNotFound =>
-                write!(f, "The URL specified is invalid and should not be used again"),
-            WebPushError::TlsError =>
-                write!(f, "Could not initialize a TLS connection"),
-            WebPushError::SslError =>
-                write!(f, "Error signing with SSL"),
-            WebPushError::IoError =>
-                write!(f, "Error opening a file"),
-            WebPushError::InvalidPackageName =>
-                write!(f, "Make sure the message was addressed to a registration token whose package name matches the value passed in the request."),
-            WebPushError::InvalidTtl => write!(f, "The TTL value provided was not valid or was not provided"),
-            WebPushError::InvalidTopic => write!(f, "The Topic value provided was invalid"),
-            WebPushError::InvalidResponse => write!(f, "The response data couldn't be parses"),
-            WebPushError::MissingCryptoKeys  => write!(f, "The request is missing cryptographic keys"),
-            WebPushError::InvalidCryptoKeys  => write!(f, "The request is having invalid cryptographic keys"),
-            WebPushError::Other(_) => write!(f, "An unknown error when connecting the notification service"),
-            WebPushError::InvalidClaims => write!(f, "At least one JWT claim was invalid.")
+        match self {
+            WebPushError::Unspecified => write!(f, "unspecified error"),
+            WebPushError::Unauthorized(info) => write!(f, "unauthorized: {}", info),
+            WebPushError::BadRequest(info) => write!(f, "bad request: {}", info),
+            WebPushError::ServerError { info, .. } => write!(f, "server error: {}", info),
+            WebPushError::PayloadTooLarge => write!(f, "maximum payload size of 3070 characters exceeded"),
+            WebPushError::InvalidUri => write!(f, "invalid uri provided"),
+            WebPushError::NotImplemented(info) => write!(f, "not implemented: {}", info),
+            WebPushError::EndpointNotValid(info) => write!(f, "endpoint not valid: {}", info),
+            WebPushError::EndpointNotFound(info) => write!(f, "endpoint not found: {}", info),
+            WebPushError::Io(err) => write!(f, "i/o error: {}", err),
+            WebPushError::InvalidPackageName => write!(
+                f,
+                "package name of registration token does not match package name provided in the request"
+            ),
+            WebPushError::InvalidTtl => write!(f, "invalid or missing ttl value"),
+            WebPushError::InvalidTopic => write!(f, "invalid topic value"),
+            WebPushError::InvalidResponse => write!(f, "could not parse response data"),
+            WebPushError::MissingCryptoKeys => write!(f, "request is missing cryptographic keys"),
+            WebPushError::InvalidCryptoKeys => write!(f, "request has invalid cryptographic keys"),
+            WebPushError::Other(info) => write!(f, "other: {}", info),
+            WebPushError::InvalidClaims => write!(f, "at least one jwt claim was invalid"),
         }
     }
 }

src/http_ece.rs

@@ -147,7 +147,7 @@ mod tests {
         //This content is one above limit.
         let content = [0u8; 3801];
 
-        assert_eq!(Err(WebPushError::PayloadTooLarge), http_ece.encrypt(&content));
+        assert!(matches!(http_ece.encrypt(&content), Err(WebPushError::PayloadTooLarge)));
     }
 
     /// Tests that the content encryption is properly reversible while using aes128gcm.