From c3c3e639ee7bd972342445eab582edbf343a6638 Mon Sep 17 00:00:00 2001 From: Mingwei Samuel Date: Tue, 22 Oct 2019 19:33:15 -0700 Subject: [PATCH] update error handling --- src/consts/champion.rs | 2 +- src/endpoints.rs | 58 +++++++++++++++--------------- src/endpoints/dto.rs | 2 +- src/lib.rs | 5 ++- src/req/regional_requester.rs | 67 ++++++++++++++++------------------- src/riot_api.rs | 4 +-- srcgen/endpoints.rs.dt | 4 +-- 7 files changed, 70 insertions(+), 72 deletions(-) diff --git a/src/consts/champion.rs b/src/consts/champion.rs index 0bc36be..7bdc770 100644 --- a/src/consts/champion.rs +++ b/src/consts/champion.rs @@ -1,7 +1,7 @@  // This file is automatically generated. // Do not directly edit. -// Generated on 2019-10-22T21:42:10.101Z +// Generated on 2019-10-23T01:41:03.103Z use std::fmt; use num_derive; diff --git a/src/endpoints.rs b/src/endpoints.rs index fbd9f34..28da537 100644 --- a/src/endpoints.rs +++ b/src/endpoints.rs @@ -1,7 +1,7 @@ // This file is automatically generated. // Do not directly edit. -// Generated on 2019-10-22T21:42:10.216Z +// Generated on 2019-10-23T01:41:03.165Z // http://www.mingweisamuel.com/riotapi-schema/tool/ // Version 0c74167e0eaaeb6de1c7e8219fecaabcf8386d1f @@ -12,9 +12,9 @@ pub use dto::*; use std::future::Future; use std::vec::Vec; -use reqwest; use url::form_urlencoded::Serializer; +use crate::Result; use crate::consts::Region; use crate::riot_api::RiotApi; @@ -112,7 +112,7 @@ impl<'a> ChampionMasteryV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` - Summoner ID associated with the player pub fn get_all_champion_masteries(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future>, reqwest::Error>> + 'a + -> impl Future>>> + 'a { let path_string = format!("/lol/champion-mastery/v4/champion-masteries/by-summoner/{}", encrypted_summoner_id); self.base.get::>("champion-mastery-v4.getAllChampionMasteries", region, path_string, None) @@ -126,7 +126,7 @@ impl<'a> ChampionMasteryV4<'a> { /// * `championId` - Champion ID to retrieve Champion Mastery for /// * `encryptedSummonerId` - Summoner ID associated with the player pub fn get_champion_mastery(&self, region: Region, encrypted_summoner_id: &str, champion_id: i64) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/champion-mastery/v4/champion-masteries/by-summoner/{}/by-champion/{}", encrypted_summoner_id, champion_id); self.base.get::("champion-mastery-v4.getChampionMastery", region, path_string, None) @@ -139,7 +139,7 @@ impl<'a> ChampionMasteryV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` - Summoner ID associated with the player pub fn get_champion_mastery_score(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/champion-mastery/v4/scores/by-summoner/{}", encrypted_summoner_id); self.base.get::("champion-mastery-v4.getChampionMasteryScore", region, path_string, None) @@ -160,7 +160,7 @@ impl<'a> ChampionV3<'a> { /// # Parameters /// * `region` - Region to query. pub fn get_champion_info(&self, region: Region) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = "/lol/platform/v3/champion-rotations".to_owned(); self.base.get::("champion-v3.getChampionInfo", region, path_string, None) @@ -185,7 +185,7 @@ impl<'a> LeagueExpV4<'a> { /// * `division` /// * `page` (optional) - Starts with page 1. pub fn get_league_entries(&self, region: Region, division: &str, tier: &str, queue: &str, page: Option) - -> impl Future>, reqwest::Error>> + 'a + -> impl Future>>> + 'a { let mut query_params = Serializer::new(String::new()); if let Some(page) = page { query_params.append_pair("page", &*page.to_string()); }; @@ -210,7 +210,7 @@ impl<'a> LeagueV4<'a> { /// * `region` - Region to query. /// * `queue` pub fn get_challenger_league(&self, region: Region, queue: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/league/v4/challengerleagues/by-queue/{}", queue); self.base.get::("league-v4.getChallengerLeague", region, path_string, None) @@ -223,7 +223,7 @@ impl<'a> LeagueV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` pub fn get_league_entries_for_summoner(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future>, reqwest::Error>> + 'a + -> impl Future>>> + 'a { let path_string = format!("/lol/league/v4/entries/by-summoner/{}", encrypted_summoner_id); self.base.get::>("league-v4.getLeagueEntriesForSummoner", region, path_string, None) @@ -239,7 +239,7 @@ impl<'a> LeagueV4<'a> { /// * `queue` - Note that the queue value must be a valid ranked queue. /// * `page` (optional) - Starts with page 1. pub fn get_league_entries(&self, region: Region, queue: &str, tier: &str, division: &str, page: Option) - -> impl Future>, reqwest::Error>> + 'a + -> impl Future>>> + 'a { let mut query_params = Serializer::new(String::new()); if let Some(page) = page { query_params.append_pair("page", &*page.to_string()); }; @@ -255,7 +255,7 @@ impl<'a> LeagueV4<'a> { /// * `region` - Region to query. /// * `queue` pub fn get_grandmaster_league(&self, region: Region, queue: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/league/v4/grandmasterleagues/by-queue/{}", queue); self.base.get::("league-v4.getGrandmasterLeague", region, path_string, None) @@ -268,7 +268,7 @@ impl<'a> LeagueV4<'a> { /// * `region` - Region to query. /// * `leagueId` - The UUID of the league. pub fn get_league_by_id(&self, region: Region, league_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/league/v4/leagues/{}", league_id); self.base.get::("league-v4.getLeagueById", region, path_string, None) @@ -281,7 +281,7 @@ impl<'a> LeagueV4<'a> { /// * `region` - Region to query. /// * `queue` pub fn get_master_league(&self, region: Region, queue: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/league/v4/masterleagues/by-queue/{}", queue); self.base.get::("league-v4.getMasterLeague", region, path_string, None) @@ -304,7 +304,7 @@ impl<'a> LolStatusV3<'a> { /// # Parameters /// * `region` - Region to query. pub fn get_shard_data(&self, region: Region) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = "/lol/status/v3/shard-data".to_owned(); self.base.get::("lol-status-v3.getShardData", region, path_string, None) @@ -326,7 +326,7 @@ impl<'a> MatchV4<'a> { /// * `region` - Region to query. /// * `tournamentCode` - The tournament code. pub fn get_match_ids_by_tournament_code(&self, region: Region, tournament_code: &str) - -> impl Future>, reqwest::Error>> + 'a + -> impl Future>>> + 'a { let path_string = format!("/lol/match/v4/matches/by-tournament-code/{}/ids", tournament_code); self.base.get::>("match-v4.getMatchIdsByTournamentCode", region, path_string, None) @@ -339,7 +339,7 @@ impl<'a> MatchV4<'a> { /// * `region` - Region to query. /// * `matchId` - The match ID. pub fn get_match(&self, region: Region, match_id: i64) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/match/v4/matches/{}", match_id); self.base.get::("match-v4.getMatch", region, path_string, None) @@ -353,7 +353,7 @@ impl<'a> MatchV4<'a> { /// * `tournamentCode` - The tournament code. /// * `matchId` - The match ID. pub fn get_match_by_tournament_code(&self, region: Region, match_id: i64, tournament_code: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/match/v4/matches/{}/by-tournament-code/{}", match_id, tournament_code); self.base.get::("match-v4.getMatchByTournamentCode", region, path_string, None) @@ -379,7 +379,7 @@ impl<'a> MatchV4<'a> { /// * `endIndex` (optional) - The end index to use for filtering matchlist. If beginIndex is specified, but not endIndex, then endIndex defaults to beginIndex+100. If endIndex is specified, but not beginIndex, then beginIndex defaults to 0. If both are specified, then endIndex must be greater than beginIndex. The maximum range allowed is 100, otherwise a 400 error code is returned. /// * `beginIndex` (optional) - The begin index to use for filtering matchlist. If beginIndex is specified, but not endIndex, then endIndex defaults to beginIndex+100. If endIndex is specified, but not beginIndex, then beginIndex defaults to 0. If both are specified, then endIndex must be greater than beginIndex. The maximum range allowed is 100, otherwise a 400 error code is returned. pub fn get_matchlist(&self, region: Region, encrypted_account_id: &str, champion: Option>, queue: Option>, season: Option>, end_time: Option, begin_time: Option, end_index: Option, begin_index: Option) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let mut query_params = Serializer::new(String::new()); if let Some(champion) = champion { query_params.extend_pairs(champion.iter().map(|w| ("champion", w.to_string()))); }; @@ -403,7 +403,7 @@ impl<'a> MatchV4<'a> { /// * `region` - Region to query. /// * `matchId` - The match ID. pub fn get_match_timeline(&self, region: Region, match_id: i64) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/match/v4/timelines/by-match/{}", match_id); self.base.get::("match-v4.getMatchTimeline", region, path_string, None) @@ -425,7 +425,7 @@ impl<'a> SpectatorV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` - The ID of the summoner. pub fn get_current_game_info_by_summoner(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/spectator/v4/active-games/by-summoner/{}", encrypted_summoner_id); self.base.get::("spectator-v4.getCurrentGameInfoBySummoner", region, path_string, None) @@ -437,7 +437,7 @@ impl<'a> SpectatorV4<'a> { /// # Parameters /// * `region` - Region to query. pub fn get_featured_games(&self, region: Region) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = "/lol/spectator/v4/featured-games".to_owned(); self.base.get::("spectator-v4.getFeaturedGames", region, path_string, None) @@ -459,7 +459,7 @@ impl<'a> SummonerV4<'a> { /// * `region` - Region to query. /// * `encryptedAccountId` pub fn get_by_account_id(&self, region: Region, encrypted_account_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/summoner/v4/summoners/by-account/{}", encrypted_account_id); self.base.get::("summoner-v4.getByAccountId", region, path_string, None) @@ -472,7 +472,7 @@ impl<'a> SummonerV4<'a> { /// * `region` - Region to query. /// * `summonerName` - Summoner Name pub fn get_by_summoner_name(&self, region: Region, summoner_name: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/summoner/v4/summoners/by-name/{}", summoner_name); self.base.get::("summoner-v4.getBySummonerName", region, path_string, None) @@ -485,7 +485,7 @@ impl<'a> SummonerV4<'a> { /// * `region` - Region to query. /// * `encryptedPUUID` - Summoner ID pub fn get_by_puuid(&self, region: Region, encrypted_puuid: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/summoner/v4/summoners/by-puuid/{}", encrypted_puuid); self.base.get::("summoner-v4.getByPUUID", region, path_string, None) @@ -498,7 +498,7 @@ impl<'a> SummonerV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` - Summoner ID pub fn get_by_summoner_id(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/summoner/v4/summoners/{}", encrypted_summoner_id); self.base.get::("summoner-v4.getBySummonerId", region, path_string, None) @@ -520,7 +520,7 @@ impl<'a> ThirdPartyCodeV4<'a> { /// * `region` - Region to query. /// * `encryptedSummonerId` pub fn get_third_party_code_by_summoner_id(&self, region: Region, encrypted_summoner_id: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/platform/v4/third-party-code/by-summoner/{}", encrypted_summoner_id); self.base.get::("third-party-code-v4.getThirdPartyCodeBySummonerId", region, path_string, None) @@ -542,7 +542,7 @@ impl<'a> TournamentStubV4<'a> { /// * `region` - Region to query. /// * `tournamentCode` - The short code to look up lobby events for pub fn get_lobby_events_by_code(&self, region: Region, tournament_code: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/tournament-stub/v4/lobby-events/by-code/{}", tournament_code); self.base.get::("tournament-stub-v4.getLobbyEventsByCode", region, path_string, None) @@ -564,7 +564,7 @@ impl<'a> TournamentV4<'a> { /// * `region` - Region to query. /// * `tournamentCode` - The tournament code string. pub fn get_tournament_code(&self, region: Region, tournament_code: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/tournament/v4/codes/{}", tournament_code); self.base.get::("tournament-v4.getTournamentCode", region, path_string, None) @@ -577,7 +577,7 @@ impl<'a> TournamentV4<'a> { /// * `region` - Region to query. /// * `tournamentCode` - The short code to look up lobby events for pub fn get_lobby_events_by_code(&self, region: Region, tournament_code: &str) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { let path_string = format!("/lol/tournament/v4/lobby-events/by-code/{}", tournament_code); self.base.get::("tournament-v4.getLobbyEventsByCode", region, path_string, None) diff --git a/src/endpoints/dto.rs b/src/endpoints/dto.rs index fa14542..0242bef 100644 --- a/src/endpoints/dto.rs +++ b/src/endpoints/dto.rs @@ -1,7 +1,7 @@ // This file is automatically generated. // Do not directly edit. -// Generated on 2019-10-22T21:42:10.207Z +// Generated on 2019-10-23T01:41:03.173Z // http://www.mingweisamuel.com/riotapi-schema/tool/ // Version 0c74167e0eaaeb6de1c7e8219fecaabcf8386d1f diff --git a/src/lib.rs b/src/lib.rs index 0bdff24..551d54b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,6 @@ +pub use reqwest::Error as Error; +pub use reqwest::Result as Result; + pub mod consts; pub mod endpoints; @@ -44,7 +47,7 @@ mod tests { let my_future = riot_api.champion_mastery_v4().get_all_champion_masteries( consts::Region::NA, "SBM8Ubipo4ge2yj7bhEzL7yvV0C9Oc1XA2l6v5okGMA_nCw"); let val = rt.block_on(my_future).unwrap(); - println!("VAL {}: {:#?}", i, val.unwrap()); + //println!("VAL {}: {:#?}", i, val.unwrap()); } } } diff --git a/src/req/regional_requester.rs b/src/req/regional_requester.rs index e873659..8a3c6d2 100644 --- a/src/req/regional_requester.rs +++ b/src/req/regional_requester.rs @@ -1,9 +1,11 @@ use std::future::Future; use std::sync::Arc; +use log; use reqwest::{ Client, StatusCode, Url }; use tokio_timer::delay_for; +use crate::Result; use crate::riot_api_config::RiotApiConfig; use crate::consts::Region; use crate::util::InsertOnlyCHashMap; @@ -12,9 +14,6 @@ use super::RateLimit; use super::RateLimitType; pub struct RegionalRequester { - - - /// Represents the app rate limit. app_rate_limit: RateLimit, /// Represents method rate limits. @@ -39,15 +38,13 @@ impl RegionalRequester { pub fn get<'a, T: serde::de::DeserializeOwned>(self: Arc, config: &'a RiotApiConfig, client: &'a Client, method_id: &'static str, region: Region, path: String, query: Option) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { async move { let query = query.as_deref(); - let mut attempts: u8 = 0; - for _ in 0..=config.retries { - attempts += 1; - + let mut retries: u8 = 0; + loop { let method_rate_limit: Arc = self.method_rate_limits .get_or_insert_with(method_id, || RateLimit::new(RateLimitType::Method)); @@ -63,15 +60,10 @@ impl RegionalRequester { url.set_path(&*path); url.set_query(query); - let result = client.get(url) + let response = client.get(url) .header(Self::RIOT_KEY_HEADER, &*config.api_key) .send() - .await; - - let response = match result { - Err(e) => return Err(e), - Ok(r) => r, - }; + .await?; // Maybe update rate limits (based on response headers). self.app_rate_limit.on_response(&response); @@ -79,30 +71,33 @@ impl RegionalRequester { // Handle response. let status = response.status(); - // Success, return None. + log::trace!("Response {} (retried {} times).", status, retries); + // Special "none success" cases, return None. if Self::is_none_status_code(&status) { - return Ok(None); + break Ok(None); } - // Success, return a value. - if status.is_success() { - let value = response.json::().await; - return match value { - Err(e) => Err(e), - Ok(v) => Ok(Some(v)), - } - } - // Retryable. - if StatusCode::TOO_MANY_REQUESTS == status || status.is_server_error() { - continue; - } - // Failure (non-retryable). - if status.is_client_error() { - break; - } - panic!("NOT HANDLED: {}!", status); + // Handle normal success / failure cases. + match response.error_for_status_ref() { + // Success. + Ok(_) => { + let value = response.json::().await; + break value.map(|v| Some(v)); + }, + // 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()) + { + break Err(err); + } + }, + }; + + retries += 1; } - // TODO: return error. - panic!("FAILED AFTER {} ATTEMPTS!", attempts); } } diff --git a/src/riot_api.rs b/src/riot_api.rs index 9dc4ca4..c7ba653 100644 --- a/src/riot_api.rs +++ b/src/riot_api.rs @@ -1,6 +1,6 @@ use std::future::Future; -use log::*; +use log; use reqwest::Client; use crate::RiotApiConfig; @@ -20,7 +20,7 @@ pub struct RiotApi { impl RiotApi { pub fn with_config(config: RiotApiConfig) -> Self { - trace!("Creating client (TODO: configuration)."); + log::trace!("Creating client (TODO: configuration)."); Self { config: config, client: Client::new(), diff --git a/srcgen/endpoints.rs.dt b/srcgen/endpoints.rs.dt index bfa569c..f6287e6 100644 --- a/srcgen/endpoints.rs.dt +++ b/srcgen/endpoints.rs.dt @@ -15,9 +15,9 @@ pub use dto::*; use std::future::Future; use std::vec::Vec; -use reqwest; use url::form_urlencoded::Serializer; +use crate::Result; use crate::consts::Region; use crate::riot_api::RiotApi; @@ -133,7 +133,7 @@ impl<'a> {{= endpoint }}<'a> { } }} pub fn {{= method }}(&self, region: Region{{= argBuilder.join('') }}) - -> impl Future, reqwest::Error>> + 'a + -> impl Future>> + 'a { {{? queryParams.length }} let mut query_params = Serializer::new(String::new());