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

Author: josecelano

Cargo.lock

@@ -7,6 +7,7 @@ use std::sync::Arc;
 use aquatic_udp_protocol::AnnounceEvent;
 use axum::extract::State;
 use axum::response::{IntoResponse, Response};
+use bittorrent_http_tracker_core::services::announce::HttpAnnounceError;
 use bittorrent_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event};
 use bittorrent_http_tracker_protocol::v1::responses::{self};
 use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
@@ -111,7 +112,12 @@ async fn handle(
     .await
     {
         Ok(announce_data) => announce_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(announce_request, announce_data)
 }
@@ -126,7 +132,7 @@ async fn handle_announce(
     announce_request: &Announce,
     client_ip_sources: &ClientIpSources,
     maybe_key: Option<Key>,
-) -> Result<AnnounceData, responses::error::Error> {
+) -> Result<AnnounceData, HttpAnnounceError> {
     bittorrent_http_tracker_core::services::announce::handle_announce(
         &core_config.clone(),
         &announce_handler.clone(),
@@ -290,6 +296,7 @@ mod tests {
 
         use std::str::FromStr;
 
+        use bittorrent_http_tracker_protocol::v1::responses;
         use bittorrent_tracker_core::authentication;
 
         use super::{initialize_private_tracker, sample_announce_request, sample_client_ip_sources};
@@ -315,7 +322,14 @@ mod tests {
             .await
             .unwrap_err();
 
-            assert_error_response(&response, "Tracker authentication error: Missing authentication key");
+            let error_response = responses::error::Error {
+                failure_reason: response.to_string(),
+            };
+
+            assert_error_response(
+                &error_response,
+                "Tracker core error: Tracker core authentication error: Missing authentication key",
+            );
         }
 
         #[tokio::test]
@@ -339,15 +353,21 @@ mod tests {
             .await
             .unwrap_err();
 
+            let error_response = responses::error::Error {
+                failure_reason: response.to_string(),
+            };
+
             assert_error_response(
-                &response,
-                "Tracker authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
+                &error_response,
+                "Tracker core error: Tracker core authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
             );
         }
     }
 
     mod with_tracker_in_listed_mode {
 
+        use bittorrent_http_tracker_protocol::v1::responses;
+
         use super::{initialize_listed_tracker, sample_announce_request, sample_client_ip_sources};
         use crate::v1::handlers::announce::handle_announce;
         use crate::v1::handlers::announce::tests::assert_error_response;
@@ -371,10 +391,14 @@ mod tests {
             .await
             .unwrap_err();
 
+            let error_response = responses::error::Error {
+                failure_reason: response.to_string(),
+            };
+
             assert_error_response(
-                &response,
+                &error_response,
                 &format!(
-                    "Tracker whitelist error: The torrent: {}, is not whitelisted",
+                    "Tracker core error: Tracker core whitelist error: The torrent: {}, is not whitelisted",
                     announce_request.info_hash
                 ),
             );
@@ -383,6 +407,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_announce_request};
@@ -411,15 +436,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_announce_request};
@@ -448,8 +478,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/axum-http-tracker-server/src/v1/handlers/announce.rs

@@ -147,3 +147,13 @@ pub async fn assert_authentication_error_response(response: Response) {
         Location::caller(),
     );
 }
+
+pub async fn assert_tracker_core_authentication_error_response(response: Response) {
+    assert_eq!(response.status(), 200);
+
+    assert_bencoded_error(
+        &response.text().await.unwrap(),
+        "Tracker core error: Tracker core authentication error",
+        Location::caller(),
+    );
+}

packages/axum-http-tracker-server/tests/server/asserts.rs

@@ -1448,7 +1448,9 @@ mod configured_as_private {
         use torrust_axum_http_tracker_server::environment::Started;
         use torrust_tracker_test_helpers::{configuration, logging};
 
-        use crate::server::asserts::{assert_authentication_error_response, assert_is_announce_response};
+        use crate::server::asserts::{
+            assert_authentication_error_response, assert_is_announce_response, assert_tracker_core_authentication_error_response,
+        };
         use crate::server::client::Client;
         use crate::server::requests::announce::QueryBuilder;
 
@@ -1487,7 +1489,7 @@ mod configured_as_private {
                 .announce(&QueryBuilder::default().with_info_hash(&info_hash).query())
                 .await;
 
-            assert_authentication_error_response(response).await;
+            assert_tracker_core_authentication_error_response(response).await;
 
             env.stop().await;
         }
@@ -1522,7 +1524,7 @@ mod configured_as_private {
                 .announce(&QueryBuilder::default().query())
                 .await;
 
-            assert_authentication_error_response(response).await;
+            assert_tracker_core_authentication_error_response(response).await;
 
             env.stop().await;
         }

packages/axum-http-tracker-server/tests/server/v1/contract.rs

@@ -36,7 +36,7 @@ pub struct ClientIpSources {
 }
 
 /// The error that can occur when resolving the peer IP.
-#[derive(Error, Debug)]
+#[derive(Error, Debug, Clone)]
 pub enum PeerIpResolutionError {
     /// The peer IP cannot be obtained because the tracker is configured as a
     /// reverse proxy but the `X-Forwarded-For` HTTP header is missing or

packages/http-protocol/src/v1/services/peer_ip_resolver.rs

@@ -19,6 +19,7 @@ bittorrent-http-tracker-protocol = { version = "3.0.0-develop", path = "../http-
 bittorrent-primitives = "0.1.0"
 bittorrent-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" }
 futures = "0"
+thiserror = "2"
 tokio = { version = "1", features = ["macros", "net", "rt-multi-thread", "signal", "sync"] }
 torrust-tracker-configuration = { version = "3.0.0-develop", path = "../configuration" }
 torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" }

packages/http-tracker-core/Cargo.toml

@@ -12,17 +12,67 @@ use std::panic::Location;
 use std::sync::Arc;
 
 use bittorrent_http_tracker_protocol::v1::requests::announce::{peer_from_request, Announce};
-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_tracker_core::announce_handler::{AnnounceHandler, PeersWanted};
 use bittorrent_tracker_core::authentication::service::AuthenticationService;
 use bittorrent_tracker_core::authentication::{self, Key};
+use bittorrent_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistError};
 use bittorrent_tracker_core::whitelist;
 use torrust_tracker_configuration::Core;
 use torrust_tracker_primitives::core::AnnounceData;
 
 use crate::statistics;
 
+/// Errors related to announce requests.
+#[derive(thiserror::Error, Debug, Clone)]
+pub enum HttpAnnounceError {
+    #[error("Error resolving peer IP: {source}")]
+    PeerIpResolutionError { source: PeerIpResolutionError },
+
+    #[error("Tracker core error: {source}")]
+    TrackerCoreError { source: TrackerCoreError },
+}
+
+impl From<PeerIpResolutionError> for HttpAnnounceError {
+    fn from(peer_ip_resolution_error: PeerIpResolutionError) -> Self {
+        Self::PeerIpResolutionError {
+            source: peer_ip_resolution_error,
+        }
+    }
+}
+
+impl From<TrackerCoreError> for HttpAnnounceError {
+    fn from(tracker_core_error: TrackerCoreError) -> Self {
+        Self::TrackerCoreError {
+            source: tracker_core_error,
+        }
+    }
+}
+
+impl From<AnnounceError> for HttpAnnounceError {
+    fn from(announce_error: AnnounceError) -> Self {
+        Self::TrackerCoreError {
+            source: announce_error.into(),
+        }
+    }
+}
+
+impl From<WhitelistError> for HttpAnnounceError {
+    fn from(whitelist_error: WhitelistError) -> Self {
+        Self::TrackerCoreError {
+            source: whitelist_error.into(),
+        }
+    }
+}
+
+impl From<authentication::key::Error> for HttpAnnounceError {
+    fn from(whitelist_error: authentication::key::Error) -> Self {
+        Self::TrackerCoreError {
+            source: whitelist_error.into(),
+        }
+    }
+}
+
 /// The HTTP tracker `announce` service.
 ///
 /// The service sends an statistics event that increments:
@@ -50,7 +100,7 @@ pub async fn handle_announce(
     announce_request: &Announce,
     client_ip_sources: &ClientIpSources,
     maybe_key: Option<Key>,
-) -> Result<AnnounceData, responses::error::Error> {
+) -> Result<AnnounceData, HttpAnnounceError> {
     // Authentication
     if core_config.private {
         match maybe_key {
@@ -59,22 +109,23 @@ pub async fn handle_announce(
                 Err(error) => return Err(error.into()),
             },
             None => {
-                return Err(responses::error::Error::from(authentication::key::Error::MissingAuthKey {
+                return Err(authentication::key::Error::MissingAuthKey {
                     location: Location::caller(),
-                }))
+                }
+                .into())
             }
         }
     }
 
     // Authorization
     match whitelist_authorization.authorize(&announce_request.info_hash).await {
         Ok(()) => (),
-        Err(error) => return Err(responses::error::Error::from(error)),
+        Err(error) => return Err(error.into()),
     }
 
     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()),
     };
 
     let mut peer = peer_from_request(announce_request, &peer_ip);

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

@@ -166,7 +166,7 @@ pub fn verify_key_expiration(auth_key: &PeerKey) -> Result<(), Error> {
 
 /// Verification error. Error returned when an [`PeerKey`] cannot be
 /// verified with the [`crate::authentication::key::verify_key_expiration`] function.
-#[derive(Debug, Error)]
+#[derive(Debug, Error, Clone)]
 #[allow(dead_code)]
 pub enum Error {
     /// Wraps an underlying error encountered during key verification.

packages/tracker-core/src/authentication/key/mod.rs

@@ -14,6 +14,41 @@ use torrust_tracker_located_error::LocatedError;
 
 use super::authentication::key::ParseKeyError;
 use super::databases;
+use crate::authentication;
+
+/// Wrapper for all errors returned by the tracker core.
+#[derive(thiserror::Error, Debug, Clone)]
+pub enum TrackerCoreError {
+    /// Error returned when there was an error with the tracker core announce handler.
+    #[error("Tracker core announce error: {source}")]
+    AnnounceError { source: AnnounceError },
+
+    /// Error returned when there was an error with the tracker core whitelist.
+    #[error("Tracker core whitelist error: {source}")]
+    WhitelistError { source: WhitelistError },
+
+    /// Error returned when there was an error with the authentication in the tracker core.
+    #[error("Tracker core authentication error: {source}")]
+    AuthenticationError { source: authentication::key::Error },
+}
+
+impl From<AnnounceError> for TrackerCoreError {
+    fn from(announce_error: AnnounceError) -> Self {
+        Self::AnnounceError { source: announce_error }
+    }
+}
+
+impl From<WhitelistError> for TrackerCoreError {
+    fn from(whitelist_error: WhitelistError) -> Self {
+        Self::WhitelistError { source: whitelist_error }
+    }
+}
+
+impl From<authentication::key::Error> for TrackerCoreError {
+    fn from(whitelist_error: authentication::key::Error) -> Self {
+        Self::AuthenticationError { source: whitelist_error }
+    }
+}
 
 /// Errors related to announce requests.
 #[derive(thiserror::Error, Debug, Clone)]