refactor: [#680] http return errors instead of panicking

hungfnt · hungfnt · commit 895efe9c4154 · 2024-05-02T22:08:46.000+07:00
diff --git 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::<Announce>(&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
@@ -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
@@ -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<Self> {
+        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<Self> {
+        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<Self> {
+        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<Response> {
         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<Response> {
         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<Response> {
         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<Response> {
         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<Response> {
+        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<Response> {
+        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 {
