Skip to content

Commit

Permalink
Merge torrust#425: Improve Tracker API errors log
Browse files Browse the repository at this point in the history
ff90e9e feat: [torrust#420] ignore torrent not found when importing statistics from tracker (Jose Celano)
f975bf5 fix: [torrust#420] improve error loggin for statistics importer (Jose Celano)
70ac664 fix: [torrust#420] proper handling for tracker API responses (Jose Celano)
7620ed1 chore: disable clippy error until we fix it (Jose Celano)
35b079f refactor: [torrust#420] decouple tracker API errors from app errors (Jose Celano)

Pull request description:

  We need the specific error requesting the tracker API to include the error in the logs.

  - [x] Refactor to decouple API errors from app errors.
  - [x] Change the statistic importer log to include the exact error from the tracker API.
  - [x] When we import statistics from the tracker, don't log an error when the torrent is not found. It could have been removed because there are no peers.

ACKs for top commit:
  josecelano:
    ACK ff90e9e

Tree-SHA512: 4505ee1ec40e3fe4b01c3d3593487718afd67c9b98e0f225315d455218fa9c331b0bc964afa04d653916b9631b20b1b3ed074a13ed15436b8b9eae1761e21664
  • Loading branch information
josecelano committed Jan 4, 2024
2 parents a471a5b + ff90e9e commit 9c4b5f0
Show file tree
Hide file tree
Showing 5 changed files with 260 additions and 192 deletions.
47 changes: 41 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,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<sqlx::Error> for ServiceError {
Expand Down Expand Up @@ -228,6 +243,22 @@ impl From<DecodeTorrentFileError> for ServiceError {
}
}

impl From<TrackerAPIError> 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)]
Expand Down Expand Up @@ -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,
}
}

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
Loading

0 comments on commit 9c4b5f0

Please sign in to comment.