From 895efe9c4154a6883008e9cf26592f3fed38602d Mon Sep 17 00:00:00 2001 From: ngthhu Date: Thu, 2 May 2024 16:02:42 +0700 Subject: [PATCH] refactor: [#680] http return errors instead of panicking --- src/console/clients/checker/checks/http.rs | 22 +++++- src/console/clients/http/app.rs | 10 +-- .../bit_torrent/tracker/http/client/mod.rs | 72 ++++++++++--------- 3 files changed, 63 insertions(+), 41 deletions(-) diff --git a/src/console/clients/checker/checks/http.rs b/src/console/clients/checker/checks/http.rs index e0b14b48..e526b5e5 100644 --- a/src/console/clients/checker/checks/http.rs +++ b/src/console/clients/checker/checks/http.rs @@ -61,9 +61,19 @@ async fn check_http_announce(tracker_url: &Url) -> Result<(), CheckError> { // We should change the client to catch that error and return a `CheckError`. // Otherwise the checking process will stop. The idea is to process all checks // and return a final report. - let response = Client::new(tracker_url.clone()) + let Ok(client) = Client::new(tracker_url.clone()) else { + return Err(CheckError::HttpError { + url: (tracker_url.to_owned()), + }); + }; + let Ok(response) = client .announce(&QueryBuilder::with_default_values().with_info_hash(&info_hash).query()) - .await; + .await + else { + return Err(CheckError::HttpError { + url: (tracker_url.to_owned()), + }); + }; if let Ok(body) = response.bytes().await { if let Ok(_announce_response) = serde_bencode::from_bytes::(&body) { @@ -89,7 +99,13 @@ async fn check_http_scrape(url: &Url) -> Result<(), CheckError> { // We should change the client to catch that error and return a `CheckError`. // Otherwise the checking process will stop. The idea is to process all checks // and return a final report. - let response = Client::new(url.clone()).scrape(&query).await; + + let Ok(client) = Client::new(url.clone()) else { + return Err(CheckError::HttpError { url: (url.to_owned()) }); + }; + let Ok(response) = client.scrape(&query).await else { + return Err(CheckError::HttpError { url: (url.to_owned()) }); + }; if let Ok(body) = response.bytes().await { if let Ok(_scrape_response) = scrape::Response::try_from_bencoded(&body) { diff --git a/src/console/clients/http/app.rs b/src/console/clients/http/app.rs index 511fb662..8fc9db0c 100644 --- a/src/console/clients/http/app.rs +++ b/src/console/clients/http/app.rs @@ -64,11 +64,11 @@ async fn announce_command(tracker_url: String, info_hash: String) -> anyhow::Res let info_hash = InfoHash::from_str(&info_hash).expect("Invalid infohash. Example infohash: `9c38422213e30bff212b30c360d26f9a02136422`"); - let response = Client::new(base_url) + let response = Client::new(base_url)? .announce(&QueryBuilder::with_default_values().with_info_hash(&info_hash).query()) - .await; + .await?; - let body = response.bytes().await.unwrap(); + let body = response.bytes().await?; let announce_response: Announce = serde_bencode::from_bytes(&body) .unwrap_or_else(|_| panic!("response body should be a valid announce response, got: \"{:#?}\"", &body)); @@ -85,9 +85,9 @@ async fn scrape_command(tracker_url: &str, info_hashes: &[String]) -> anyhow::Re let query = requests::scrape::Query::try_from(info_hashes).context("failed to parse infohashes")?; - let response = Client::new(base_url).scrape(&query).await; + let response = Client::new(base_url)?.scrape(&query).await?; - let body = response.bytes().await.unwrap(); + let body = response.bytes().await?; let scrape_response = scrape::Response::try_from_bencoded(&body) .unwrap_or_else(|_| panic!("response body should be a valid scrape response, got: \"{:#?}\"", &body)); diff --git a/src/shared/bit_torrent/tracker/http/client/mod.rs b/src/shared/bit_torrent/tracker/http/client/mod.rs index a75b0fec..f5b1b331 100644 --- a/src/shared/bit_torrent/tracker/http/client/mod.rs +++ b/src/shared/bit_torrent/tracker/http/client/mod.rs @@ -3,6 +3,7 @@ pub mod responses; use std::net::IpAddr; +use anyhow::{anyhow, Result}; use requests::announce::{self, Query}; use requests::scrape; use reqwest::{Client as ReqwestClient, Response, Url}; @@ -25,78 +26,83 @@ pub struct Client { /// base url path query /// ``` impl Client { - /// # Panics + /// # Errors /// /// This method fails if the client builder fails. - #[must_use] - pub fn new(base_url: Url) -> Self { - Self { + pub fn new(base_url: Url) -> Result { + let reqwest = reqwest::Client::builder().build()?; + Ok(Self { base_url, - reqwest: reqwest::Client::builder().build().unwrap(), + reqwest, key: None, - } + }) } /// Creates the new client binding it to an specific local address. /// - /// # Panics + /// # Errors /// /// This method fails if the client builder fails. - #[must_use] - pub fn bind(base_url: Url, local_address: IpAddr) -> Self { - Self { + pub fn bind(base_url: Url, local_address: IpAddr) -> Result { + let reqwest = reqwest::Client::builder().local_address(local_address).build()?; + Ok(Self { base_url, - reqwest: reqwest::Client::builder().local_address(local_address).build().unwrap(), + reqwest, key: None, - } + }) } - /// # Panics + /// # Errors /// /// This method fails if the client builder fails. - #[must_use] - pub fn authenticated(base_url: Url, key: Key) -> Self { - Self { + pub fn authenticated(base_url: Url, key: Key) -> Result { + let reqwest = reqwest::Client::builder().build()?; + Ok(Self { base_url, - reqwest: reqwest::Client::builder().build().unwrap(), + reqwest, key: Some(key), - } + }) } - pub async fn announce(&self, query: &announce::Query) -> Response { + /// # Errors + pub async fn announce(&self, query: &announce::Query) -> Result { self.get(&self.build_announce_path_and_query(query)).await } - pub async fn scrape(&self, query: &scrape::Query) -> Response { + /// # Errors + pub async fn scrape(&self, query: &scrape::Query) -> Result { self.get(&self.build_scrape_path_and_query(query)).await } - pub async fn announce_with_header(&self, query: &Query, key: &str, value: &str) -> Response { + /// # Errors + pub async fn announce_with_header(&self, query: &Query, key: &str, value: &str) -> Result { self.get_with_header(&self.build_announce_path_and_query(query), key, value) .await } - pub async fn health_check(&self) -> Response { + /// # Errors + pub async fn health_check(&self) -> Result { self.get(&self.build_path("health_check")).await } - /// # Panics + /// # Errors /// /// This method fails if there was an error while sending request. - pub async fn get(&self, path: &str) -> Response { - self.reqwest.get(self.build_url(path)).send().await.unwrap() + pub async fn get(&self, path: &str) -> Result { + match self.reqwest.get(self.build_url(path)).send().await { + Ok(response) => Ok(response), + Err(err) => Err(anyhow!("{err}")), + } } - /// # Panics + /// # Errors /// /// This method fails if there was an error while sending request. - pub async fn get_with_header(&self, path: &str, key: &str, value: &str) -> Response { - self.reqwest - .get(self.build_url(path)) - .header(key, value) - .send() - .await - .unwrap() + pub async fn get_with_header(&self, path: &str, key: &str, value: &str) -> Result { + match self.reqwest.get(self.build_url(path)).header(key, value).send().await { + Ok(response) => Ok(response), + Err(err) => Err(anyhow!("{err}")), + } } fn build_announce_path_and_query(&self, query: &announce::Query) -> String {