From 81326a5452ebe77bd74532b37a0d7e951e8333e1 Mon Sep 17 00:00:00 2001
From: Mingwei Samuel <mingwei.samuel@gmail.com>
Date: Tue, 31 Mar 2020 12:49:17 -0700
Subject: [PATCH] Add status code to RiotApiError, handle 204 to return None

---
 src/error.rs                  | 22 ++++++++++++++++------
 src/req/regional_requester.rs | 18 ++++++++++--------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index 614609b..e2c59d6 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -3,8 +3,9 @@ use std::fmt;
 
 /// Re-exported `reqwest` types.
 pub mod reqwest {
-    pub use reqwest::Error;
-    pub use reqwest::Response;
+    pub use reqwest::{
+        Error, Response, StatusCode, Url
+    };
 }
 use ::reqwest::*;
 
@@ -20,13 +21,15 @@ pub struct RiotApiError {
     reqwest_error: Error,
     retries: u8,
     response: Option<Response>,
+    status_code: Option<StatusCode>,
 }
 impl RiotApiError {
-    pub fn new(reqwest_error: Error, retries: u8, response: Option<Response>) -> Self {
+    pub(crate) fn new(reqwest_error: Error, retries: u8, response: Option<Response>, status_code: Option<StatusCode>) -> Self {
         Self {
             reqwest_error: reqwest_error,
             retries: retries,
             response: response,
+            status_code: status_code,
         }
     }
     /// The reqwest::Error for the final failed request.
@@ -37,11 +40,18 @@ impl RiotApiError {
     pub fn retries(&self) -> u8 {
         self.retries
     }
-    /// The failed response, if the request was sent and failed.
-    /// Will be `None` if JSON parsing failed.
-    pub fn response<'a>(&self) -> Option<&Response> {
+    /// The failed response.
+    /// `Some(reqwest::Response)` if the request was sent and failed.
+    /// `None` if the request was not sent, OR if parsing the response JSON failed.
+    pub fn response(&self) -> Option<&Response> {
         self.response.as_ref()
     }
+    /// The failed response's HTTP status code.
+    /// `Some(reqwest::StatusCode)` if the request was sent and failed, OR if parsing the response JSON failed.
+    /// `None` if the request was not sent.
+    pub fn status_code(&self) -> Option<StatusCode> {
+        self.status_code
+    }
 }
 impl fmt::Display for RiotApiError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
diff --git a/src/req/regional_requester.rs b/src/req/regional_requester.rs
index b9ad5ef..0d7f93a 100644
--- a/src/req/regional_requester.rs
+++ b/src/req/regional_requester.rs
@@ -24,9 +24,11 @@ impl RegionalRequester {
     /// Request header name for the Riot API key.
     const RIOT_KEY_HEADER: &'static str = "X-Riot-Token";
 
-    /// Http status code 404, considered a success but will return None.
-    const NONE_STATUS_CODE: StatusCode = StatusCode::NOT_FOUND;
-
+    /// HTTP status codes which are considered a success but will results in `None`.
+    const NONE_STATUS_CODES: [StatusCode; 2] = [
+        StatusCode::NO_CONTENT, // 204
+        StatusCode::NOT_FOUND,  // 404
+    ];
 
     pub fn new() -> Self {
         Self {
@@ -45,8 +47,8 @@ impl RegionalRequester {
                 method_id, region_platform, path, query).await;
             response_result.map(|value| Some(value))
                 .or_else(|e| {
-                    if let Some(response) = e.response() {
-                    if Self::NONE_STATUS_CODE == response.status() {
+                    if let Some(status) = e.status_code() {
+                    if Self::NONE_STATUS_CODES.contains(&status) {
                         return Ok(None);
                     }}
                     Err(e)
@@ -84,7 +86,7 @@ impl RegionalRequester {
                     .header(Self::RIOT_KEY_HEADER, &*config.api_key)
                     .send()
                     .await
-                    .map_err(|e| RiotApiError::new(e, retries, None))?;
+                    .map_err(|e| RiotApiError::new(e, retries, None, None))?;
 
                 // Maybe update rate limits (based on response headers).
                 self.app_rate_limit.on_response(&config, &response);
@@ -97,7 +99,7 @@ impl RegionalRequester {
                     Ok(_response) => {
                         log::trace!("Response {} (retried {} times), parsed result.", status, retries);
                         let value = response.json::<T>().await;
-                        break value.map_err(|e| RiotApiError::new(e, retries, None));
+                        break value.map_err(|e| RiotApiError::new(e, retries, None, Some(status)));
                     },
                     // Failure, may or may not be retryable.
                     Err(err) => {
@@ -108,7 +110,7 @@ impl RegionalRequester {
                             && !status.is_server_error())
                         {
                             log::debug!("Response {} (retried {} times), returning.", status, retries);
-                            break Err(RiotApiError::new(err, retries, Some(response)));
+                            break Err(RiotApiError::new(err, retries, Some(response), Some(status)));
                         }
                         log::debug!("Response {} (retried {} times), retrying.", status, retries);
                     },