Restructure, add RiotApi.get_raw_response(...) method useful for proxies.

New RiotApi.get_raw_response(...) method similar to existing
RiotApi.get(...) and RiotApi.get_optional(...), but does not parse the
result. Useful for proxies which just want to forward the content body.

Also adds tests to JP of failure cases.

src/lib.rs

@@ -24,6 +24,9 @@ pub mod models;
 
 mod req;
 
+mod response_info;
+pub use response_info::*;
+
 mod riot_api;
 pub use riot_api::*;
 

src/req/regional_requester.rs

@@ -6,6 +6,7 @@ use reqwest::{ Client, StatusCode, Url };
 use tokio::time::delay_for;
 
 use crate::Result;
+use crate::ResponseInfo;
 use crate::RiotApiError;
 use crate::RiotApiConfig;
 use crate::util::InsertOnlyCHashMap;
@@ -14,9 +15,9 @@ use super::RateLimit;
 use super::RateLimitType;
 
 pub struct RegionalRequester {
-    /// Represents the app rate limit.
+    /// The app rate limit.
     app_rate_limit: RateLimit,
-    /// Represents method rate limits.
+    /// Method rate limits.
     method_rate_limits: InsertOnlyCHashMap<&'static str, RateLimit>,
 }
 
@@ -37,29 +38,10 @@ impl RegionalRequester {
         }
     }
 
-    pub fn get_optional<'a, T: serde::de::DeserializeOwned>(self: Arc<Self>,
+    pub fn get<'a>(self: Arc<Self>,
         config: &'a RiotApiConfig, client: &'a Client,
         method_id: &'static str, region_platform: &'a str, path: String, query: Option<String>)
-        -> impl Future<Output = Result<Option<T>>> + 'a
-    {
-        async move {
-            let response_result = self.get(config, client,
-                method_id, region_platform, path, query).await;
-            response_result.map(|value| Some(value))
-                .or_else(|e| {
-                    if let Some(status) = e.status_code() {
-                    if Self::NONE_STATUS_CODES.contains(&status) {
-                        return Ok(None);
-                    }}
-                    Err(e)
-                })
-        }
-    }
-
-    pub fn get<'a, T: serde::de::DeserializeOwned>(self: Arc<Self>,
-        config: &'a RiotApiConfig, client: &'a Client,
-        method_id: &'static str, region_platform: &'a str, path: String, query: Option<String>)
-        -> impl Future<Output = Result<T>> + 'a
+        -> impl Future<Output = Result<ResponseInfo>> + 'a
     {
         async move {
             #[cfg(feature = "nightly")] let query = query.as_deref();
@@ -94,27 +76,29 @@ impl RegionalRequester {
 
                 let status = response.status();
                 // Handle normal success / failure cases.
-                match response.error_for_status_ref() {
-                    // Success.
-                    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, Some(status)));
-                    },
-                    // Failure, may or may not be retryable.
-                    Err(err) => {
-                        // Not-retryable: no more retries or 4xx or ? (3xx, redirects exceeded).
-                        // Retryable: retries remaining, and 429 or 5xx.
-                        if retries >= config.retries ||
-                            (StatusCode::TOO_MANY_REQUESTS != status
-                            && !status.is_server_error())
-                        {
-                            log::debug!("Response {} (retried {} times), returning.", status, retries);
-                            break Err(RiotApiError::new(err, retries, Some(response), Some(status)));
-                        }
-                        log::debug!("Response {} (retried {} times), retrying.", status, retries);
-                    },
-                };
+                let status_none = Self::NONE_STATUS_CODES.contains(&status);
+                // Success case.
+                if status.is_success() || status_none {
+                    log::trace!("Response {} (retried {} times), success, returning result.", status, retries);
+                    break Ok(ResponseInfo {
+                        response: response,
+                        retries: retries,
+                        status_none: status_none,
+                    });
+                }
+                let err = response.error_for_status_ref().err().unwrap_or_else(
+                    || panic!("Unhandlable response status code, neither success nor failure: {}.", status));
+                // Failure, may or may not be retryable.
+                // Not-retryable: no more retries or 4xx or ? (3xx, redirects exceeded).
+                // Retryable: retries remaining, and 429 or 5xx.
+                if retries >= config.retries ||
+                    (StatusCode::TOO_MANY_REQUESTS != status
+                    && !status.is_server_error())
+                {
+                    log::debug!("Response {} (retried {} times), failure, returning error.", status, retries);
+                    break Err(RiotApiError::new(err, retries, Some(response), Some(status)));
+                }
+                log::debug!("Response {} (retried {} times), retrying.", status, retries);
 
                 retries += 1;
             }

src/response_info.rs

@@ -0,0 +1,11 @@
+use reqwest::Response;
+
+/// A "raw" unparsed successful response from the Riot API, for internal or advanced use cases.
+pub struct ResponseInfo {
+    /// The reqwest response.
+    pub response: Response,
+    /// The number of retries used, zero for first-try success.
+    pub retries: u8,
+    /// If the response has an HTTP status code indicating a `None` response (i.e. 204, 404).
+    pub status_none: bool,
+}

src/riot_api.rs

@@ -5,7 +5,9 @@ use log;
 use reqwest::Client;
 
 use crate::Result;
+use crate::ResponseInfo;
 use crate::RiotApiConfig;
+use crate::RiotApiError;
 use crate::req::RegionalRequester;
 use crate::util::InsertOnlyCHashMap;
 
@@ -74,12 +76,22 @@ impl RiotApi {
     /// * `region_platform` - The stringified platform, prepended to `.api.riotgames.com` to create the hostname.
     /// * `path` - The path relative to the hostname.
     /// * `query` - An optional query string.
-    pub fn get_optional<'a, T: serde::de::DeserializeOwned + 'a>(&'a self,
+    ///
+    /// # Returns
+    /// A future resolving to a `Result` containg either a `Option<T>` (success) or a `RiotApiError` (failure).
+    pub async fn get_optional<'a, T: serde::de::DeserializeOwned + 'a>(&'a self,
         method_id: &'static str, region_platform: &'static str, path: String, query: Option<String>)
-        -> impl Future<Output = Result<Option<T>>> + 'a
+        -> Result<Option<T>>
     {
-        self.regional_requester(region_platform)
-            .get_optional(&self.config, &self.client, method_id, region_platform, path, query)
+        let rinfo = self.get_raw_response(method_id, region_platform, path, query).await?;
+        if rinfo.status_none {
+            return Ok(None);
+        }
+        let retries = rinfo.retries;
+        let status = rinfo.response.status();
+        let value = rinfo.response.json::<T>().await;
+        value.map(|v| Some(v))
+            .map_err(|e| RiotApiError::new(e, retries, None, Some(status)))
     }
 
     /// This method is not meant to be used directly.
@@ -91,9 +103,37 @@ impl RiotApi {
     /// * `region_platform` - The stringified platform, prepended to `.api.riotgames.com` to create the hostname.
     /// * `path` - The path relative to the hostname.
     /// * `query` - An optional query string.
-    pub fn get<'a, T: serde::de::DeserializeOwned + 'a>(&'a self,
+    ///
+    /// # Returns
+    /// A future resolving to a `Result` containg either a `T` (success) or a `RiotApiError` (failure).
+    pub async fn get<'a, T: serde::de::DeserializeOwned + 'a>(&'a self,
         method_id: &'static str, region_platform: &'static str, path: String, query: Option<String>)
-        -> impl Future<Output = Result<T>> + 'a
+        -> Result<T>
+    {
+        let rinfo = self.get_raw_response(method_id, region_platform, path, query).await?;
+        let retries = rinfo.retries;
+        let status = rinfo.response.status();
+        let value = rinfo.response.json::<T>().await;
+        value.map_err(|e| RiotApiError::new(e, retries, None, Some(status)))
+    }
+
+    /// This method is not meant to be used directly.
+    ///
+    /// This sends a GET request based on the given parameters and returns a raw `ResponseInfo`.
+    ///
+    /// This can be used to implement a Riot API proxy without needing to deserialize and reserialize JSON responses.
+    ///
+    /// # Parameters
+    /// * `method_id` - A unique string id representing the endpoint method for per-method rate limiting.
+    /// * `region_platform` - The stringified platform, prepended to `.api.riotgames.com` to create the hostname.
+    /// * `path` - The path relative to the hostname.
+    /// * `query` - An optional query string.
+    ///
+    /// # Returns
+    /// A future resolving to a `Result` containg either a `ResponseInfo` (success) or a `RiotApiError` (failure).
+    pub fn get_raw_response<'a>(&'a self,
+        method_id: &'static str, region_platform: &'static str, path: String, query: Option<String>)
+        -> impl Future<Output = Result<ResponseInfo>> + 'a
     {
         self.regional_requester(region_platform)
             .get(&self.config, &self.client, method_id, region_platform, path, query)

tests/tests_jp.rs

@@ -9,6 +9,7 @@ use colored::*;
 
 use riven::consts::*;
 
+
 async_tests!{
     my_runner {
         // Summoner tests.
@@ -18,5 +19,32 @@ async_tests!{
             rassert_eq!("私の頭がかたい", s.name);
             Ok(())
         },
+
+        // Failure cases.
+        // Make sure get_raw_response(...) with invalid path fails as expected.
+        raw_response_invalid: async {
+            let p = RIOT_API.get_raw_response("summoner-v4.getBySummonerName", Region::JP.into(), "INVALID/PATH".to_owned(), None);
+            let r = p.await;
+            rassert!(r.is_err());
+            Ok(())
+        },
+        // summoner_v4().get_by_summoner_name(...) normally returns an option.
+        // If we use `get` (instead of `get_optional`) make sure it errors.
+        get_nonoptional_invalid: async {
+            let path_string = format!("/lol/summoner/v4/summoners/by-name/{}", "SUMMONER THAT DOES NOT EXIST");
+            let p = RIOT_API.get::<riven::models::summoner_v4::Summoner>(
+                "summoner-v4.getBySummonerName", Region::JP.into(), path_string, None);
+            let r = p.await;
+            rassert!(r.is_err());
+            Ok(())
+        },
+        // Make sure 403 is handled as expected.
+        tournament_forbidden: async {
+            let p = RIOT_API.tournament_v4().get_tournament_code(Region::JP, "INVALID_CODE");
+            let r = p.await;
+            rassert!(r.is_err());
+            rassert_eq!(Some(reqwest::StatusCode::FORBIDDEN), r.unwrap_err().status_code());
+            Ok(())
+        },
     }
 }