Skip to content

Commit

Permalink
refactor: [#680] http return errors instead of panicking
Browse files Browse the repository at this point in the history
  • Loading branch information
hungfnt committed May 2, 2024
1 parent 90c7780 commit 895efe9
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 41 deletions.
22 changes: 19 additions & 3 deletions src/console/clients/checker/checks/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions src/console/clients/http/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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));
Expand Down
72 changes: 39 additions & 33 deletions src/shared/bit_torrent/tracker/http/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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 {

Check warning on line 34 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L32-L34

Added lines #L32 - L34 were not covered by tests
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 {

Check warning on line 48 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L46-L48

Added lines #L46 - L48 were not covered by tests
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 {

Check warning on line 60 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L58-L60

Added lines #L58 - L60 were not covered by tests
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> {

Check warning on line 68 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L68

Added line #L68 was not covered by tests
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> {

Check warning on line 73 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L73

Added line #L73 was not covered by tests
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> {

Check warning on line 78 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L78

Added line #L78 was not covered by tests
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> {

Check warning on line 84 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L84

Added line #L84 was not covered by tests
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> {

Check warning on line 91 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L91

Added line #L91 was not covered by tests
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> {

Check warning on line 101 in src/shared/bit_torrent/tracker/http/client/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/shared/bit_torrent/tracker/http/client/mod.rs#L101

Added line #L101 was not covered by tests
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 {
Expand Down

0 comments on commit 895efe9

Please sign in to comment.