Skip to content

refactor(http-protocol): decouple from tracker-core #1833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josecelano merged 2 commits into from
May 27, 2026
Merged

refactor(http-protocol): decouple from tracker-core #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54f8356
refactor(http-protocol): decouple from tracker-core
josecelano May 27, 2026
19b0ce1
refactor(http-tracker-core): deduplicate protocol error mapping
josecelano May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 27 additions & 28 deletions docs/issues/open/1669-overhaul-packages/EPIC.md
View file
Open in desktop

Large diffs are not rendered by default.

52 changes: 26 additions & 26 deletions docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ status: open
priority: p1
github-issue: 1830
spec-path: docs/issues/open/1830-1669-12-decouple-http-protocol-from-tracker-core.md
branch: null
branch: 1830-1669-12-decouple-http-protocol-from-tracker-core
related-pr: null
last-updated-utc: 2026-05-26 00:00
last-updated-utc: 2026-05-27 00:00
semantic-links:
skill-links:
- create-issue
Expand Down Expand Up @@ -104,47 +104,47 @@ Usage purpose:

Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`.

| ID | Status | Task | Notes / Expected Output |
| --- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- |
| T1 | TODO | Confirm all tracker-core usage in `http-protocol` is limited to `responses/error.rs` | Evidence captured in PR description |
| T2 | TODO | Remove `From<tracker-core error>` impls from `packages/http-protocol/src/v1/responses/error.rs` | No direct imports or type references to `bittorrent_tracker_core::*` remain |
| T3 | TODO | Remove `bittorrent-tracker-core` from `packages/http-protocol/Cargo.toml` | `cargo metadata` shows no `http-protocol -> tracker-core` edge |
| T4 | TODO | Add/adjust mapping at higher layer (`http-tracker-core` and/or `axum-http-tracker-server`) for equivalent client-visible failure messages | Existing behavior preserved for announce/scrape/auth/whitelist errors |
| T5 | TODO | Update or add tests for failure mapping behavior | Coverage in affected crates; assertions on error message fragments |
| T6 | TODO | Run verification commands | Build/tests/lints pass |
| T7 | TODO | Update EPIC tracking rows and draft list as needed | Active Subissues remain consistent |
| T8 | TODO | Update EPIC after implementation | Update Active Subissues progress and EPIC sections: Package Inventory, Desired Package State, Torrust Dependency Lists (Direct, Non-dev) |
| ID | Status | Task | Notes / Expected Output |
| --- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |
| T1 | DONE | Confirm all tracker-core usage in `http-protocol` is limited to `responses/error.rs` | Confirmed by `rg` before edits (`torrust_tracker_core::*` only in `responses/error.rs`) |
| T2 | DONE | Remove `From<tracker-core error>` impls from `packages/http-protocol/src/v1/responses/error.rs` | Removed announce/scrape/whitelist/authentication conversion impls |
| T3 | DONE | Remove `bittorrent-tracker-core` from `packages/http-protocol/Cargo.toml` | Removed dependency; `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` has no tracker-core edge |
| T4 | DONE | Add/adjust mapping at higher layer (`http-tracker-core` and/or `axum-http-tracker-server`) for equivalent client-visible failure messages | Added `From<HttpAnnounceError>` and `From<HttpScrapeError>` into protocol `responses::error::Error` in `http-tracker-core` |
| T5 | DONE | Update or add tests for failure mapping behavior | Updated axum handler unit/integration assertions to use boundary mapping with expected message fragments |
| T6 | DONE | Run verification commands | `cargo build --workspace`, targeted crate tests, `linter all` all passed |
| T7 | DONE | Update EPIC tracking rows and draft list as needed | Updated in EPIC Active Subissues and details table |
| T8 | DONE | Update EPIC after implementation | Updated EPIC dependency narrative and `torrust-tracker-http-tracker-protocol` direct dependency list |

## Acceptance Criteria

- [ ] `packages/http-protocol/Cargo.toml` has no `bittorrent-tracker-core` dependency.
- [ ] `packages/http-protocol` has no source-level references to `bittorrent_tracker_core::`.
- [ ] Client-visible HTTP error responses still include meaningful failure reasons
- [x] `packages/http-protocol/Cargo.toml` has no `bittorrent-tracker-core` dependency.
- [x] `packages/http-protocol` has no source-level references to `bittorrent_tracker_core::`.
- [x] Client-visible HTTP error responses still include meaningful failure reasons
for announce/scrape/auth/whitelist failures.
- [ ] `cargo build --workspace` passes.
- [ ] Relevant tests in HTTP protocol/core/server packages pass.
- [ ] `linter all` exits with code `0`.
- [ ] EPIC tracking is updated to include this subissue.
- [x] `cargo build --workspace` passes.
- [x] Relevant tests in HTTP protocol/core/server packages pass.
- [x] `linter all` exits with code `0`.
- [x] EPIC tracking is updated to include this subissue.

## Verification Plan

### Automatic Checks

- `cargo build --workspace`
- `cargo test -p bittorrent-http-tracker-protocol`
- `cargo test -p bittorrent-http-tracker-core`
- `cargo test -p torrust-tracker-http-tracker-protocol`
- `cargo test -p torrust-tracker-http-tracker-core`
- `cargo test -p torrust-tracker-axum-http-server`
- `linter all`

### Manual Verification Scenarios

Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`.

| ID | Scenario | Command / Steps | Expected Result | Status | Evidence |
| --- | ------------------------------------------ | ------------------------------------------------------------ | ------------------------------------------------------ | ------ | -------- |
| M1 | No forbidden edge remains | `cargo tree -p bittorrent-http-tracker-protocol --depth 1` | No dependency on `bittorrent-tracker-core` | TODO | |
| M2 | No tracker-core symbols in protocol source | `rg "bittorrent_tracker_core::" packages/http-protocol` | No matches | TODO | |
| M3 | Error mapping behavior preserved | Trigger announce/scrape/auth failure cases in existing tests | Error responses still include expected message context | TODO | |
| ID | Scenario | Command / Steps | Expected Result | Status | Evidence |
| --- | ------------------------------------------ | ------------------------------------------------------------------------------- | ------------------------------------------------------ | ------ | ---------------------------------------------------------------------------- |
| M1 | No forbidden edge remains | `cargo tree -p torrust-tracker-http-tracker-protocol --depth 1` | No dependency on `torrust-tracker-core` | DONE | Tree output shows no tracker-core dependency |
| M2 | No tracker-core symbols in protocol source | `rg "torrust_tracker_core::\|bittorrent_tracker_core::" packages/http-protocol` | No matches | DONE | `rg` returned no output |
| M3 | Error mapping behavior preserved | Trigger announce/scrape/auth failure cases in existing tests | Error responses still include expected message context | DONE | `cargo test -p torrust-tracker-axum-http-server` passed (unit + integration) |

## Risks and Trade-offs

Expand Down
33 changes: 9 additions & 24 deletions packages/axum-http-server/src/v1/handlers/announce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ async fn handle(
{
Ok(announce_data) => announce_data,
Err(error) => {
let error_response = responses::error::Error {
failure_reason: error.to_string(),
};
let error_response = responses::error::Error::from(error);
return (StatusCode::OK, error_response.write()).into_response();
}
};
Expand Down Expand Up @@ -253,14 +251,9 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
"Tracker core error: Tracker core authentication error: Missing authentication key",
);
assert_error_response(&error_response, "Tracker authentication error: Missing authentication key");
}

#[tokio::test]
Expand All @@ -284,13 +277,11 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
"Tracker core error: Tracker core authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
"Tracker authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
);
}
}
Expand Down Expand Up @@ -325,14 +316,12 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
&format!(
"Tracker core error: Tracker core whitelist error: The torrent: {}, is not whitelisted",
"Tracker whitelist error: The torrent: {}, is not whitelisted",
announce_request.info_hash
),
);
Expand Down Expand Up @@ -373,9 +362,7 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
Expand Down Expand Up @@ -418,9 +405,7 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
Expand Down
12 changes: 3 additions & 9 deletions packages/axum-http-server/src/v1/handlers/scrape.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ async fn handle(
{
Ok(scrape_data) => scrape_data,
Err(error) => {
let error_response = responses::error::Error {
failure_reason: error.to_string(),
};
let error_response = responses::error::Error::from(error);
return (StatusCode::OK, error_response.write()).into_response();
}
};
Expand Down Expand Up @@ -332,9 +330,7 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
Expand Down Expand Up @@ -379,9 +375,7 @@ mod tests {
.await
.unwrap_err();

let error_response = responses::error::Error {
failure_reason: response.to_string(),
};
let error_response = responses::error::Error::from(response);

assert_error_response(
&error_response,
Expand Down
8 changes: 1 addition & 7 deletions packages/axum-http-server/tests/server/asserts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,5 @@ pub async fn assert_authentication_error_response(response: Response) {
}

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(),
);
assert_authentication_error_response(response).await;
}
1 change: 0 additions & 1 deletion packages/http-protocol/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ version.workspace = true
[dependencies]
torrust-tracker-udp-tracker-protocol = { version = "3.0.0-develop", path = "../udp-protocol" }
bittorrent-primitives = "0.2.0"
torrust-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" }
derive_more = { version = "2", features = [ "as_ref", "constructor", "from" ] }
multimap = "0"
percent-encoding = "2"
Expand Down
32 changes: 0 additions & 32 deletions packages/http-protocol/src/v1/responses/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,38 +64,6 @@ impl From<PeerIpResolutionError> for Error {
}
}

impl From<torrust_tracker_core::error::AnnounceError> for Error {
fn from(err: torrust_tracker_core::error::AnnounceError) -> Self {
Error {
failure_reason: format!("Tracker announce error: {err}"),
}
}
}

impl From<torrust_tracker_core::error::ScrapeError> for Error {
fn from(err: torrust_tracker_core::error::ScrapeError) -> Self {
Error {
failure_reason: format!("Tracker scrape error: {err}"),
}
}
}

impl From<torrust_tracker_core::error::WhitelistError> for Error {
fn from(err: torrust_tracker_core::error::WhitelistError) -> Self {
Error {
failure_reason: format!("Tracker whitelist error: {err}"),
}
}
}

impl From<torrust_tracker_core::authentication::Error> for Error {
fn from(err: torrust_tracker_core::authentication::Error) -> Self {
Error {
failure_reason: format!("Tracker authentication error: {err}"),
}
}
}

#[cfg(test)]
mod tests {
use std::panic::Location;
Expand Down
11 changes: 11 additions & 0 deletions packages/http-tracker-core/src/services/announce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use torrust_tracker_core::authentication::{self, Key};
use torrust_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistError};
use torrust_tracker_core::whitelist;
use torrust_tracker_http_tracker_protocol::v1::requests::announce::{Announce, peer_from_request};
use torrust_tracker_http_tracker_protocol::v1::responses::error::Error as HttpProtocolErrorResponse;
use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::{
ClientIpSources, PeerIpResolutionError, RemoteClientAddr, resolve_remote_client_addr,
};
Expand All @@ -27,6 +28,7 @@ use torrust_tracker_primitives::peer::PeerAnnouncement;

use crate::event;
use crate::event::Event;
use crate::services::error_mapping::protocol_error_from_tracker_core_error;

/// The HTTP tracker `announce` service.
///
Expand Down Expand Up @@ -201,6 +203,15 @@ impl From<authentication::key::Error> for HttpAnnounceError {
}
}

impl From<HttpAnnounceError> for HttpProtocolErrorResponse {
fn from(error: HttpAnnounceError) -> Self {
match error {
HttpAnnounceError::PeerIpResolutionError { source } => source.into(),
HttpAnnounceError::TrackerCoreError { source } => protocol_error_from_tracker_core_error(source),
}
}
}

#[cfg(test)]
mod tests {
use std::net::SocketAddr;
Expand Down
19 changes: 19 additions & 0 deletions packages/http-tracker-core/src/services/error_mapping.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use torrust_tracker_core::error::TrackerCoreError;
use torrust_tracker_http_tracker_protocol::v1::responses::error::Error as HttpProtocolErrorResponse;

pub(crate) fn protocol_error_from_tracker_core_error(error: TrackerCoreError) -> HttpProtocolErrorResponse {
match error {
TrackerCoreError::AnnounceError { source } => HttpProtocolErrorResponse {
failure_reason: format!("Tracker announce error: {source}"),
},
TrackerCoreError::ScrapeError { source } => HttpProtocolErrorResponse {
failure_reason: format!("Tracker scrape error: {source}"),
},
TrackerCoreError::WhitelistError { source } => HttpProtocolErrorResponse {
failure_reason: format!("Tracker whitelist error: {source}"),
},
TrackerCoreError::AuthenticationError { source } => HttpProtocolErrorResponse {
failure_reason: format!("Tracker authentication error: {source}"),
},
}
}
1 change: 1 addition & 0 deletions packages/http-tracker-core/src/services/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
//!
//! Refer to [`torrust_tracker`](crate) documentation.
pub mod announce;
pub(crate) mod error_mapping;
pub mod scrape;
11 changes: 11 additions & 0 deletions packages/http-tracker-core/src/services/scrape.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ use torrust_tracker_core::authentication::{self, Key};
use torrust_tracker_core::error::{ScrapeError, TrackerCoreError, WhitelistError};
use torrust_tracker_core::scrape_handler::ScrapeHandler;
use torrust_tracker_http_tracker_protocol::v1::requests::scrape::Scrape;
use torrust_tracker_http_tracker_protocol::v1::responses::error::Error as HttpProtocolErrorResponse;
use torrust_tracker_http_tracker_protocol::v1::services::peer_ip_resolver::{
ClientIpSources, PeerIpResolutionError, RemoteClientAddr, resolve_remote_client_addr,
};
use torrust_tracker_primitives::ScrapeData;

use crate::event::{ConnectionContext, Event};
use crate::services::error_mapping::protocol_error_from_tracker_core_error;

/// The HTTP tracker `scrape` service.
///
Expand Down Expand Up @@ -163,6 +165,15 @@ impl From<authentication::key::Error> for HttpScrapeError {
}
}

impl From<HttpScrapeError> for HttpProtocolErrorResponse {
fn from(error: HttpScrapeError) -> Self {
match error {
HttpScrapeError::PeerIpResolutionError { source } => source.into(),
HttpScrapeError::TrackerCoreError { source } => protocol_error_from_tracker_core_error(source),
}
}
}

#[cfg(test)]
mod tests {

Expand Down
Loading