README.md (1)

569-570: ⚡ Quick win

Improve changelog specificity.

The changelog entry "Various audit findings and performance fixes" is too vague for users to understand what changed in this release. Given that the AI summary mentions specific improvements (scrape response refactoring, hex encoding utilities, environment variable parsing, error handling), consider adding more detail.

For example:

#### v4.2.10
* Refactored scrape responses to use aggregated TorrentCounts
* Extracted hex encoding utilities into reusable helpers
* Centralized environment variable parsing
* Improved error handling across server initialization and database operations
* Various performance optimizations

This helps users and maintainers understand the scope of changes and aids in debugging if issues arise.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 569 - 570, Update the v4.2.10 changelog entry (the
"#### v4.2.10" section) to replace the vague line "Various audit findings and
performance fixes" with specific bullet points describing the notable changes:
mention the refactor of scrape responses to use aggregated TorrentCounts,
extraction of hex encoding utilities into reusable helpers, centralization of
environment variable parsing, improvements to error handling across server
initialization and database operations, and a general note about performance
optimizations.

src/tracker/impls/torrent_tracker_torrents_updates.rs (1)

111-116: ⚡ Quick win

Avoid cloning the full batch on the persistence hot path.

torrents_to_save.clone() duplicates every TorrentEntry right before save_torrents, which can eat a lot of CPU and memory on large batches and undercuts the gain from draining instead of cloning the queue. Precompute the small cache/delete payloads you need later, then move torrents_to_save into save_torrents (or change save_torrents to borrow).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tracker/impls/torrent_tracker_torrents_updates.rs` around lines 111 -
116, The code clones the entire BTreeMap into save_torrents by calling
torrents_to_save.clone(), which is expensive; instead, avoid duplicating
TorrentEntry by either (A) constructing only the minimal payloads needed for
persistence (e.g., separate small cache/delete lists) from mapping and pass
those to save_torrents, or (B) change the signature of save_torrents to accept
torrents_to_save by value (moving it) or by reference/borrow
(&BTreeMap<InfoHash,(TorrentEntry,UpdatesAction)>) so you can move
torrents_to_save into the call without cloning; locate the creation of
torrents_to_save (mapping.iter().map(...).collect()) and the save_torrents call
and replace the clone with a move or pass a smaller precomputed payload as
appropriate, keeping symbols mapping, torrents_to_save, save_torrents,
TorrentEntry, UpdatesAction, is_persistent and torrent_tracker in mind.

src/websocket/websocket.rs (1)

587-591: ⚡ Quick win

Inconsistent bind error handling compared to HTTP/API servers.

The websocket_master_service still uses .unwrap() on bind operations (lines 588, 605), while the HTTP and API server handlers were updated to use .unwrap_or_else() with proper error logging before exit(1).

For consistency with the audit fixes applied elsewhere, consider applying the same pattern here.

♻️ Proposed fix for consistent error handling

         .bind_rustls_0_23((addr.ip(), addr.port()), tls_config)
-        .unwrap()
+        .unwrap_or_else(|e| {
+            error!("[WEBSOCKET MASTER] Unable to bind to {addr}: {e}");
+            exit(1);
+        })
         .disable_signals()

And for the non-SSL path:

     .bind((addr.ip(), addr.port()))
-    .unwrap()
+    .unwrap_or_else(|e| {
+        error!("[WEBSOCKET MASTER] Unable to bind to {addr}: {e}");
+        exit(1);
+    })
     .disable_signals()

Also applies to: 604-608

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/websocket/websocket.rs` around lines 587 - 591, The
websocket_master_service currently calls .bind_rustls_0_23(...).unwrap() (and
the non-SSL .bind(...) path) which is inconsistent with the HTTP/API servers;
change those unwrap() calls to use .unwrap_or_else(|e| { error!("Failed to bind
websocket server on {}:{}: {}", addr.ip(), addr.port(), e);
std::process::exit(1) }) (or the existing project logger/exit pattern) so
binding failures are logged with the error before exiting; apply the same
replacement for both the TLS bind (bind_rustls_0_23) and the non-TLS bind to
match the other servers and preserve returning (server.handle(), server).

src/database/impls/database_connector_mysql.rs (1)

69-88: ⚡ Quick win

Use match for clearer error handling

The current if let Err pattern works, but using match is more idiomatic and makes the flow explicit. This would avoid shadowing the variable name and make it clear that only the Ok path continues to the unwrap:

Suggested refactor

-        let mysql_connect =
-            DatabaseConnectorMySQL::create(config.database.clone().path.as_str()).await;
-        if let Err(mysql_connect) = mysql_connect {
+        let mysql_pool =
+            match DatabaseConnectorMySQL::create(config.database.clone().path.as_str()).await {
+                Ok(pool) => pool,
+                Err(mysql_connect) => {
                     error!(
                         "{} Unable to connect to MySQL on DSL {}",
                         LOG_PREFIX,
                         config.database.clone().path
                     );
-            error!("{LOG_PREFIX} Message: {mysql_connect}");
-            exit(1);
-        }
+                    error!("{LOG_PREFIX} Message: {mysql_connect}");
+                    exit(1);
+                }
+            };
@@
         structure.mysql = Some(DatabaseConnectorMySQL {
-            pool: mysql_connect.unwrap(),
+            pool: mysql_pool,
         });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/database/impls/database_connector_mysql.rs` around lines 69 - 88, Replace
the current if-let shadowing flow with a match on the result returned by
DatabaseConnectorMySQL::create so the Ok and Err branches are explicit; in the
Err branch log the error using processLogger-style error calls (preserving
LOG_PREFIX and config.database.path) and exit, and in the Ok(branch) take the
successful pool value and set structure.mysql = Some(DatabaseConnectorMySQL {
pool: pool_value }) without calling unwrap; reference
DatabaseConnectorMySQL::create, the mysql_connect variable/result, and the
structure.mysql assignment to locate and change the logic.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/tracker/impls/torrent_tracker_torrents.rs (1)

80-81: 💤 Low value

Verify: RTC peers intentionally excluded from global stats?

The rtc_seeds and rtc_peers fields are now correctly copied during updates, but they're not included in the Seeds/Peers stats calculations (unlike IPv4 and IPv6 counts). This appears consistent with the vacant path, so likely intentional.

Please confirm this is the desired behavior—if RTC peers should contribute to the global seed/peer counts, the delta calculations on lines 62-65 would need to include them as well.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tracker/impls/torrent_tracker_torrents.rs` around lines 80 - 81, The
update copies current.rtc_seeds and current.rtc_peers from torrent_entry but the
global Seeds/Peers stats computations (the seed_delta/peer_delta calculation
that sums IPv4/IPv6 counts) currently omit RTC counts; if RTC peers should
contribute, add current.rtc_seeds and current.rtc_peers (and corresponding
previous values) into the seed_delta and peer_delta calculations used to update
global stats so the deltas include RTC counts alongside IPv4/IPv6; otherwise
leave as-is and add a comment in the update path near
current.rtc_seeds/current.rtc_peers to document that RTC is intentionally
excluded.

src/tracker/impls/torrent_tracker_torrents_updates.rs (1)

175-183: 💤 Low value

On failure, only deduplicated entries are restored—superseded updates are lost.

The restore loop iterates over mapping which contains only the newest entry per InfoHash. If the original queue had multiple updates for the same hash (e.g., timestamps 1, 2, 3 for hash A), only the newest (timestamp 3) is restored after a DB failure. The older updates (1, 2) are permanently discarded.

This is likely acceptable since superseded updates represent stale state, but if the intent was to preserve all original entries on failure for potential retry, this behavior differs. The stats accounting is correct for the post-restore state.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/tracker/impls/torrent_tracker_torrents_updates.rs` around lines 175 -
183, The restore loop currently iterates over a deduplicated `mapping` (newest
per `InfoHash`) and inserts only one entry per hash into
`self.torrents_updates.write()` via `lock.entry(timestamp)`, losing superseded
updates; change the restore logic to iterate over the full original set of saved
updates (or make `mapping` map each `InfoHash` to a Vec of `(timestamp,
torrent_entry, updates_action)`) and insert each preserved update into the
write-lock (ensuring you still use `lock.entry(timestamp)` or a disambiguated
key to avoid collisions), then compute `restored` as the count of all
re-inserted entries and call `self.update_stats(StatsEvent::TorrentsUpdates,
restored)` so all original updates are restored rather than only the
deduplicated newest ones.