refactor: [torrust#1318] return error instead of response from http scrape handler

Author: josecelano

packages/axum-http-tracker-server/src/v1/handlers/scrape.rs

@@ -6,6 +6,7 @@ use std::sync::Arc;
 
 use axum::extract::State;
 use axum::response::{IntoResponse, Response};
+use bittorrent_http_tracker_core::services::scrape::HttpScrapeError;
 use bittorrent_http_tracker_protocol::v1::requests::scrape::Scrape;
 use bittorrent_http_tracker_protocol::v1::responses;
 use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
@@ -101,7 +102,12 @@ async fn handle(
     .await
     {
         Ok(scrape_data) => scrape_data,
-        Err(error) => return (StatusCode::OK, error.write()).into_response(),
+        Err(error) => {
+            let error_response = responses::error::Error {
+                failure_reason: error.to_string(),
+            };
+            return (StatusCode::OK, error_response.write()).into_response();
+        }
     };
 
     build_response(scrape_data)
@@ -116,7 +122,7 @@ async fn handle_scrape(
     scrape_request: &Scrape,
     client_ip_sources: &ClientIpSources,
     maybe_key: Option<Key>,
-) -> Result<ScrapeData, responses::error::Error> {
+) -> Result<ScrapeData, HttpScrapeError> {
     bittorrent_http_tracker_core::services::scrape::handle_scrape(
         core_config,
         scrape_handler,
@@ -316,6 +322,7 @@ mod tests {
 
     mod with_tracker_on_reverse_proxy {
 
+        use bittorrent_http_tracker_protocol::v1::responses;
         use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
 
         use super::{initialize_tracker_on_reverse_proxy, sample_scrape_request};
@@ -343,15 +350,20 @@ mod tests {
             .await
             .unwrap_err();
 
+            let error_response = responses::error::Error {
+                failure_reason: response.to_string(),
+            };
+
             assert_error_response(
-                &response,
+                &error_response,
                 "Error resolving peer IP: missing or invalid the right most X-Forwarded-For IP",
             );
         }
     }
 
     mod with_tracker_not_on_reverse_proxy {
 
+        use bittorrent_http_tracker_protocol::v1::responses;
         use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
 
         use super::{initialize_tracker_not_on_reverse_proxy, sample_scrape_request};
@@ -379,8 +391,12 @@ mod tests {
             .await
             .unwrap_err();
 
+            let error_response = responses::error::Error {
+                failure_reason: response.to_string(),
+            };
+
             assert_error_response(
-                &response,
+                &error_response,
                 "Error resolving peer IP: cannot get the client IP from the connection info",
             );
         }

packages/http-tracker-core/src/services/scrape.rs

@@ -11,17 +11,67 @@ use std::net::IpAddr;
 use std::sync::Arc;
 
 use bittorrent_http_tracker_protocol::v1::requests::scrape::Scrape;
-use bittorrent_http_tracker_protocol::v1::responses;
-use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::{self, ClientIpSources};
+use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::{self, ClientIpSources, PeerIpResolutionError};
 use bittorrent_primitives::info_hash::InfoHash;
 use bittorrent_tracker_core::authentication::service::AuthenticationService;
-use bittorrent_tracker_core::authentication::Key;
+use bittorrent_tracker_core::authentication::{self, Key};
+use bittorrent_tracker_core::error::{ScrapeError, TrackerCoreError, WhitelistError};
 use bittorrent_tracker_core::scrape_handler::ScrapeHandler;
 use torrust_tracker_configuration::Core;
 use torrust_tracker_primitives::core::ScrapeData;
 
 use crate::statistics;
 
+/// Errors related to announce requests.
+#[derive(thiserror::Error, Debug, Clone)]
+pub enum HttpScrapeError {
+    #[error("Error resolving peer IP: {source}")]
+    PeerIpResolutionError { source: PeerIpResolutionError },
+
+    #[error("Tracker core error: {source}")]
+    TrackerCoreError { source: TrackerCoreError },
+}
+
+impl From<PeerIpResolutionError> for HttpScrapeError {
+    fn from(peer_ip_resolution_error: PeerIpResolutionError) -> Self {
+        Self::PeerIpResolutionError {
+            source: peer_ip_resolution_error,
+        }
+    }
+}
+
+impl From<TrackerCoreError> for HttpScrapeError {
+    fn from(tracker_core_error: TrackerCoreError) -> Self {
+        Self::TrackerCoreError {
+            source: tracker_core_error,
+        }
+    }
+}
+
+impl From<ScrapeError> for HttpScrapeError {
+    fn from(announce_error: ScrapeError) -> Self {
+        Self::TrackerCoreError {
+            source: announce_error.into(),
+        }
+    }
+}
+
+impl From<WhitelistError> for HttpScrapeError {
+    fn from(whitelist_error: WhitelistError) -> Self {
+        Self::TrackerCoreError {
+            source: whitelist_error.into(),
+        }
+    }
+}
+
+impl From<authentication::key::Error> for HttpScrapeError {
+    fn from(whitelist_error: authentication::key::Error) -> Self {
+        Self::TrackerCoreError {
+            source: whitelist_error.into(),
+        }
+    }
+}
+
 /// The HTTP tracker `scrape` service.
 ///
 /// The service sends an statistics event that increments:
@@ -47,7 +97,7 @@ pub async fn handle_scrape(
     scrape_request: &Scrape,
     client_ip_sources: &ClientIpSources,
     maybe_key: Option<Key>,
-) -> Result<ScrapeData, responses::error::Error> {
+) -> Result<ScrapeData, HttpScrapeError> {
     // Authentication
     let return_fake_scrape_data = if core_config.private {
         match maybe_key {
@@ -66,7 +116,7 @@ pub async fn handle_scrape(
 
     let peer_ip = match peer_ip_resolver::invoke(core_config.net.on_reverse_proxy, client_ip_sources) {
         Ok(peer_ip) => peer_ip,
-        Err(error) => return Err(responses::error::Error::from(error)),
+        Err(error) => return Err(error.into()),
     };
 
     if return_fake_scrape_data {

packages/tracker-core/src/error.rs

@@ -23,6 +23,10 @@ pub enum TrackerCoreError {
     #[error("Tracker core announce error: {source}")]
     AnnounceError { source: AnnounceError },
 
+    /// Error returned when there was an error with the tracker core scrape handler.
+    #[error("Tracker core scrape error: {source}")]
+    ScrapeError { source: ScrapeError },
+
     /// Error returned when there was an error with the tracker core whitelist.
     #[error("Tracker core whitelist error: {source}")]
     WhitelistError { source: WhitelistError },
@@ -38,6 +42,12 @@ impl From<AnnounceError> for TrackerCoreError {
     }
 }
 
+impl From<ScrapeError> for TrackerCoreError {
+    fn from(scrape_error: ScrapeError) -> Self {
+        Self::ScrapeError { source: scrape_error }
+    }
+}
+
 impl From<WhitelistError> for TrackerCoreError {
     fn from(whitelist_error: WhitelistError) -> Self {
         Self::WhitelistError { source: whitelist_error }