diff --git a/src/errors.rs b/src/errors.rs index 24a9661e..301d841c 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -6,6 +6,7 @@ use hyper::StatusCode; use crate::databases::database; use crate::models::torrent::MetadataError; +use crate::tracker::service::TrackerAPIError; use crate::utils::parse_torrent::DecodeTorrentFileError; pub type ServiceResult = Result; @@ -84,9 +85,6 @@ pub enum ServiceError { /// token invalid TokenInvalid, - #[display(fmt = "Torrent not found.")] - TorrentNotFound, - #[display(fmt = "Uploaded torrent is not valid.")] InvalidTorrentFile, @@ -120,9 +118,6 @@ pub enum ServiceError { #[display(fmt = "This torrent title has already been used.")] TorrentTitleAlreadyExists, - #[display(fmt = "Sorry, we have an error with our tracker connection.")] - TrackerOffline, - #[display(fmt = "Could not whitelist torrent.")] WhitelistingError, @@ -141,6 +136,9 @@ pub enum ServiceError { #[display(fmt = "Tag name cannot be empty.")] TagNameEmpty, + #[display(fmt = "Torrent not found.")] + TorrentNotFound, + #[display(fmt = "Category not found.")] CategoryNotFound, @@ -149,6 +147,23 @@ pub enum ServiceError { #[display(fmt = "Database error.")] DatabaseError, + + // Begin tracker errors + #[display(fmt = "Sorry, we have an error with our tracker connection.")] + TrackerOffline, + + #[display(fmt = "Tracker response error. The operation could not be performed.")] + TrackerResponseError, + + #[display(fmt = "Tracker unknown response. Unexpected response from tracker. For example, if it can be parsed.")] + TrackerUnknownResponse, + + #[display(fmt = "Torrent not found in tracker.")] + TorrentNotFoundInTracker, + + #[display(fmt = "Invalid tracker API token.")] + InvalidTrackerToken, + // End tracker errors } impl From for ServiceError { @@ -228,6 +243,22 @@ impl From for ServiceError { } } +impl From for ServiceError { + fn from(e: TrackerAPIError) -> Self { + eprintln!("{e}"); + match e { + TrackerAPIError::TrackerOffline => ServiceError::TrackerOffline, + TrackerAPIError::InternalServerError => ServiceError::TrackerResponseError, + TrackerAPIError::TorrentNotFound => ServiceError::TorrentNotFoundInTracker, + TrackerAPIError::UnexpectedResponseStatus + | TrackerAPIError::MissingResponseBody + | TrackerAPIError::FailedToParseTrackerResponse { body: _ } => ServiceError::TrackerUnknownResponse, + TrackerAPIError::CannotSaveUserKey => ServiceError::DatabaseError, + TrackerAPIError::InvalidToken => ServiceError::InvalidTrackerToken, + } + } +} + #[must_use] pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode { #[allow(clippy::match_same_arms)] @@ -276,6 +307,10 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode { ServiceError::DatabaseError => StatusCode::INTERNAL_SERVER_ERROR, ServiceError::CategoryNotFound => StatusCode::NOT_FOUND, ServiceError::TagNotFound => StatusCode::NOT_FOUND, + ServiceError::TrackerResponseError => StatusCode::INTERNAL_SERVER_ERROR, + ServiceError::TrackerUnknownResponse => StatusCode::INTERNAL_SERVER_ERROR, + ServiceError::TorrentNotFoundInTracker => StatusCode::NOT_FOUND, + ServiceError::InvalidTrackerToken => StatusCode::INTERNAL_SERVER_ERROR, } } diff --git a/src/services/torrent.rs b/src/services/torrent.rs index 0a1f6b94..73eef75d 100644 --- a/src/services/torrent.rs +++ b/src/services/torrent.rs @@ -165,7 +165,7 @@ impl Index { { // If the torrent can't be whitelisted somehow, remove the torrent from database drop(self.torrent_repository.delete(&torrent_id).await); - return Err(e); + return Err(e.into()); } // Build response @@ -304,7 +304,8 @@ impl Index { self.torrent_repository.delete(&torrent_listing.torrent_id).await?; // Remove info-hash from tracker whitelist - let _ = self + // todo: handle the error when the tracker is offline or not well configured. + let _unused = self .tracker_service .remove_info_hash_from_whitelist(info_hash.to_string()) .await; diff --git a/src/tracker/service.rs b/src/tracker/service.rs index 19ac3b95..265109d2 100644 --- a/src/tracker/service.rs +++ b/src/tracker/service.rs @@ -1,17 +1,44 @@ use std::sync::Arc; +use derive_more::{Display, Error}; use hyper::StatusCode; -use log::error; -use reqwest::Response; +use log::{debug, error}; use serde::{Deserialize, Serialize}; use super::api::{Client, ConnectionInfo}; use crate::config::Configuration; use crate::databases::database::Database; -use crate::errors::ServiceError; use crate::models::tracker_key::TrackerKey; use crate::models::user::UserId; +#[derive(Debug, Display, PartialEq, Eq, Error)] +#[allow(dead_code)] +pub enum TrackerAPIError { + #[display(fmt = "Error with tracker connection.")] + TrackerOffline, + + #[display(fmt = "Invalid token for tracker API. Check the tracker token in settings.")] + InvalidToken, + + #[display(fmt = "Tracker returned an internal server error.")] + InternalServerError, + + #[display(fmt = "Tracker returned an unexpected response status.")] + UnexpectedResponseStatus, + + #[display(fmt = "Could not save the newly generated user key into the database.")] + CannotSaveUserKey, + + #[display(fmt = "Torrent not found.")] + TorrentNotFound, + + #[display(fmt = "Expected body in tracker response, received empty body.")] + MissingResponseBody, + + #[display(fmt = "Expected body in tracker response, received empty body.")] + FailedToParseTrackerResponse { body: String }, +} + #[derive(Debug, Serialize, Deserialize, PartialEq)] pub struct TorrentInfo { pub info_hash: String, @@ -69,18 +96,39 @@ impl Service { /// /// Will return an error if the HTTP request failed (for example if the /// tracker API is offline) or if the tracker API returned an error. - pub async fn whitelist_info_hash(&self, info_hash: String) -> Result<(), ServiceError> { - let response = self.api_client.whitelist_torrent(&info_hash).await; + pub async fn whitelist_info_hash(&self, info_hash: String) -> Result<(), TrackerAPIError> { + debug!(target: "tracker-service", "add to whitelist: {info_hash}"); + + let maybe_response = self.api_client.whitelist_torrent(&info_hash).await; + + debug!(target: "tracker-service", "add to whitelist response result: {:?}", maybe_response); - match response { + match maybe_response { Ok(response) => { - if response.status().is_success() { - Ok(()) - } else { - Err(ServiceError::WhitelistingError) + let status: StatusCode = response.status(); + + let body = response.text().await.map_err(|_| { + error!(target: "tracker-service", "response without body"); + TrackerAPIError::MissingResponseBody + })?; + + match status { + StatusCode::OK => Ok(()), + StatusCode::INTERNAL_SERVER_ERROR => { + if body == "Unhandled rejection: Err { reason: \"token not valid\" }" { + Err(TrackerAPIError::InvalidToken) + } else { + error!(target: "tracker-service", "add to whitelist 500 response: status {status}, body: {body}"); + Err(TrackerAPIError::InternalServerError) + } + } + _ => { + error!(target: "tracker-service", "add to whitelist unexpected response: status {status}, body: {body}"); + Err(TrackerAPIError::UnexpectedResponseStatus) + } } } - Err(_) => Err(ServiceError::TrackerOffline), + Err(_) => Err(TrackerAPIError::TrackerOffline), } } @@ -90,18 +138,39 @@ impl Service { /// /// Will return an error if the HTTP request failed (for example if the /// tracker API is offline) or if the tracker API returned an error. - pub async fn remove_info_hash_from_whitelist(&self, info_hash: String) -> Result<(), ServiceError> { - let response = self.api_client.remove_torrent_from_whitelist(&info_hash).await; + pub async fn remove_info_hash_from_whitelist(&self, info_hash: String) -> Result<(), TrackerAPIError> { + debug!(target: "tracker-service", "remove from whitelist: {info_hash}"); - match response { + let maybe_response = self.api_client.remove_torrent_from_whitelist(&info_hash).await; + + debug!(target: "tracker-service", "remove from whitelist response result: {:?}", maybe_response); + + match maybe_response { Ok(response) => { - if response.status().is_success() { - Ok(()) - } else { - Err(ServiceError::InternalServerError) + let status: StatusCode = response.status(); + + let body = response.text().await.map_err(|_| { + error!(target: "tracker-service", "response without body"); + TrackerAPIError::MissingResponseBody + })?; + + match status { + StatusCode::OK => Ok(()), + StatusCode::INTERNAL_SERVER_ERROR => { + if body == Self::invalid_token_body() { + Err(TrackerAPIError::InvalidToken) + } else { + error!(target: "tracker-service", "remove from whitelist 500 response: status {status}, body: {body}"); + Err(TrackerAPIError::InternalServerError) + } + } + _ => { + error!(target: "tracker-service", "remove from whitelist unexpected response: status {status}, body: {body}"); + Err(TrackerAPIError::UnexpectedResponseStatus) + } } } - Err(_) => Err(ServiceError::InternalServerError), + Err(_) => Err(TrackerAPIError::TrackerOffline), } } @@ -116,14 +185,16 @@ impl Service { /// /// Will return an error if the HTTP request to get generated a new /// user tracker key failed. - pub async fn get_personal_announce_url(&self, user_id: UserId) -> Result { + pub async fn get_personal_announce_url(&self, user_id: UserId) -> Result { + debug!(target: "tracker-service", "get personal announce url for user: {user_id}"); + let tracker_key = self.database.get_user_tracker_key(user_id).await; match tracker_key { - Some(v) => Ok(self.announce_url_with_key(&v)), + Some(tracker_key) => Ok(self.announce_url_with_key(&tracker_key)), None => match self.retrieve_new_tracker_key(user_id).await { - Ok(v) => Ok(self.announce_url_with_key(&v)), - Err(_) => Err(ServiceError::TrackerOffline), + Ok(new_tracker_key) => Ok(self.announce_url_with_key(&new_tracker_key)), + Err(_) => Err(TrackerAPIError::TrackerOffline), }, } } @@ -134,141 +205,116 @@ impl Service { /// /// Will return an error if the HTTP request to get torrent info fails or /// if the response cannot be parsed. - pub async fn get_torrent_info(&self, info_hash: &str) -> Result { - let response = self - .api_client - .get_torrent_info(info_hash) - .await - .map_err(|_| ServiceError::InternalServerError)?; - - map_torrent_info_response(response).await - } - - /// It builds the announce url appending the user tracker key. - /// Eg: - fn announce_url_with_key(&self, tracker_key: &TrackerKey) -> String { - format!("{}/{}", self.tracker_url, tracker_key.key) - } - - /// Issue a new tracker key from tracker and save it in database, - /// tied to a user - async fn retrieve_new_tracker_key(&self, user_id: i64) -> Result { - // Request new tracker key from tracker - let response = self - .api_client - .retrieve_new_tracker_key(self.token_valid_seconds) - .await - .map_err(|_| ServiceError::InternalServerError)?; - - // Parse tracker key from response - let tracker_key = response - .json::() - .await - .map_err(|_| ServiceError::InternalServerError)?; - - // Add tracker key to database (tied to a user) - self.database.add_tracker_key(user_id, &tracker_key).await?; - - // return tracker key - Ok(tracker_key) - } -} - -async fn map_torrent_info_response(response: Response) -> Result { - if response.status() == StatusCode::NOT_FOUND { - return Err(ServiceError::TorrentNotFound); - } - - let body = response.text().await.map_err(|_| { - error!("Tracker API response without body"); - ServiceError::InternalServerError - })?; - - if body == "\"torrent not known\"" { - // todo: temporary fix. the service should return a 404 (StatusCode::NOT_FOUND). - return Err(ServiceError::TorrentNotFound); - } - - serde_json::from_str(&body).map_err(|e| { - error!( - "Failed to parse torrent info from tracker response. Body: {}, Error: {}", - body, e - ); - ServiceError::InternalServerError - }) -} - -#[cfg(test)] -mod tests { - - mod getting_the_torrent_info_from_the_tracker { - use hyper::{Response, StatusCode}; - - use crate::errors::ServiceError; - use crate::tracker::service::{map_torrent_info_response, TorrentInfo}; - - #[tokio::test] - async fn it_should_return_a_torrent_not_found_response_when_the_tracker_returns_the_current_torrent_not_known_response() { - let tracker_response = Response::new("\"torrent not known\""); + pub async fn get_torrent_info(&self, info_hash: &str) -> Result { + debug!(target: "tracker-service", "get torrent info: {info_hash}"); - let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err(); + let maybe_response = self.api_client.get_torrent_info(info_hash).await; - assert_eq!(result, ServiceError::TorrentNotFound); - } - - #[tokio::test] - async fn it_should_return_a_torrent_not_found_response_when_the_tracker_returns_the_future_torrent_not_known_response() { - // In the future the tracker should return a 4040 response. - // See: https://github.com/torrust/torrust-tracker/issues/144 - - let tracker_response = Response::builder().status(StatusCode::NOT_FOUND).body("").unwrap(); - - let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err(); + debug!(target: "tracker-service", "get torrent info response result: {:?}", maybe_response); - assert_eq!(result, ServiceError::TorrentNotFound); + match maybe_response { + Ok(response) => { + let status: StatusCode = response.status(); + + let body = response.text().await.map_err(|_| { + error!(target: "tracker-service", "response without body"); + TrackerAPIError::MissingResponseBody + })?; + + match status { + StatusCode::NOT_FOUND => Err(TrackerAPIError::TorrentNotFound), + StatusCode::OK => { + if body == Self::torrent_not_known_body() { + // todo: temporary fix. the service should return a 404 (StatusCode::NOT_FOUND). + return Err(TrackerAPIError::TorrentNotFound); + } + + serde_json::from_str(&body).map_err(|e| { + error!( + target: "tracker-service", "Failed to parse torrent info from tracker response. Body: {}, Error: {}", + body, e + ); + TrackerAPIError::FailedToParseTrackerResponse { body } + }) + } + StatusCode::INTERNAL_SERVER_ERROR => { + if body == Self::invalid_token_body() { + Err(TrackerAPIError::InvalidToken) + } else { + error!(target: "tracker-service", "get torrent info 500 response: status {status}, body: {body}"); + Err(TrackerAPIError::InternalServerError) + } + } + _ => { + error!(target: "tracker-service", "get torrent info unhandled response: status {status}, body: {body}"); + Err(TrackerAPIError::UnexpectedResponseStatus) + } + } + } + Err(_) => Err(TrackerAPIError::TrackerOffline), } + } - #[tokio::test] - async fn it_should_return_the_torrent_info_when_the_tracker_returns_the_torrent_info() { - let body = r#" - { - "info_hash": "4f2ae7294f2c4865c38565f92a077d1591a0dd41", - "seeders": 0, - "completed": 0, - "leechers": 0, - "peers": [] - } - "#; + /// Issue a new tracker key from tracker. + async fn retrieve_new_tracker_key(&self, user_id: i64) -> Result { + debug!(target: "tracker-service", "retrieve key: {user_id}"); - let tracker_response = Response::new(body); + let maybe_response = self.api_client.retrieve_new_tracker_key(self.token_valid_seconds).await; - let torrent_info = map_torrent_info_response(tracker_response.into()).await.unwrap(); + debug!(target: "tracker-service", "retrieve key response result: {:?}", maybe_response); - assert_eq!( - torrent_info, - TorrentInfo { - info_hash: "4f2ae7294f2c4865c38565f92a077d1591a0dd41".to_string(), - seeders: 0, - completed: 0, - leechers: 0, - peers: vec![] + match maybe_response { + Ok(response) => { + let status: StatusCode = response.status(); + + let body = response.text().await.map_err(|_| { + error!(target: "tracker-service", "response without body"); + TrackerAPIError::MissingResponseBody + })?; + + match status { + StatusCode::OK => { + // Parse tracker key from response + let tracker_key = + serde_json::from_str(&body).map_err(|_| TrackerAPIError::FailedToParseTrackerResponse { body })?; + + // Add tracker key to database (tied to a user) + self.database + .add_tracker_key(user_id, &tracker_key) + .await + .map_err(|_| TrackerAPIError::CannotSaveUserKey)?; + + Ok(tracker_key) + } + StatusCode::INTERNAL_SERVER_ERROR => { + if body == Self::invalid_token_body() { + Err(TrackerAPIError::InvalidToken) + } else { + error!(target: "tracker-service", "retrieve key 500 response: status {status}, body: {body}"); + Err(TrackerAPIError::InternalServerError) + } + } + _ => { + error!(target: "tracker-service", " retrieve key unexpected response: status {status}, body: {body}"); + Err(TrackerAPIError::UnexpectedResponseStatus) + } } - ); + } + Err(_) => Err(TrackerAPIError::TrackerOffline), } + } - #[tokio::test] - async fn it_should_return_an_internal_server_error_when_the_tracker_response_cannot_be_parsed() { - let invalid_json_body_for_torrent_info = r#" - { - "field": "value" - } - "#; - - let tracker_response = Response::new(invalid_json_body_for_torrent_info); + /// It builds the announce url appending the user tracker key. + /// Eg: + fn announce_url_with_key(&self, tracker_key: &TrackerKey) -> String { + format!("{}/{}", self.tracker_url, tracker_key.key) + } - let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err(); + fn invalid_token_body() -> String { + "Unhandled rejection: Err { reason: \"token not valid\" }".to_string() + } - assert_eq!(result, ServiceError::InternalServerError); - } + fn torrent_not_known_body() -> String { + "\"torrent not known\"".to_string() } } diff --git a/src/tracker/statistics_importer.rs b/src/tracker/statistics_importer.rs index 128cce12..91c649df 100644 --- a/src/tracker/statistics_importer.rs +++ b/src/tracker/statistics_importer.rs @@ -2,10 +2,9 @@ use std::sync::Arc; use log::{error, info}; -use super::service::{Service, TorrentInfo}; +use super::service::{Service, TorrentInfo, TrackerAPIError}; use crate::config::Configuration; use crate::databases::database::{self, Database}; -use crate::errors::ServiceError; pub struct StatisticsImporter { database: Arc>, @@ -39,22 +38,14 @@ impl StatisticsImporter { let ret = self.import_torrent_statistics(torrent.torrent_id, &torrent.info_hash).await; - // code-review: should we treat differently for each case?. The - // tracker API could be temporarily offline, or there could be a - // tracker misconfiguration. - // - // This is the log when the torrent is not found in the tracker: - // - // ``` - // 2023-05-09T13:31:24.497465723+00:00 [torrust_index::tracker::statistics_importer][ERROR] Error updating torrent tracker stats for torrent with id 140: TorrentNotFound - // ``` - if let Some(err) = ret.err() { - let message = format!( - "Error updating torrent tracker stats for torrent with id {}: {:?}", - torrent.torrent_id, err - ); - error!("{}", message); + if err != TrackerAPIError::TorrentNotFound { + let message = format!( + "Error updating torrent tracker stats for torrent. Torrent: id {}; infohash {}. Error: {:?}", + torrent.torrent_id, torrent.info_hash, err + ); + error!(target: "statistics_importer", "{}", message); + } } } @@ -67,17 +58,20 @@ impl StatisticsImporter { /// /// Will return an error if the HTTP request failed or the torrent is not /// found. - pub async fn import_torrent_statistics(&self, torrent_id: i64, info_hash: &str) -> Result { - if let Ok(torrent_info) = self.tracker_service.get_torrent_info(info_hash).await { - drop( - self.database - .update_tracker_info(torrent_id, &self.tracker_url, torrent_info.seeders, torrent_info.leechers) - .await, - ); - Ok(torrent_info) - } else { - drop(self.database.update_tracker_info(torrent_id, &self.tracker_url, 0, 0).await); - Err(ServiceError::TorrentNotFound) + pub async fn import_torrent_statistics(&self, torrent_id: i64, info_hash: &str) -> Result { + match self.tracker_service.get_torrent_info(info_hash).await { + Ok(torrent_info) => { + drop( + self.database + .update_tracker_info(torrent_id, &self.tracker_url, torrent_info.seeders, torrent_info.leechers) + .await, + ); + Ok(torrent_info) + } + Err(err) => { + drop(self.database.update_tracker_info(torrent_id, &self.tracker_url, 0, 0).await); + Err(err) + } } } } diff --git a/tests/common/contexts/torrent/responses.rs b/tests/common/contexts/torrent/responses.rs index a6fd4946..a776623f 100644 --- a/tests/common/contexts/torrent/responses.rs +++ b/tests/common/contexts/torrent/responses.rs @@ -74,17 +74,9 @@ pub struct TorrentDetails { pub encoding: Option, } -#[rustversion::stable] -#[derive(Deserialize, PartialEq, Debug)] -pub struct Category { - pub category_id: CategoryId, // todo: rename to `id` - pub name: String, - pub num_torrents: u64, -} - -#[rustversion::nightly] -#[derive(Deserialize, PartialEq, Debug)] +#[allow(unknown_lints)] #[allow(clippy::struct_field_names)] +#[derive(Deserialize, PartialEq, Debug)] pub struct Category { pub category_id: CategoryId, // todo: rename to `id` pub name: String,