From 1b62d218dd60046d025331c084200f847c90a65f Mon Sep 17 00:00:00 2001
From: michel <michel@re-invention.nl>
Date: Sun, 19 Jul 2026 20:49:26 +0200
Subject: [PATCH] test(e2e): browse a firewalled peer via the server broker
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add an e2e that forces the server-brokered (firewalled) browse path —
the path that matters on the real network. The sharer does NOT listen,
so the browser's direct dial fails and the server must broker the peer
connecting back to the browser's listener. This exercises
PeerConnectFailed -> ConnectToPeer -> inbound PierceFirewall ->
PeerConnected flush, which the localhost direct-dial test never hits.

Also soften user-facing download failure reasons from protocol jargon
("Peer …") to plain language ("The user went offline", etc.).
---
 soulseek-rs-lib/src/client.rs         |  8 ++--
 soulseek-rs-lib/src/download_store.rs |  2 +-
 soulseek-rs-lib/tests/e2e.rs          | 61 +++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/soulseek-rs-lib/src/client.rs b/soulseek-rs-lib/src/client.rs
index 270d864..875f7ce 100644
--- a/soulseek-rs-lib/src/client.rs
+++ b/soulseek-rs-lib/src/client.rs
@@ -1174,9 +1174,9 @@ impl Client {
 
         if failed {
             let reason = if peer_registered {
-                "Peer rejected the download request"
+                "The user declined the download"
             } else {
-                "Could not connect to the peer"
+                "Could not connect to the user"
             };
             let _ = download
                 .sender
@@ -1213,7 +1213,7 @@ impl Client {
             .map(|d| (d.token, d.sender.clone()))
             .collect();
         for (token, sender) in doomed {
-            let reason = Some("Peer disconnected".to_string());
+            let reason = Some("The user went offline".to_string());
             let _ = sender.send(DownloadStatus::Failed(reason.clone()));
             context.update_download_with_status(
                 token,
@@ -1274,7 +1274,7 @@ impl Client {
                     context.downloads.update_status(
                         token,
                         DownloadStatus::Failed(Some(
-                            "Peer reported the upload failed".to_string(),
+                            "The upload failed on the other side".to_string(),
                         )),
                     );
                     context.downloads.remove(token);
diff --git a/soulseek-rs-lib/src/download_store.rs b/soulseek-rs-lib/src/download_store.rs
index f02a70e..e5fcf60 100644
--- a/soulseek-rs-lib/src/download_store.rs
+++ b/soulseek-rs-lib/src/download_store.rs
@@ -150,7 +150,7 @@ pub fn collect_failed_tokens(
         })
         .map(|d| {
             let _ = d.sender.send(DownloadStatus::Failed(Some(
-                "Peer reported the upload failed".to_string(),
+                "The upload failed on the other side".to_string(),
             )));
             d.token
         })
diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs
index 6760530..912ebf4 100644
--- a/soulseek-rs-lib/tests/e2e.rs
+++ b/soulseek-rs-lib/tests/e2e.rs
@@ -1274,3 +1274,64 @@ fn browse_a_peers_shared_files() {
 
     let _ = std::fs::remove_dir_all(share_dir);
 }
+
+// Browsing a peer that is NOT listening exercises the server-brokered
+// (firewalled) path: the direct dial fails, the client asks the server to
+// broker, and the peer connects back to our listener. This is the path that
+// matters on the real network, where most peers are firewalled. The browser
+// MUST be listening so the peer can connect back.
+#[test]
+fn browse_a_firewalled_peer_via_broker() {
+    let server = server_or_skip!();
+
+    let share_dir = unique_download_dir();
+    std::fs::create_dir_all(share_dir.join("album")).unwrap();
+    std::fs::write(share_dir.join("album").join("hidden.flac"), b"xxxx")
+        .unwrap();
+
+    // Sharer does NOT listen (firewalled): the browser's direct dial will fail,
+    // forcing the server-brokered connect-back.
+    let mut sharer = Client::with_settings(ClientSettings {
+        shared_directory: Some(share_dir.display().to_string()),
+        ..server.settings("e2e_fw_sharer", "pw")
+    });
+    sharer.connect().expect("sharer connect");
+    assert!(sharer.login().expect("sharer login"));
+
+    // Browser listens so the firewalled peer can connect back to it.
+    let browser_port = free_port().expect("browser port");
+    let mut browser = Client::with_settings(server.listening_settings(
+        "e2e_fw_browser",
+        "pw",
+        browser_port,
+    ));
+    browser.connect().expect("browser connect");
+    assert!(browser.login().expect("browser login"));
+
+    std::thread::sleep(Duration::from_secs(1));
+
+    browser
+        .browse_user("e2e_fw_sharer")
+        .expect("browse request");
+
+    // The brokered round-trip has more hops; give it a generous deadline.
+    let deadline = Instant::now() + Duration::from_secs(30);
+    let mut listing = None;
+    while Instant::now() < deadline {
+        if let Some(result) = browser.take_browse_result("e2e_fw_sharer") {
+            listing = Some(result);
+            break;
+        }
+        std::thread::sleep(Duration::from_millis(200));
+    }
+    let directories =
+        listing.expect("browser should receive the firewalled peer's listing");
+    assert!(
+        directories
+            .iter()
+            .any(|d| d.files.iter().any(|(name, _)| name == "hidden.flac")),
+        "the brokered listing should include the shared file"
+    );
+
+    let _ = std::fs::remove_dir_all(share_dir);
+}