diff --git a/soulseek-rs-lib/src/client.rs b/soulseek-rs-lib/src/client.rs index 6d45864..a48abb9 100644 --- a/soulseek-rs-lib/src/client.rs +++ b/soulseek-rs-lib/src/client.rs @@ -374,7 +374,7 @@ fn download_without_a_connection_resolves_failed() { .expect("download() should return a handle"); assert!(matches!( receiver.recv_timeout(Duration::from_secs(1)), - Ok(DownloadStatus::Failed) + Ok(DownloadStatus::Failed(_)) )); } @@ -398,7 +398,7 @@ fn fail_queued_downloads_notifies_receiver_and_store() { Client::fail_queued_downloads(&client.context, "peer"); - assert!(matches!(receiver.try_recv(), Ok(DownloadStatus::Failed))); + assert!(matches!(receiver.try_recv(), Ok(DownloadStatus::Failed(_)))); assert!(matches!( client .context @@ -407,7 +407,7 @@ fn fail_queued_downloads_notifies_receiver_and_store() { .get_download_by_token(7) .unwrap() .status, - DownloadStatus::Failed + DownloadStatus::Failed(_) )); } @@ -1056,10 +1056,18 @@ impl Client { }; if failed { - let _ = download.sender.send(DownloadStatus::Failed); - self.context - .write_safe()? - .update_download_with_status(token, DownloadStatus::Failed); + let reason = if peer_registered { + "Peer rejected the download request" + } else { + "Could not connect to the peer" + }; + let _ = download + .sender + .send(DownloadStatus::Failed(Some(reason.to_string()))); + self.context.write_safe()?.update_download_with_status( + token, + DownloadStatus::Failed(Some(reason.to_string())), + ); } Ok((download, download_receiver)) @@ -1088,8 +1096,12 @@ impl Client { .map(|d| (d.token, d.sender.clone())) .collect(); for (token, sender) in doomed { - let _ = sender.send(DownloadStatus::Failed); - context.update_download_with_status(token, DownloadStatus::Failed); + let reason = Some("Peer disconnected".to_string()); + let _ = sender.send(DownloadStatus::Failed(reason.clone())); + context.update_download_with_status( + token, + DownloadStatus::Failed(reason), + ); } } @@ -1142,9 +1154,12 @@ impl Client { match client_context.write_safe() { Ok(mut context) => { for token in failed_tokens { - context - .downloads - .update_status(token, DownloadStatus::Failed); + context.downloads.update_status( + token, + DownloadStatus::Failed(Some( + "Peer reported the upload failed".to_string(), + )), + ); context.downloads.remove(token); } } @@ -1326,9 +1341,12 @@ impl Client { ); } Err(e) => { - let _ = download.sender.send(DownloadStatus::Failed); + let reason = Some( + e.to_string(), + ); + let _ = download.sender.send(DownloadStatus::Failed(reason.clone())); match client_context_clone.write_safe() { - Ok(mut ctx) => ctx.update_download_with_status(download.token, DownloadStatus::Failed), + Ok(mut ctx) => ctx.update_download_with_status(download.token, DownloadStatus::Failed(reason)), Err(e) => error!("[client] download failed write: {}", e), } error!( diff --git a/soulseek-rs-lib/src/download_store.rs b/soulseek-rs-lib/src/download_store.rs index 539acfa..f02a70e 100644 --- a/soulseek-rs-lib/src/download_store.rs +++ b/soulseek-rs-lib/src/download_store.rs @@ -149,7 +149,9 @@ pub fn collect_failed_tokens( d.username == username && filename.is_none_or(|f| d.filename == *f) }) .map(|d| { - let _ = d.sender.send(DownloadStatus::Failed); + let _ = d.sender.send(DownloadStatus::Failed(Some( + "Peer reported the upload failed".to_string(), + ))); d.token }) .collect() @@ -301,7 +303,7 @@ mod tests { assert_eq!(tokens, vec![1]); assert!(matches!( rx_match.try_recv().unwrap(), - DownloadStatus::Failed + DownloadStatus::Failed(_) )); } } diff --git a/soulseek-rs-lib/src/peer/listen.rs b/soulseek-rs-lib/src/peer/listen.rs index bb0cd86..a878c96 100644 --- a/soulseek-rs-lib/src/peer/listen.rs +++ b/soulseek-rs-lib/src/peer/listen.rs @@ -212,7 +212,7 @@ fn handle_file_connection( match context.client_context.write_safe() { Ok(mut ctx) => ctx.update_download_with_status( failure_token, - DownloadStatus::Failed, + DownloadStatus::Failed(Some(e.to_string())), ), Err(e) => { error!( diff --git a/soulseek-rs-lib/src/types.rs b/soulseek-rs-lib/src/types.rs index bd520b8..cc31e65 100644 --- a/soulseek-rs-lib/src/types.rs +++ b/soulseek-rs-lib/src/types.rs @@ -114,7 +114,7 @@ impl Download { matches!( self.status, DownloadStatus::Completed - | DownloadStatus::Failed + | DownloadStatus::Failed(_) | DownloadStatus::TimedOut ) } @@ -159,7 +159,8 @@ pub enum DownloadStatus { total_bytes: u64, }, Completed, - Failed, + /// Failed, optionally with a human-readable reason. + Failed(Option), TimedOut, } impl Transfer { diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index d4a1a10..35d1b6a 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -614,7 +614,7 @@ fn a_file_downloads_from_a_peer_over_p_and_f_connections() { completed = true; break; } - Ok(DownloadStatus::Failed | DownloadStatus::TimedOut) => { + Ok(DownloadStatus::Failed(_) | DownloadStatus::TimedOut) => { break; } _ => {} @@ -862,7 +862,7 @@ fn a_file_downloads_from_a_peer_via_direct_connection() { completed = true; break; } - Ok(DownloadStatus::Failed | DownloadStatus::TimedOut) => break, + Ok(DownloadStatus::Failed(_) | DownloadStatus::TimedOut) => break, _ => {} } if client @@ -1021,7 +1021,7 @@ fn a_file_downloads_from_a_firewalled_peer_via_server_broker() { completed = true; break; } - Ok(DownloadStatus::Failed | DownloadStatus::TimedOut) => break, + Ok(DownloadStatus::Failed(_) | DownloadStatus::TimedOut) => break, _ => {} } if client @@ -1122,7 +1122,7 @@ fn two_real_clients_search_and_download() { completed = true; break; } - Ok(DownloadStatus::Failed | DownloadStatus::TimedOut) => break, + Ok(DownloadStatus::Failed(_) | DownloadStatus::TimedOut) => break, _ => {} } if leecher diff --git a/soulseek-rs/src/ui/downloads.rs b/soulseek-rs/src/ui/downloads.rs index b53316c..3b9dbe5 100644 --- a/soulseek-rs/src/ui/downloads.rs +++ b/soulseek-rs/src/ui/downloads.rs @@ -231,7 +231,7 @@ impl MultiDownloadProgress { DownloadStatus::InProgress { .. } => "⧗", DownloadStatus::Paused { .. } => "⏸", DownloadStatus::Completed => "✓", - DownloadStatus::Failed => "✗", + DownloadStatus::Failed(_) => "✗", DownloadStatus::TimedOut => "⏱", }; @@ -272,7 +272,7 @@ impl MultiDownloadProgress { DownloadStatus::InProgress { .. } => warning_style(), DownloadStatus::Paused { .. } => info_style(), DownloadStatus::Completed => primary_style(), - DownloadStatus::Failed | DownloadStatus::TimedOut => { + DownloadStatus::Failed(_) | DownloadStatus::TimedOut => { error_style() } }; @@ -346,7 +346,7 @@ pub fn render_download_stats( .filter(|d| { matches!( d.download.status, - DownloadStatus::Failed | DownloadStatus::TimedOut + DownloadStatus::Failed(_) | DownloadStatus::TimedOut ) }) .count(); diff --git a/soulseek-rs/src/ui/main_tui.rs b/soulseek-rs/src/ui/main_tui.rs index 0f55932..320cf9a 100644 --- a/soulseek-rs/src/ui/main_tui.rs +++ b/soulseek-rs/src/ui/main_tui.rs @@ -958,7 +958,7 @@ impl MainTui { }; if !matches!( entry.download.status, - DownloadStatus::Failed | DownloadStatus::TimedOut + DownloadStatus::Failed(_) | DownloadStatus::TimedOut ) { return; } @@ -995,7 +995,7 @@ impl MainTui { !matches!( entry.download.status, DownloadStatus::Completed - | DownloadStatus::Failed + | DownloadStatus::Failed(_) | DownloadStatus::TimedOut ) }); diff --git a/soulseek-rs/src/ui/panes/download_info_pane.rs b/soulseek-rs/src/ui/panes/download_info_pane.rs index 30063c1..e3f4bc4 100644 --- a/soulseek-rs/src/ui/panes/download_info_pane.rs +++ b/soulseek-rs/src/ui/panes/download_info_pane.rs @@ -72,7 +72,7 @@ fn build_info_lines( } DownloadStatus::Paused { .. } => ("Paused".to_string(), info_style()), DownloadStatus::Completed => ("Completed".to_string(), success_style()), - DownloadStatus::Failed => ("Failed".to_string(), error_style()), + DownloadStatus::Failed(_) => ("Failed".to_string(), error_style()), DownloadStatus::TimedOut => ("Timed out".to_string(), error_style()), }; lines.push(label_value_styled("Status", status_text, status_style)); @@ -139,9 +139,20 @@ fn build_info_lines( lines.push(Line::from("")); push_progress_lines(&mut lines, *bytes_downloaded, *total_bytes); } - DownloadStatus::Completed - | DownloadStatus::Failed - | DownloadStatus::TimedOut => {} + DownloadStatus::Failed(reason) => { + if let Some(reason) = reason { + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + format!("{: {} } lines @@ -256,4 +267,52 @@ mod tests { assert_eq!(basename, "track.mp3"); assert_eq!(parent, ""); } + + fn download_with_status( + status: DownloadStatus, + ) -> soulseek_rs::types::Download { + let (sender, _rx) = std::sync::mpsc::channel(); + soulseek_rs::types::Download { + username: "peer".to_string(), + filename: "song.mp3".to_string(), + token: 1, + size: 100, + download_directory: "~/dl".to_string(), + status, + sender, + queue_position: None, + metadata: soulseek_rs::types::DownloadMetadata::default(), + } + } + + fn lines_to_text(lines: &[Line<'static>]) -> String { + lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n") + } + + #[test] + fn failed_download_renders_its_error_reason() { + let download = download_with_status(DownloadStatus::Failed(Some( + "Peer disconnected".to_string(), + ))); + let text = lines_to_text(&build_info_lines(&download)); + assert!(text.contains("Error"), "missing Error label: {text}"); + assert!(text.contains("Peer disconnected"), "missing reason: {text}"); + } + + #[test] + fn failed_download_without_reason_shows_no_error_line() { + let download = download_with_status(DownloadStatus::Failed(None)); + let text = lines_to_text(&build_info_lines(&download)); + assert!(text.contains("Failed"), "status should still show Failed"); + assert!(!text.contains("Error"), "unexpected Error line: {text}"); + } } diff --git a/soulseek-rs/src/ui/panes/downloads_pane.rs b/soulseek-rs/src/ui/panes/downloads_pane.rs index f339971..0b354f4 100644 --- a/soulseek-rs/src/ui/panes/downloads_pane.rs +++ b/soulseek-rs/src/ui/panes/downloads_pane.rs @@ -54,7 +54,7 @@ pub fn render_downloads_pane( DownloadStatus::InProgress { .. } => ("⧗", warning_style()), DownloadStatus::Paused { .. } => ("⏸", info_style()), DownloadStatus::Completed => ("✓", success_style()), - DownloadStatus::Failed => ("✗", error_style()), + DownloadStatus::Failed(_) => ("✗", error_style()), DownloadStatus::TimedOut => ("⏱", error_style()), }; @@ -91,7 +91,7 @@ pub fn render_downloads_pane( ) } DownloadStatus::Completed => "Completed".to_string(), - DownloadStatus::Failed => "Failed".to_string(), + DownloadStatus::Failed(_) => "Failed".to_string(), DownloadStatus::TimedOut => "Timed out".to_string(), };