Modernize error handling

This sprinkles in some thiserror and anyhow instead of the hand-rolled
'error_froms!' macros and the MainResult. The benefit of this is that
we're hooking into an established ecosystem of error handling and we're
saving a lot of effort in the hand-written Display and Error
implementations.

The reason for not sprinkling anyhow everywhere is because the
retrieval/rendering part of kondou could be split off into a library at
some point, in which case we want to have a proper KondouError type.
However, the argument could be made that the current split of errors is
not very good, especially because many of them boil down to the same
inner errors (RenderError wrapping ApiError wrapping reqwest::Error),
which keeps unnecessary information.

Some future improvements may include
1.) Unifying those error enums into one bigger enum
2.) Attaching more context through anyhow in the application layer
3.) Properly define an API and split off the inner logic from the UI
  logic

1 files changed, 15 insertions, 53 deletions

diff --git a/src/main.rs b/src/main.rs
index 98dbc1e..5b11a40 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,18 +1,3 @@
-use std::error::Error as StdError;
-use std::fmt;
-
-macro_rules! error_froms {
-    ($tname:ty, $($ename:ident : $fty:ty => $res:expr,)*) => {
-        $(
-            impl From<$fty> for $tname {
-                fn from($ename: $fty) -> Self {
-                    $res
-                }
-            }
-        )*
-    }
-}
-
 mod api;
 mod bt;
 mod cache;
@@ -20,6 +5,8 @@ mod output;
 mod render;
 mod useropts;
 
+use anyhow::{Context, Result};
+use thiserror::Error;
 use clap::{App, Arg, ArgMatches};
 
 use api::{Api, Profession, Skill};
@@ -32,54 +19,30 @@ use render::RenderError;
 const APP_NAME: &str = "kondou";
 
 /// Return value indicating that a requested resource could not be found.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Error)]
 enum NotFound {
     /// Used when the requested profession can not be found.
     ///
     /// The argument is the requested profession.
+    #[error("The profession '{0}' could not be found")]
     Profession(String),
     /// Used when a skill given by its ID could not be found.
     ///
     /// The argument is the requested skill id.
+    #[error("The skill with ID {0} could not be found")]
     SkillId(u32),
     /// Used when a skill given by its name could not be found.
     ///
     /// The argument is the requested skill name.
+    #[error("The skill named '{0}' could not be found")]
     SkillName(String),
     /// Used when a specialization could not be found.
     ///
     /// The argument is the requested specialization.
+    #[error("The specialization named '{0}' could not be found")]
     Specialization(String),
 }
 
-impl fmt::Display for NotFound {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            NotFound::Profession(ref name) => {
-                write!(f, "the profession '{}' could not be found", name)
-            }
-            NotFound::SkillId(id) => write!(f, "the skill with the ID '{}' was not found", id),
-            NotFound::SkillName(ref name) => {
-                write!(f, "the skill with the name '{}' was not found", name)
-            }
-            NotFound::Specialization(ref name) => write!(
-                f,
-                "the specialization with the name '{}' was not found",
-                name
-            ),
-        }
-    }
-}
-
-impl StdError for NotFound {}
-
-/// A top-level result.
-///
-/// We use a dynamic error dispatch here for two reasons:
-/// 1. The only thing that we really do here is displaying the error to the user.
-/// 2. We don't want yet another error kind with a lot of kinds.
-type MainResult<T> = Result<T, Box<dyn StdError>>;
-
 /// A trait for containers that only contain a single item.
 trait SingleContainer<T> {
     /// Extract the single element by consuming the container.
@@ -94,7 +57,7 @@ impl<T> SingleContainer<T> for Vec<T> {
 }
 
 /// Find the profession by the given name.
-fn find_profession(api: &mut Api, name: &str) -> MainResult<Profession> {
+fn find_profession(api: &mut Api, name: &str) -> Result<Profession> {
     let profession_ids = api.get_profession_ids()?;
     let lower_name = name.to_lowercase();
     let profession_id = profession_ids
@@ -110,7 +73,7 @@ fn find_profession(api: &mut Api, name: &str) -> MainResult<Profession> {
 ///
 /// `text` can either be a skill name, in which case all skills of the profession will be searched.
 /// Alternatively, it can also be a numeric ID, in which case it will be requested directly.
-fn resolve_skill(api: &mut Api, profession: &Profession, text: &str) -> MainResult<Skill> {
+fn resolve_skill(api: &mut Api, profession: &Profession, text: &str) -> Result<Skill> {
     // Try it as an ID first
     let numeric = text.parse::<u32>();
     if let Ok(num_id) = numeric {
@@ -135,7 +98,7 @@ fn resolve_skill(api: &mut Api, profession: &Profession, text: &str) -> MainResu
 /// Resolve a traitline.
 ///
 /// `text` must be in the `"name:choice1:choice2:choice3"` format.
-fn resolve_traitline(api: &mut Api, profession: &Profession, text: &str) -> MainResult<Traitline> {
+fn resolve_traitline(api: &mut Api, profession: &Profession, text: &str) -> Result<Traitline> {
     let parts = text.split(':').collect::<Vec<_>>();
     assert_eq!(
         parts.len(),
@@ -162,7 +125,7 @@ fn resolve_traitline(api: &mut Api, profession: &Profession, text: &str) -> Main
 }
 
 /// Create the build template by manually combining the given skills/traitlines from the CLI.
-fn run_searching(api: &mut Api, matches: &ArgMatches) -> MainResult<BuildTemplate> {
+fn run_searching(api: &mut Api, matches: &ArgMatches) -> Result<BuildTemplate> {
     let requested_profession = matches
         .value_of("profession")
         .expect("clap handles missing argument");
@@ -229,7 +192,7 @@ fn run_searching(api: &mut Api, matches: &ArgMatches) -> MainResult<BuildTemplat
 }
 
 /// Create the build template by parsing a chat link.
-fn run_chatlink(api: &mut Api, matches: &ArgMatches) -> MainResult<BuildTemplate> {
+fn run_chatlink(api: &mut Api, matches: &ArgMatches) -> Result<BuildTemplate> {
     let link = matches.value_of("chatlink").unwrap();
     Ok(BuildTemplate::from_chatlink(api, link)?)
 }
@@ -262,7 +225,7 @@ fn validate_legend(input: String) -> Result<(), String> {
         .map_err(|_| "invalid legend name".to_owned())
 }
 
-fn run() -> MainResult<()> {
+fn run() -> Result<()> {
     let matches = App::new(APP_NAME)
         .version("0.1")
         .author("Peter Parker IV")
@@ -375,8 +338,7 @@ fn run() -> MainResult<()> {
         }
         Err(RenderError::EmptyBuild) => (),
         Err(err) => {
-            eprintln!("Image could not be rendered:");
-            output::show_error(&err)?;
+            return Err(err).context("Image could not be rendered");
         }
     }
 
@@ -386,7 +348,7 @@ fn run() -> MainResult<()> {
 fn main() {
     let result = run();
     if let Err(e) = result {
-        output::show_error(e.as_ref()).expect("Error while displaying error");
+        output::show_error(e).expect("Error while displaying error");
         std::process::exit(1);
     }
 }