Improve error handling and prevent panics during file processing

.github/workflows/rust.yml

@@ -2,13 +2,13 @@ name: Rust
 
 on:
   push:
-    branches: [ main ]
+    branches: [main]
     paths-ignore:
-      - '**.md'
+      - "**.md"
   pull_request:
-    branches: [ main ]
+    branches: [main]
     paths-ignore:
-      - '**.md'
+      - "**.md"
 
 env:
   CARGO_TERM_COLOR: always
@@ -17,8 +17,12 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
-    - name: Build
-      run: cargo build --verbose
-    - name: Run tests
-      run: cargo test --verbose
+      - uses: actions/checkout@v4
+      - name: Check formatting
+        run: cargo fmt --check
+      - name: Build
+        run: cargo build --verbose
+      - name: Run tests
+        run: cargo test --verbose
+      - name: Run clippy
+        run: cargo clippy -- -D warnings

src/bin/monocle.rs

@@ -1,4 +1,6 @@
 #![allow(clippy::type_complexity)]
+#![deny(clippy::unwrap_used)]
+#![deny(clippy::expect_used)]
 
 use std::collections::{HashMap, HashSet};
 use std::io::Write;
@@ -249,27 +251,38 @@ enum RadarCommands {
     },
 }
 
-fn elem_to_string(elem: &BgpElem, json: bool, pretty: bool, collector: &str) -> String {
+fn elem_to_string(
+    elem: &BgpElem,
+    json: bool,
+    pretty: bool,
+    collector: &str,
+) -> Result<String, anyhow::Error> {
     if json {
         let mut val = json!(elem);
         val.as_object_mut()
-            .unwrap()
+            .ok_or_else(|| anyhow::anyhow!("Expected JSON object"))?
             .insert("collector".to_string(), collector.into());
         if pretty {
-            serde_json::to_string_pretty(&val).unwrap()
+            Ok(serde_json::to_string_pretty(&val)?)
         } else {
-            val.to_string()
+            Ok(val.to_string())
         }
     } else {
-        format!("{}|{}", elem, collector)
+        Ok(format!("{}|{}", elem, collector))
     }
 }
 
 fn main() {
     dotenvy::dotenv().ok();
     let cli = Cli::parse();
 
-    let config = MonocleConfig::new(&cli.config);
+    let config = match MonocleConfig::new(&cli.config) {
+        Ok(c) => c,
+        Err(e) => {
+            eprintln!("Failed to load configuration: {}", e);
+            std::process::exit(1);
+        }
+    };
 
     if cli.debug {
         tracing_subscriber::fmt()
@@ -294,16 +307,34 @@ fn main() {
                 return;
             }
 
-            let file_path = file_path.to_str().unwrap();
-            let parser = filters.to_parser(file_path).unwrap();
+            let file_path = match file_path.to_str() {
+                Some(path) => path,
+                None => {
+                    eprintln!("Invalid file path");
+                    std::process::exit(1);
+                }
+            };
+            let parser = match filters.to_parser(file_path) {
+                Ok(p) => p,
+                Err(e) => {
+                    eprintln!("Failed to create parser for {}: {}", file_path, e);
+                    std::process::exit(1);
+                }
+            };
 
             let mut stdout = std::io::stdout();
 
             match mrt_path {
                 None => {
                     for elem in parser {
                         // output to stdout
-                        let output_str = elem_to_string(&elem, json, pretty, "");
+                        let output_str = match elem_to_string(&elem, json, pretty, "") {
+                            Ok(s) => s,
+                            Err(e) => {
+                                eprintln!("Failed to format element: {}", e);
+                                continue;
+                            }
+                        };
                         if let Err(e) = writeln!(stdout, "{}", &output_str) {
                             if e.kind() != std::io::ErrorKind::BrokenPipe {
                                 eprintln!("ERROR: {e}");
@@ -348,15 +379,31 @@ fn main() {
             }
 
             let mut sqlite_path_str = "".to_string();
-            let sqlite_db = sqlite_path.map(|p| {
-                sqlite_path_str = p.to_str().unwrap().to_string();
-                MsgStore::new(&Some(sqlite_path_str.clone()), sqlite_reset)
+            let sqlite_db = sqlite_path.and_then(|p| {
+                p.to_str()
+                    .map(|s| {
+                        sqlite_path_str = s.to_string();
+                        match MsgStore::new(&Some(sqlite_path_str.clone()), sqlite_reset) {
+                            Ok(store) => Some(store),
+                            Err(e) => {
+                                eprintln!("Failed to create SQLite store: {}", e);
+                                std::process::exit(1);
+                            }
+                        }
+                    })
+                    .flatten()
             });
-            let mrt_path = mrt_path.map(|p| p.to_str().unwrap().to_string());
+            let mrt_path = mrt_path.and_then(|p| p.to_str().map(|s| s.to_string()));
             let show_progress = sqlite_db.is_some() || mrt_path.is_some();
 
             // it's fine to unwrap as the filters.validate() function has already checked for issues
-            let items = filters.to_broker_items().unwrap();
+            let items = match filters.to_broker_items() {
+                Ok(items) => items,
+                Err(e) => {
+                    eprintln!("Failed to convert filters to broker items: {}", e);
+                    std::process::exit(1);
+                }
+            };
 
             let total_items = items.len();
 
@@ -409,15 +456,24 @@ fn main() {
                     msg_count += 1;
 
                     if display_stdout {
-                        let output_str = elem_to_string(&elem, json, pretty, collector.as_str());
+                        let output_str =
+                            match elem_to_string(&elem, json, pretty, collector.as_str()) {
+                                Ok(s) => s,
+                                Err(e) => {
+                                    eprintln!("Failed to format element: {}", e);
+                                    continue;
+                                }
+                            };
                         println!("{output_str}");
                         continue;
                     }
 
                     msg_cache.push((elem, collector));
                     if msg_cache.len() >= 100000 {
                         if let Some(db) = &sqlite_db {
-                            db.insert_elems(&msg_cache);
+                            if let Err(e) = db.insert_elems(&msg_cache) {
+                                eprintln!("Failed to insert elements to database: {}", e);
+                            }
                         }
                         if let Some((encoder, _writer)) = &mut mrt_writer {
                             for (elem, _) in &msg_cache {
@@ -430,7 +486,9 @@ fn main() {
 
                 if !msg_cache.is_empty() {
                     if let Some(db) = &sqlite_db {
-                        db.insert_elems(&msg_cache);
+                        if let Err(e) = db.insert_elems(&msg_cache) {
+                            eprintln!("Failed to insert elements to database: {}", e);
+                        }
                     }
                     if let Some((encoder, _writer)) = &mut mrt_writer {
                         for (elem, _) in &msg_cache {
@@ -476,7 +534,13 @@ fn main() {
                     let url = item.url;
                     let collector = item.collector_id;
                     info!("start parsing {}", url.as_str());
-                    let parser = filters.to_parser(url.as_str()).unwrap();
+                    let parser = match filters.to_parser(url.as_str()) {
+                        Ok(p) => p,
+                        Err(e) => {
+                            eprintln!("Failed to parse {}: {}", url.as_str(), e);
+                            return;
+                        }
+                    };
 
                     let mut elems_count = 0;
                     for elem in parser {
@@ -510,7 +574,10 @@ fn main() {
 
             if update {
                 // if the update flag is set, clear existing as2org data and re-download later
-                as2org.clear_db();
+                if let Err(e) = as2org.clear_db() {
+                    eprintln!("Failed to clear database: {}", e);
+                    std::process::exit(1);
+                }
             }
 
             if as2org.is_db_empty() {

src/config.rs

@@ -1,3 +1,4 @@
+use anyhow::{anyhow, Result};
 use config::Config;
 use std::collections::HashMap;
 use std::path::Path;
@@ -15,10 +16,14 @@ const EMPTY_CONFIG: &str = r#"### monocle configuration file
 
 impl MonocleConfig {
     /// function to create and initialize a new configuration
-    pub fn new(path: &Option<String>) -> MonocleConfig {
+    pub fn new(path: &Option<String>) -> Result<MonocleConfig> {
         let mut builder = Config::builder();
         // by default use $HOME/.monocle.toml as the configuration file path
-        let home_dir = dirs::home_dir().unwrap().to_str().unwrap().to_owned();
+        let home_dir = dirs::home_dir()
+            .ok_or_else(|| anyhow!("Could not find home directory"))?
+            .to_str()
+            .ok_or_else(|| anyhow!("Could not convert home directory path to string"))?
+            .to_owned();
         // config dir
         let monocle_dir = format!("{}/.monocle", home_dir.as_str());
 
@@ -27,44 +32,60 @@ impl MonocleConfig {
             Some(p) => {
                 let path = Path::new(p.as_str());
                 if path.exists() {
-                    builder = builder.add_source(config::File::with_name(path.to_str().unwrap()));
+                    let path_str = path
+                        .to_str()
+                        .ok_or_else(|| anyhow!("Could not convert path to string"))?;
+                    builder = builder.add_source(config::File::with_name(path_str));
                 } else {
-                    std::fs::write(p.as_str(), EMPTY_CONFIG).expect("Unable to create config file");
+                    std::fs::write(p.as_str(), EMPTY_CONFIG)
+                        .map_err(|e| anyhow!("Unable to create config file: {}", e))?;
                 }
             }
             None => {
-                std::fs::create_dir_all(monocle_dir.as_str()).unwrap();
+                std::fs::create_dir_all(monocle_dir.as_str())
+                    .map_err(|e| anyhow!("Unable to create monocle directory: {}", e))?;
                 let p = format!("{}/monocle.toml", monocle_dir.as_str());
                 if Path::new(p.as_str()).exists() {
                     builder = builder.add_source(config::File::with_name(p.as_str()));
                 } else {
-                    std::fs::write(p.as_str(), EMPTY_CONFIG)
-                        .unwrap_or_else(|_| panic!("Unable to create config file {}", p.as_str()));
+                    std::fs::write(p.as_str(), EMPTY_CONFIG).map_err(|e| {
+                        anyhow!("Unable to create config file {}: {}", p.as_str(), e)
+                    })?;
                 }
             }
         }
         // Add in settings from the environment (with a prefix of APP)
         // Eg.. `MONOCLE_DEBUG=1 ./target/app` would set the `debug` key
         builder = builder.add_source(config::Environment::with_prefix("MONOCLE"));
 
-        let settings = builder.build().unwrap();
+        let settings = builder
+            .build()
+            .map_err(|e| anyhow!("Failed to build configuration: {}", e))?;
         let config = settings
             .try_deserialize::<HashMap<String, String>>()
-            .unwrap();
+            .map_err(|e| anyhow!("Failed to deserialize configuration: {}", e))?;
 
         // check data directory config
         let data_dir = match config.get("data_dir") {
             Some(p) => {
                 let path = Path::new(p);
-                path.to_str().unwrap().to_string()
+                path.to_str()
+                    .ok_or_else(|| anyhow!("Could not convert data_dir path to string"))?
+                    .to_string()
             }
             None => {
-                let dir = format!("{}/.monocle/", dirs::home_dir().unwrap().to_str().unwrap());
-                std::fs::create_dir_all(dir.as_str()).unwrap();
+                let home =
+                    dirs::home_dir().ok_or_else(|| anyhow!("Could not find home directory"))?;
+                let home_str = home
+                    .to_str()
+                    .ok_or_else(|| anyhow!("Could not convert home directory path to string"))?;
+                let dir = format!("{}/.monocle/", home_str);
+                std::fs::create_dir_all(dir.as_str())
+                    .map_err(|e| anyhow!("Unable to create data directory: {}", e))?;
                 dir
             }
         };
 
-        MonocleConfig { data_dir }
+        Ok(MonocleConfig { data_dir })
     }
 }