Skip to content

Commit

Permalink
refactor: [#420] decouple tracker API errors from app errors
Browse files Browse the repository at this point in the history
We need the specifix error requesting the tracker API to include the
error in the logs.
  • Loading branch information
josecelano committed Jan 3, 2024
1 parent a471a5b commit 35b079f
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 59 deletions.
43 changes: 37 additions & 6 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<V> = Result<V, ServiceError>;
Expand Down Expand Up @@ -84,9 +85,6 @@ pub enum ServiceError {
/// token invalid
TokenInvalid,

#[display(fmt = "Torrent not found.")]
TorrentNotFound,

#[display(fmt = "Uploaded torrent is not valid.")]
InvalidTorrentFile,

Expand Down Expand Up @@ -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,

Expand All @@ -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,

Expand All @@ -149,6 +147,19 @@ pub enum ServiceError {

#[display(fmt = "Database error.")]
DatabaseError,

// 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,
}

impl From<sqlx::Error> for ServiceError {
Expand Down Expand Up @@ -228,6 +239,23 @@ impl From<DecodeTorrentFileError> for ServiceError {
}
}

impl From<TrackerAPIError> for ServiceError {
fn from(e: TrackerAPIError) -> Self {
eprintln!("{e}");
match e {
TrackerAPIError::TrackerOffline => ServiceError::TrackerOffline,
TrackerAPIError::AddToWhitelistError
| TrackerAPIError::RemoveFromWhitelistError
| TrackerAPIError::RetrieveUserKeyError => ServiceError::TrackerResponseError,
TrackerAPIError::TorrentNotFound => ServiceError::TorrentNotFoundInTracker,
TrackerAPIError::MissingResponseBody | TrackerAPIError::FailedToParseTrackerResponse { body: _ } => {
ServiceError::TrackerUnknownResponse
}
TrackerAPIError::CannotSaveUserKey => ServiceError::DatabaseError,
}
}
}

#[must_use]
pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
#[allow(clippy::match_same_arms)]
Expand Down Expand Up @@ -276,6 +304,9 @@ 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,
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/services/torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
147 changes: 96 additions & 51 deletions src/tracker/service.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use derive_more::{Display, Error};
use hyper::StatusCode;
use log::error;
use reqwest::Response;
Expand All @@ -8,10 +9,37 @@ 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 = "Could not whitelist torrent.")]
AddToWhitelistError,

#[display(fmt = "Could not remove torrent from whitelist.")]
RemoveFromWhitelistError,

#[display(fmt = "Could not retrieve a new user key.")]
RetrieveUserKeyError,

#[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,
Expand Down Expand Up @@ -69,18 +97,18 @@ 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> {
pub async fn whitelist_info_hash(&self, info_hash: String) -> Result<(), TrackerAPIError> {
let response = self.api_client.whitelist_torrent(&info_hash).await;

match response {
Ok(response) => {
if response.status().is_success() {
Ok(())
} else {
Err(ServiceError::WhitelistingError)
Err(TrackerAPIError::AddToWhitelistError)
}
}
Err(_) => Err(ServiceError::TrackerOffline),
Err(_) => Err(TrackerAPIError::TrackerOffline),
}
}

Expand All @@ -90,18 +118,18 @@ 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> {
pub async fn remove_info_hash_from_whitelist(&self, info_hash: String) -> Result<(), TrackerAPIError> {
let response = self.api_client.remove_torrent_from_whitelist(&info_hash).await;

match response {
Ok(response) => {
if response.status().is_success() {
Ok(())
} else {
Err(ServiceError::InternalServerError)
Err(TrackerAPIError::RemoveFromWhitelistError)
}
}
Err(_) => Err(ServiceError::InternalServerError),
Err(_) => Err(TrackerAPIError::TrackerOffline),
}
}

Expand All @@ -116,14 +144,14 @@ 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<String, ServiceError> {
pub async fn get_personal_announce_url(&self, user_id: UserId) -> Result<String, TrackerAPIError> {
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),
},
}
}
Expand All @@ -134,14 +162,19 @@ 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<TorrentInfo, ServiceError> {
let response = self
.api_client
.get_torrent_info(info_hash)
.await
.map_err(|_| ServiceError::InternalServerError)?;

map_torrent_info_response(response).await
pub async fn get_torrent_info(&self, info_hash: &str) -> Result<TorrentInfo, TrackerAPIError> {
let response = self.api_client.get_torrent_info(info_hash).await;

match response {
Ok(response) => {
if response.status().is_success() {
map_torrent_info_response(response).await
} else {
Err(TrackerAPIError::RemoveFromWhitelistError)
}
}
Err(_) => Err(TrackerAPIError::TrackerOffline),
}
}

/// It builds the announce url appending the user tracker key.
Expand All @@ -150,51 +183,59 @@ impl Service {
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<TrackerKey, ServiceError> {
// 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::<TrackerKey>()
.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)
/// Issue a new tracker key from tracker.
async fn retrieve_new_tracker_key(&self, user_id: i64) -> Result<TrackerKey, TrackerAPIError> {
let response = self.api_client.retrieve_new_tracker_key(self.token_valid_seconds).await;

match response {
Ok(response) => {
if response.status().is_success() {
let body = response.text().await.map_err(|_| {
error!("Tracker API response without body");
TrackerAPIError::MissingResponseBody
})?;

// 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)
} else {
Err(TrackerAPIError::RetrieveUserKeyError)
}
}
Err(_) => Err(TrackerAPIError::TrackerOffline),
}
}
}

async fn map_torrent_info_response(response: Response) -> Result<TorrentInfo, ServiceError> {
async fn map_torrent_info_response(response: Response) -> Result<TorrentInfo, TrackerAPIError> {
if response.status() == StatusCode::NOT_FOUND {
return Err(ServiceError::TorrentNotFound);
return Err(TrackerAPIError::TorrentNotFound);
}

let body = response.text().await.map_err(|_| {
error!("Tracker API response without body");
ServiceError::InternalServerError
TrackerAPIError::MissingResponseBody
})?;

if body == "\"torrent not known\"" {
// todo: temporary fix. the service should return a 404 (StatusCode::NOT_FOUND).
return Err(ServiceError::TorrentNotFound);
return Err(TrackerAPIError::TorrentNotFound);
}

serde_json::from_str(&body).map_err(|e| {
error!(
"Failed to parse torrent info from tracker response. Body: {}, Error: {}",
body, e
);
ServiceError::InternalServerError
TrackerAPIError::FailedToParseTrackerResponse { body }
})
}

Expand All @@ -204,16 +245,15 @@ 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};
use crate::tracker::service::{map_torrent_info_response, TorrentInfo, TrackerAPIError};

#[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\"");

let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err();

assert_eq!(result, ServiceError::TorrentNotFound);
assert_eq!(result, TrackerAPIError::TorrentNotFound);
}

#[tokio::test]
Expand All @@ -225,7 +265,7 @@ mod tests {

let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err();

assert_eq!(result, ServiceError::TorrentNotFound);
assert_eq!(result, TrackerAPIError::TorrentNotFound);
}

#[tokio::test]
Expand Down Expand Up @@ -266,9 +306,14 @@ mod tests {

let tracker_response = Response::new(invalid_json_body_for_torrent_info);

let result = map_torrent_info_response(tracker_response.into()).await.unwrap_err();
let err = map_torrent_info_response(tracker_response.into()).await.unwrap_err();

assert_eq!(result, ServiceError::InternalServerError);
assert_eq!(
err,
TrackerAPIError::FailedToParseTrackerResponse {
body: invalid_json_body_for_torrent_info.to_string()
}
);
}
}
}

0 comments on commit 35b079f

Please sign in to comment.