Merge pull request #299 from Kibouo/improve-error-handling

Improve error handling
denisidoro · Mar 23, 2020 · 7734723 · 7734723
2 parents c48213b + 53c69aa
commit 7734723
Show file tree

Hide file tree

Showing 25 changed files with 569 additions and 284 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "navi"
-version = "2.3.1"
+version = "2.4.0"
 authors = ["Denis Isidoro <denis_isidoro@live.com>"]
 edition = "2018"
 description = "An interactive cheatsheet tool for the command-line"
@@ -24,6 +24,8 @@ dirs = "2.0.0"
 terminal_size = "0.1.10"
 walkdir = "2"
 shellwords = "1.0.0"
+anyhow = "1.0.27"
+thiserror = "1.0.12"
 
 [dependencies.git2]
 version = "0.10.0"

diff --git a/src/bin/main.rs b/src/bin/main.rs
@@ -1,7 +1,5 @@
 extern crate navi;
 
-use std::error::Error;
-
-fn main() -> Result<(), Box<dyn Error>> {
-    navi::handle_config(navi::config_from_env())
+fn main() -> Result<(), anyhow::Error> {
+    navi::handle_config(navi::config_from_env()).map_err(|e| navi::FileAnIssue::new(e).into())
 }
diff --git a/src/display.rs b/src/display.rs
@@ -13,7 +13,7 @@ pub const DELIMITER: &str = r"  ⠀";
 
 lazy_static! {
     pub static ref WIDTHS: (usize, usize) = get_widths();
-    pub static ref NEWLINE_REGEX: Regex = Regex::new(r"\\\s+").unwrap();
+    pub static ref NEWLINE_REGEX: Regex = Regex::new(r"\\\s+").expect("Invalid regex");
 }
 
 fn get_widths() -> (usize, usize) {

diff --git a/src/filesystem.rs b/src/filesystem.rs
@@ -1,90 +1,119 @@
+use crate::structures::error::filesystem::InvalidPath;
+use crate::structures::error::filesystem::UnreadableDir;
 use crate::structures::option::Config;
+use anyhow::Context;
+use anyhow::Error;
+use core::fmt::Display;
 use std::fs;
 use std::fs::File;
-use std::io::{self, BufRead, BufReader, Lines};
+use std::io::{self, BufRead};
 use std::path::{Path, PathBuf};
 
-pub fn read_lines<P>(filename: P) -> io::Result<Lines<BufReader<File>>>
+pub fn read_lines<P>(filename: P) -> Result<impl Iterator<Item = Result<String, Error>>, Error>
 where
-    P: AsRef<Path>,
+    P: AsRef<Path> + Display + Copy,
 {
-    let file = File::open(filename)?;
-    Ok(io::BufReader::new(file).lines())
+    let file = File::open(filename).with_context(|| format!("Failed to open file {}", filename))?;
+    Ok(io::BufReader::new(file)
+        .lines()
+        .map(|line| line.map_err(Error::from)))
 }
 
-pub fn pathbuf_to_string(pathbuf: PathBuf) -> String {
-    pathbuf.as_os_str().to_str().unwrap().to_string()
+pub fn pathbuf_to_string(pathbuf: PathBuf) -> Result<String, Error> {
+    Ok(pathbuf
+        .as_os_str()
+        .to_str()
+        .ok_or_else(|| InvalidPath(pathbuf.to_path_buf()))
+        .map(str::to_string)?)
 }
 
-pub fn cheat_pathbuf() -> Option<PathBuf> {
-    match dirs::data_dir() {
-        Some(mut d) => {
-            d.push("navi");
-            d.push("cheats");
-            Some(d)
-        }
-        None => None,
-    }
+pub fn cheat_pathbuf() -> Result<PathBuf, Error> {
+    dirs::data_dir()
+        .map(|mut dir| {
+            dir.push("navi");
+            dir.push("cheats");
+            dir
+        })
+        .ok_or_else(|| anyhow!("Unable to acquire user data directory for cheatsheets."))
 }
 
-fn follow_symlink(pathbuf: PathBuf) -> PathBuf {
-    let other = fs::read_link(pathbuf.clone());
-    match other {
-        Ok(o) => {
-            let o_str = o.as_os_str().to_str().unwrap();
+fn follow_symlink(pathbuf: PathBuf) -> Result<PathBuf, Error> {
+    fs::read_link(pathbuf.clone())
+        .map(|o| {
+            let o_str = o
+                .as_os_str()
+                .to_str()
+                .ok_or_else(|| InvalidPath(o.to_path_buf()))?;
             if o_str.starts_with('.') {
-                let parent_str = pathbuf.parent().unwrap().as_os_str().to_str().unwrap();
+                let parent = pathbuf
+                    .parent()
+                    .ok_or_else(|| anyhow!("`{}` has no parent", pathbuf.display()))?;
+                let parent_str = parent
+                    .as_os_str()
+                    .to_str()
+                    .ok_or_else(|| InvalidPath(parent.to_path_buf()))?;
                 let path_str = format!("{}/{}", parent_str, o_str);
                 let p = PathBuf::from(path_str);
                 follow_symlink(p)
             } else {
                 follow_symlink(o)
             }
-        }
-        Err(_) => pathbuf,
-    }
+        })
+        .unwrap_or(Ok(pathbuf))
 }
 
-fn exe_pathbuf() -> PathBuf {
-    let pathbuf = std::env::current_exe().unwrap();
+fn exe_pathbuf() -> Result<PathBuf, Error> {
+    let pathbuf = std::env::current_exe().context("Unable to acquire executable's path")?;
     follow_symlink(pathbuf)
 }
 
-pub fn exe_string() -> String {
-    pathbuf_to_string(exe_pathbuf())
+pub fn exe_string() -> Result<String, Error> {
+    pathbuf_to_string(exe_pathbuf()?)
 }
 
-fn cheat_paths_from_config_dir() -> String {
-    let mut paths_str = String::from("");
-
-    if let Some(f) = cheat_pathbuf() {
-        if let Ok(paths) = fs::read_dir(pathbuf_to_string(f)) {
-            for path in paths {
-                paths_str.push_str(path.unwrap().path().into_os_string().to_str().unwrap());
+fn cheat_paths_from_config_dir() -> Result<String, Error> {
+    cheat_pathbuf()
+        .and_then(pathbuf_to_string)
+        .and_then(|path| {
+            fs::read_dir(path.clone())
+                .map_err(|e| UnreadableDir::new(path.clone(), e).into())
+                .map(|entries| (path, entries))
+        })
+        .and_then(|(path, dir_entries)| {
+            let mut paths_str = String::from("");
+            for entry in dir_entries {
+                let path = entry.map_err(|e| UnreadableDir::new(path.clone(), e))?;
+                paths_str.push_str(
+                    path.path()
+                        .into_os_string()
+                        .to_str()
+                        .ok_or_else(|| InvalidPath(path.path()))?,
+                );
                 paths_str.push_str(":");
             }
-        }
-    }
-
-    paths_str
+            Ok(paths_str)
+        })
 }
 
-pub fn cheat_paths(config: &Config) -> String {
+pub fn cheat_paths(config: &Config) -> Result<String, Error> {
     config
         .path
         .clone()
-        .unwrap_or_else(cheat_paths_from_config_dir)
+        .ok_or_else(|| anyhow!("No cheat paths"))
+        .or_else(|_| {
+            cheat_paths_from_config_dir().context("No directory for cheats in user data directory")
+        })
 }
 
-pub fn create_dir(path: &str) {
-    fs::create_dir_all(path).unwrap_or(());
+pub fn create_dir(path: &str) -> Result<(), Error> {
+    fs::create_dir_all(path).with_context(|| format!("Failed to create directory `{}`", path))
 }
 
-pub fn remove_dir(path: &str) {
-    fs::remove_dir_all(path).unwrap_or(());
+pub fn remove_dir(path: &str) -> Result<(), Error> {
+    fs::remove_dir_all(path).with_context(|| format!("Failed to remove directory `{}`", path))
 }
 
-pub fn tmp_path_str() -> String {
-    let cheat_path_str = pathbuf_to_string(cheat_pathbuf().unwrap());
-    format!("{}/tmp", cheat_path_str)
+pub fn tmp_path_str() -> Result<String, Error> {
+    let cheat_path_str = pathbuf_to_string(cheat_pathbuf()?)?;
+    Ok(format!("{}/tmp", cheat_path_str))
 }
diff --git a/src/flows/aux.rs b/src/flows/aux.rs
@@ -1,7 +1,7 @@
-use std::error::Error;
+use anyhow::Error;
 use std::process;
 
-pub fn abort(operation: &str, issue_number: u32) -> Result<(), Box<dyn Error>> {
+pub fn abort(operation: &str, issue_number: u32) -> Result<(), Error> {
     eprintln!("This version of navi doesn't support {}.", operation);
     eprintln!(
         "Please check https://github.com/denisidoro/navi/issues/{} for more info.",

diff --git a/src/flows/best.rs b/src/flows/best.rs
@@ -1,9 +1,9 @@
 use crate::flows;
 use crate::flows::core::Variant;
 use crate::structures::option::Config;
-use std::error::Error;
+use anyhow::Error;
 
-pub fn main(query: String, args: Vec<String>, config: Config) -> Result<(), Box<dyn Error>> {
+pub fn main(query: String, args: Vec<String>, config: Config) -> Result<(), Error> {
     if args.is_empty() {
         flows::core::main(Variant::Filter(query), config, false)
     } else {