fix: replace CLI panics with clean errors on bad IP args and closed stdout

src/lib.rs

@@ -3,8 +3,10 @@ pub mod measurements;
 pub mod progress;
 pub mod run;
 pub mod speedtest;
+pub mod stdout;
 use std::fmt;
 use std::fmt::Display;
+use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
 use clap::Parser;
 use clap_complete::Shell;
@@ -63,11 +65,11 @@ pub struct SpeedTestCLIOptions {
     pub verbose: bool,
 
     /// Force IPv4 with provided source IPv4 address or the default IPv4 address bound to the main interface
-    #[clap(long, value_name = "IPv4", num_args = 0..=1, default_missing_value = "0.0.0.0", conflicts_with = "ipv6")]
+    #[clap(long, value_name = "IPv4", num_args = 0..=1, default_missing_value = "0.0.0.0", value_parser = parse_ipv4_address, conflicts_with = "ipv6")]
     pub ipv4: Option<String>,
 
     /// Force IPv6 with provided source IPv6 address or the default IPv6 address bound to the main interface
-    #[clap(long, value_name = "IPv6", num_args = 0..=1, default_missing_value = "::", conflicts_with = "ipv4")]
+    #[clap(long, value_name = "IPv6", num_args = 0..=1, default_missing_value = "::", value_parser = parse_ipv6_address, conflicts_with = "ipv4")]
     pub ipv6: Option<String>,
 
     /// Disables dynamically skipping tests with larger payload sizes if the tests for the previous payload
@@ -104,10 +106,45 @@ fn parse_payload_size(input_string: &str) -> Result<PayloadSize, String> {
     PayloadSize::from(input_string.to_string())
 }
 
+fn parse_ipv4_address(input_string: &str) -> Result<String, String> {
+    match input_string.parse::<Ipv4Addr>() {
+        Ok(_) => Ok(input_string.to_string()),
+        Err(_) => Err(format!("invalid IPv4 address: '{input_string}'")),
+    }
+}
+
+fn parse_ipv6_address(input_string: &str) -> Result<String, String> {
+    match input_string.parse::<Ipv6Addr>() {
+        Ok(_) => Ok(input_string.to_string()),
+        Err(_) => Err(format!("invalid IPv6 address: '{input_string}'")),
+    }
+}
+
 fn parse_output_format(input_string: &str) -> Result<OutputFormat, String> {
     OutputFormat::from(input_string.to_string())
 }
 
+/// Parses the bound address from the CLI options. Returns an error if the
+/// provided address is not a valid IP address.
+pub fn parse_bound_address(
+    ipv4: &Option<String>,
+    ipv6: &Option<String>,
+) -> Result<Option<IpAddr>, String> {
+    if let Some(address) = ipv4 {
+        address
+            .parse::<IpAddr>()
+            .map(Some)
+            .map_err(|_| format!("invalid IPv4 address: '{address}'"))
+    } else if let Some(address) = ipv6 {
+        address
+            .parse::<IpAddr>()
+            .map(Some)
+            .map_err(|_| format!("invalid IPv6 address: '{address}'"))
+    } else {
+        Ok(None)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -169,6 +206,51 @@ mod tests {
         assert_eq!(format!("{}", OutputFormat::None), "None");
     }
 
+    #[test]
+    fn test_parse_ipv4_address_accepts_valid_addresses() {
+        assert_eq!(parse_ipv4_address("0.0.0.0"), Ok("0.0.0.0".to_string()));
+        assert_eq!(
+            parse_ipv4_address("192.168.1.5"),
+            Ok("192.168.1.5".to_string())
+        );
+    }
+
+    #[test]
+    fn test_parse_ipv4_address_rejects_invalid_addresses() {
+        assert!(parse_ipv4_address("not-an-ip").is_err());
+        assert!(parse_ipv4_address("999.1.1.1").is_err());
+        // IPv6 addresses belong to --ipv6, not --ipv4.
+        assert!(parse_ipv4_address("::1").is_err());
+    }
+
+    #[test]
+    fn test_parse_ipv6_address_accepts_valid_addresses() {
+        assert_eq!(parse_ipv6_address("::"), Ok("::".to_string()));
+        assert_eq!(parse_ipv6_address("fe80::1"), Ok("fe80::1".to_string()));
+    }
+
+    #[test]
+    fn test_parse_ipv6_address_rejects_invalid_addresses() {
+        assert!(parse_ipv6_address("not-an-ip").is_err());
+        // IPv4 addresses belong to --ipv4, not --ipv6.
+        assert!(parse_ipv6_address("192.168.1.5").is_err());
+    }
+
+    #[test]
+    fn test_parse_bound_address() {
+        assert_eq!(parse_bound_address(&None, &None), Ok(None));
+        assert_eq!(
+            parse_bound_address(&Some("127.0.0.1".to_string()), &None),
+            Ok(Some("127.0.0.1".parse().unwrap()))
+        );
+        assert_eq!(
+            parse_bound_address(&None, &Some("fe80::1".to_string())),
+            Ok(Some("fe80::1".parse().unwrap()))
+        );
+        assert!(parse_bound_address(&Some("not-an-ip".to_string()), &None).is_err());
+        assert!(parse_bound_address(&None, &Some("not-an-ip".to_string())).is_err());
+    }
+
     #[test]
     fn test_cli_options_should_download() {
         let mut options = SpeedTestCLIOptions {

src/main.rs

@@ -1,10 +1,9 @@
 use cfspeedtest::speedtest;
+use cfspeedtest::stdout;
 use cfspeedtest::OutputFormat;
 use cfspeedtest::SpeedTestCLIOptions;
 use clap::{CommandFactory, Parser};
 use clap_complete::generate;
-use std::io;
-use std::net::IpAddr;
 
 use cfspeedtest::run::RunConfig;
 use speedtest::speed_test_with_config;
@@ -29,7 +28,11 @@ struct CliOptions {
 }
 
 fn print_completions<G: clap_complete::Generator>(gen: G, cmd: &mut clap::Command) {
-    generate(gen, cmd, cmd.get_name().to_string(), &mut io::stdout());
+    // Buffer the script in memory: clap_complete panics on write errors, and
+    // piping the output into e.g. `head` must not abort the process.
+    let mut buffer = Vec::new();
+    generate(gen, cmd, cmd.get_name().to_string(), &mut buffer);
+    stdout::print(&String::from_utf8_lossy(&buffer));
 }
 
 fn main() -> ExitCode {
@@ -45,27 +48,22 @@ fn main() -> ExitCode {
     }
 
     if options.output_format == OutputFormat::StdOut {
-        println!("Starting Cloudflare speed test");
+        stdout::print_line("Starting Cloudflare speed test");
     }
-    let client;
-    if let Some(ref ip) = options.ipv4 {
-        client = reqwest::blocking::Client::builder()
-            .local_address(ip.parse::<IpAddr>().expect("Invalid IPv4 address"))
-            .timeout(std::time::Duration::from_secs(30))
-            .cookie_store(true)
-            .build();
-    } else if let Some(ref ip) = options.ipv6 {
-        client = reqwest::blocking::Client::builder()
-            .local_address(ip.parse::<IpAddr>().expect("Invalid IPv6 address"))
-            .timeout(std::time::Duration::from_secs(30))
-            .cookie_store(true)
-            .build();
-    } else {
-        client = reqwest::blocking::Client::builder()
-            .timeout(std::time::Duration::from_secs(30))
-            .cookie_store(true)
-            .build();
+    let local_address = match cfspeedtest::parse_bound_address(&options.ipv4, &options.ipv6) {
+        Ok(address) => address,
+        Err(message) => {
+            eprintln!("Error: {message}");
+            return ExitCode::FAILURE;
+        }
+    };
+    let mut client_builder = reqwest::blocking::Client::builder()
+        .timeout(Duration::from_secs(30))
+        .cookie_store(true);
+    if let Some(address) = local_address {
+        client_builder = client_builder.local_address(address);
     }
+    let client = client_builder.build();
     let config = RunConfig {
         base_url: cli.server.as_str().trim_end_matches('/').to_string(),
         max_duration: Duration::from_secs(cli.max_duration),

src/measurements.rs

@@ -2,6 +2,7 @@ use crate::boxplot;
 use crate::run::SpeedTestReport;
 use crate::speedtest::Metadata;
 use crate::speedtest::TestType;
+use crate::stdout;
 use crate::OutputFormat;
 use indexmap::IndexSet;
 use serde::Serialize;
@@ -69,11 +70,13 @@ pub(crate) fn log_measurements(
     let measurements = &report.measurements;
     let payload_attempt_stats = &report.payload_attempt_stats;
     if output_format == OutputFormat::StdOut {
-        println!("\nSummary Statistics");
+        stdout::print_line("\nSummary Statistics");
         if verbose {
-            println!("Type     Payload |  min/max/avg in mbit/s | attempts/success/skipped");
+            stdout::print_line(
+                "Type     Payload |  min/max/avg in mbit/s | attempts/success/skipped",
+            );
         } else {
-            println!("Type     Payload |  min/max/avg in mbit/s");
+            stdout::print_line("Type     Payload |  min/max/avg in mbit/s");
         }
     }
     let mut stat_measurements: Vec<StatMeasurement> = Vec::new();
@@ -99,25 +102,50 @@ pub(crate) fn log_measurements(
         OutputFormat::Csv => {
             let mut wtr = csv::Writer::from_writer(io::stdout());
             for measurement in &stat_measurements {
-                wtr.serialize(measurement).unwrap();
+                if let Err(error) = wtr.serialize(measurement) {
+                    handle_csv_write_error(error);
+                }
+            }
+            if let Err(error) = wtr.flush() {
+                stdout::handle_write_error(&error);
             }
-            wtr.flush().unwrap();
         }
         OutputFormat::Json => {
             let output = compose_report_json(&stat_measurements, report);
-            serde_json::to_writer(io::stdout(), &output).unwrap();
-            println!();
+            if let Err(error) = serde_json::to_writer(io::stdout(), &output) {
+                handle_json_write_error(error);
+            }
+            stdout::print_line("");
         }
         OutputFormat::JsonPretty => {
             let output = compose_report_json(&stat_measurements, report);
-            serde_json::to_writer_pretty(io::stdout(), &output).unwrap();
-            println!();
+            if let Err(error) = serde_json::to_writer_pretty(io::stdout(), &output) {
+                handle_json_write_error(error);
+            }
+            stdout::print_line("");
         }
         OutputFormat::StdOut => {}
         OutputFormat::None => {}
     }
 }
 
+/// Reports a CSV serialization/write failure without panicking: broken pipes
+/// are tolerated (the stdout consumer is gone), other errors are reported.
+fn handle_csv_write_error(error: csv::Error) {
+    match error.into_kind() {
+        csv::ErrorKind::Io(io_error) => stdout::handle_write_error(&io_error),
+        other => eprintln!("Failed to write CSV output: {other:?}"),
+    }
+}
+
+/// Reports a JSON serialization/write failure without panicking.
+fn handle_json_write_error(error: serde_json::Error) {
+    stdout::handle_write_kind(
+        error.io_error_kind().unwrap_or(io::ErrorKind::Other),
+        &format!("Failed to write JSON output: {error}"),
+    );
+}
+
 fn compose_report_json(
     stat_measurements: &[StatMeasurement],
     report: &SpeedTestReport,
@@ -222,22 +250,22 @@ fn log_measurements_by_test_type(
             });
             if output_format == OutputFormat::StdOut {
                 if verbose {
-                    println!(
+                    stdout::print_line(&format!(
                         "{fmt_test_type:<9} {formatted_payload:<7}|  min {min:<7.2} max {max:<7.2} avg {avg:<7.2} | {attempts:>3}/{successes:>3}/{skipped:>3}"
-                    );
+                    ));
                 } else {
-                    println!(
+                    stdout::print_line(&format!(
                         "{fmt_test_type:<9} {formatted_payload:<7}|  min {min:<7.2} max {max:<7.2} avg {avg:<7.2}"
-                    );
+                    ));
                 }
                 if successes < target_successes {
-                    println!(
+                    stdout::print_line(&format!(
                         "                    insufficient samples: collected {successes}/{target_successes} successful runs"
-                    );
+                    ));
                 }
                 if verbose {
                     let plot = boxplot::render_plot(min, q1, median, q3, max);
-                    println!("{plot}\n");
+                    stdout::print_line(&format!("{plot}\n"));
                 }
             }
         } else {
@@ -257,13 +285,13 @@ fn log_measurements_by_test_type(
             });
             if output_format == OutputFormat::StdOut {
                 if verbose {
-                    println!(
+                    stdout::print_line(&format!(
                         "{fmt_test_type:<9} {formatted_payload:<7}|  min N/A     max N/A     avg N/A     | {attempts:>3}/{successes:>3}/{skipped:>3} (insufficient samples)"
-                    );
+                    ));
                 } else {
-                    println!(
+                    stdout::print_line(&format!(
                         "{fmt_test_type:<9} {formatted_payload:<7}|  min N/A     max N/A     avg N/A     (insufficient samples)"
-                    );
+                    ));
                 }
             }
         }

src/progress.rs

@@ -1,15 +1,13 @@
-use std::io::stdout;
-use std::io::Write;
+use crate::stdout;
 
 pub fn print_progress(name: &str, curr: u32, max: u32) {
     const BAR_LEN: u32 = 30;
     let progress_line = ((curr as f32 / max as f32) * BAR_LEN as f32) as u32;
     let remaining_line = BAR_LEN - progress_line;
-    print!(
+    stdout::print(&format!(
         "\r{:<15} [{}{}]",
         name,
         (0..progress_line).map(|_| "=").collect::<String>(),
         (0..remaining_line).map(|_| "-").collect::<String>(),
-    );
-    stdout().flush().expect("error printing progress bar");
+    ));
 }