pkgs/mpdpopm: Switch error handling from snafu to anyhow

This is not a library, as such we can just use anyhow and provide better
and more concise errors to the user.

2 files changed, 48 insertions, 280 deletions

diff --git a/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopm.rs b/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopm.rs
index 82a354d6..d9d607d5 100644
--- a/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopm.rs
+++ b/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopm.rs
@@ -32,82 +32,12 @@ use mpdpopm::{
     storage::{last_played, play_count, rating_count},
 };
 
-use backtrace::Backtrace;
+use anyhow::{Context, Result, anyhow, bail};
 use clap::{Parser, Subcommand};
 use tracing::{debug, info, level_filters::LevelFilter, trace};
 use tracing_subscriber::{EnvFilter, Registry, layer::SubscriberExt};
 
-use std::{fmt, path::PathBuf};
-
-#[non_exhaustive]
-pub enum Error {
-    NoSubCommand,
-    NoConfigArg,
-    NoRating,
-    NoPlayCount,
-    NoLastPlayed,
-    NoConfig {
-        config: std::path::PathBuf,
-        cause: std::io::Error,
-    },
-    PlayerStopped,
-    BadPath {
-        path: PathBuf,
-        back: Backtrace,
-    },
-    NoPlaylist,
-    Client {
-        source: mpdpopm::clients::Error,
-        back: Backtrace,
-    },
-    Ratings {
-        source: Box<mpdpopm::storage::Error>,
-        back: Backtrace,
-    },
-    Playcounts {
-        source: Box<mpdpopm::storage::Error>,
-        back: Backtrace,
-    },
-    ExpectedInt {
-        source: std::num::ParseIntError,
-        back: Backtrace,
-    },
-    Config {
-        source: crate::config::Error,
-        back: Backtrace,
-    },
-}
-
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match self {
-            Error::NoSubCommand => write!(f, "No sub-command given"),
-            Error::NoConfigArg => write!(f, "No argument given for the configuration option"),
-            Error::NoRating => write!(f, "No rating supplied"),
-            Error::NoPlayCount => write!(f, "No play count supplied"),
-            Error::NoLastPlayed => write!(f, "No last played timestamp given"),
-            Error::NoConfig { config, cause } => write!(f, "Bad config ({:?}): {}", config, cause),
-            Error::PlayerStopped => write!(f, "The player is stopped"),
-            Error::BadPath { path, back: _ } => write!(f, "Bad path: {:?}", path),
-            Error::NoPlaylist => write!(f, "No playlist given"),
-            Error::Client { source, back: _ } => write!(f, "Client error: {}", source),
-            Error::Ratings { source, back: _ } => write!(f, "Rating error: {}", source),
-            Error::Playcounts { source, back: _ } => write!(f, "Playcount error: {}", source),
-            Error::ExpectedInt { source, back: _ } => write!(f, "Expected integer: {}", source),
-            Error::Config { source, back: _ } => {
-                write!(f, "Error reading configuration: {}", source)
-            }
-        }
-    }
-}
-
-impl fmt::Debug for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self)
-    }
-}
-
-type Result<T> = std::result::Result<T, Error>;
+use std::path::PathBuf;
 
 /// Map `tracks' argument(s) to a Vec of String containing one or more mpd URIs
 ///
@@ -132,20 +62,18 @@ async fn provide_file(client: &mut Client, maybe_file: Option<String>) -> Result
     let file = match maybe_file {
         Some(file) => file,
         None => {
-            match client.status().await.map_err(|err| Error::Client {
-                source: err,
-                back: Backtrace::new(),
-            })? {
+            match client
+                .status()
+                .await
+                .context("Failed to get status of client")?
+            {
                 PlayerStatus::Play(curr) | PlayerStatus::Pause(curr) => curr
                     .file
                     .to_str()
-                    .ok_or_else(|| Error::BadPath {
-                        path: curr.file.clone(),
-                        back: Backtrace::new(),
-                    })?
+                    .ok_or_else(|| anyhow!("Path is not utf8: `{}`", curr.file.display()))?
                     .to_string(),
                 PlayerStatus::Stopped => {
-                    return Err(Error::PlayerStopped);
+                    bail!("Player is stopped");
                 }
             }
         }
@@ -163,12 +91,7 @@ async fn get_ratings(
     let mut ratings: Vec<(String, i8)> = Vec::new();
 
     for file in map_tracks(client, tracks).await? {
-        let rating = rating_count::get(client, &file)
-            .await
-            .map_err(|err| Error::Ratings {
-                source: Box::new(err),
-                back: Backtrace::new(),
-            })?;
+        let rating = rating_count::get(client, &file).await?;
 
         ratings.push((file, rating.unwrap_or_default()));
     }
@@ -189,12 +112,7 @@ async fn set_rating(client: &mut Client, rating: i8, arg: Option<String>) -> Res
     let is_current = arg.is_none();
     let file = provide_file(client, arg).await?;
 
-    rating_count::set(client, &file, rating)
-        .await
-        .map_err(|err| Error::Ratings {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    rating_count::set(client, &file, rating).await?;
 
     match is_current {
         false => info!("Set the rating for \"{}\" to \"{}\".", file, rating),
@@ -209,19 +127,9 @@ async fn inc_rating(client: &mut Client, arg: Option<String>) -> Result<()> {
     let is_current = arg.is_none();
     let file = provide_file(client, arg).await?;
 
-    let now = rating_count::get(client, &file)
-        .await
-        .map_err(|err| Error::Ratings {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    let now = rating_count::get(client, &file).await?;
 
-    rating_count::set(client, &file, now.unwrap_or_default().saturating_add(1))
-        .await
-        .map_err(|err| Error::Ratings {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    rating_count::set(client, &file, now.unwrap_or_default().saturating_add(1)).await?;
 
     match is_current {
         false => info!("Incremented the rating for \"{}\".", file),
@@ -236,19 +144,9 @@ async fn decr_rating(client: &mut Client, arg: Option<String>) -> Result<()> {
     let is_current = arg.is_none();
     let file = provide_file(client, arg).await?;
 
-    let now = rating_count::get(client, &file)
-        .await
-        .map_err(|err| Error::Ratings {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    let now = rating_count::get(client, &file).await?;
 
-    rating_count::set(client, &file, now.unwrap_or_default().saturating_sub(1))
-        .await
-        .map_err(|err| Error::Ratings {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    rating_count::set(client, &file, now.unwrap_or_default().saturating_sub(1)).await?;
 
     match is_current {
         false => info!("Decremented the rating for \"{}\".", file),
@@ -266,13 +164,7 @@ async fn get_play_counts(
 ) -> Result<()> {
     let mut playcounts: Vec<(String, usize)> = Vec::new();
     for file in map_tracks(client, tracks).await? {
-        let playcount = play_count::get(client, &file)
-            .await
-            .map_err(|err| Error::Playcounts {
-                source: Box::new(err),
-                back: Backtrace::new(),
-            })?
-            .unwrap_or_default();
+        let playcount = play_count::get(client, &file).await?.unwrap_or_default();
         playcounts.push((file, playcount));
     }
 
@@ -292,12 +184,7 @@ async fn set_play_counts(client: &mut Client, playcount: usize, arg: Option<Stri
     let is_current = arg.is_none();
     let file = provide_file(client, arg).await?;
 
-    play_count::set(client, &file, playcount)
-        .await
-        .map_err(|err| Error::Playcounts {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    play_count::set(client, &file, playcount).await?;
 
     match is_current {
         false => info!("Set the playcount for \"{}\" to \"{}\".", file, playcount),
@@ -318,13 +205,7 @@ async fn get_last_playeds(
 ) -> Result<()> {
     let mut lastplayeds: Vec<(String, Option<u64>)> = Vec::new();
     for file in map_tracks(client, tracks).await? {
-        let lastplayed =
-            last_played::get(client, &file)
-                .await
-                .map_err(|err| Error::Playcounts {
-                    source: Box::new(err),
-                    back: Backtrace::new(),
-                })?;
+        let lastplayed = last_played::get(client, &file).await?;
         lastplayeds.push((file, lastplayed));
     }
 
@@ -357,12 +238,7 @@ async fn set_last_playeds(client: &mut Client, lastplayed: u64, arg: Option<Stri
     let is_current = arg.is_none();
     let file = provide_file(client, arg).await?;
 
-    last_played::set(client, &file, lastplayed)
-        .await
-        .map_err(|err| Error::Playcounts {
-            source: Box::new(err),
-            back: Backtrace::new(),
-        })?;
+    last_played::set(client, &file, lastplayed).await?;
 
     match is_current {
         false => info!("Set last played for \"{}\" to \"{}\".", file, lastplayed),
@@ -377,13 +253,7 @@ async fn set_last_playeds(client: &mut Client, lastplayed: u64, arg: Option<Stri
 
 /// Retrieve the list of stored playlists
 async fn get_playlists(client: &mut Client) -> Result<()> {
-    let mut pls = client
-        .get_stored_playlists()
-        .await
-        .map_err(|err| Error::Client {
-            source: err,
-            back: Backtrace::new(),
-        })?;
+    let mut pls = client.get_stored_playlists().await?;
     pls.sort();
     println!("Stored playlists:");
     for pl in pls {
@@ -397,13 +267,7 @@ async fn findadd(client: &mut Client, chan: &str, filter: &str, case: bool) -> R
     let qfilter = quote(filter);
     debug!("findadd: got ``{}'', quoted to ``{}''.", filter, qfilter);
     let cmd = format!("{} {}", if case { "findadd" } else { "searchadd" }, qfilter);
-    client
-        .send_message(chan, &cmd)
-        .await
-        .map_err(|err| Error::Client {
-            source: err,
-            back: Backtrace::new(),
-        })?;
+    client.send_message(chan, &cmd).await?;
     Ok(())
 }
 
@@ -419,11 +283,7 @@ async fn send_command(client: &mut Client, chan: &str, args: Vec<String>) -> Res
                 .join(" ")
                 .as_str(),
         )
-        .await
-        .map_err(|err| Error::Client {
-            source: err,
-            back: Backtrace::new(),
-        })?;
+        .await?;
     Ok(())
 }
 
@@ -647,18 +507,17 @@ async fn main() -> Result<()> {
 
     let config = if let Some(configpath) = &args.config {
         match std::fs::read_to_string(configpath) {
-            Ok(text) => config::from_str(&text).map_err(|err| Error::Config {
-                source: err,
-                back: Backtrace::new(),
+            Ok(text) => config::from_str(&text).with_context(|| {
+                format!("Failed to parse config file at: `{}`", configpath.display())
             })?,
             Err(err) => {
                 // Either they did _not_, in which case they probably want to know that the config
                 // file they explicitly asked for does not exist, or there was some other problem,
                 // in which case we're out of options, anyway. Either way:
-                return Err(Error::NoConfig {
-                    config: PathBuf::from(configpath),
-                    cause: err,
-                });
+                bail!(
+                    "Failed to read config file at: `{}`, because: {err}",
+                    configpath.display()
+                )
             }
         }
     } else {
@@ -691,18 +550,10 @@ async fn main() -> Result<()> {
     trace!("logging configured.");
 
     let mut client = match config.conn {
-        config::Connection::Local { path } => {
-            Client::open(path).await.map_err(|err| Error::Client {
-                source: err,
-                back: Backtrace::new(),
-            })?
+        config::Connection::Local { path } => Client::open(path).await?,
+        config::Connection::TCP { host, port } => {
+            Client::connect(format!("{}:{}", host, port)).await?
         }
-        config::Connection::TCP { host, port } => Client::connect(format!("{}:{}", host, port))
-            .await
-            .map_err(|err| Error::Client {
-                source: err,
-                back: Backtrace::new(),
-            })?,
     };
 
     match args.command {
diff --git a/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopmd.rs b/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopmd.rs
index e903774c..643611d6 100644
--- a/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopmd.rs
+++ b/pkgs/by-name/mp/mpdpopm/src/bin/mpdpopmd.rs
@@ -25,94 +25,17 @@
 //! the sticker database, by invoking external commands to keep your tags up-to-date (something
 //! along the lines of [mpdcron](https://alip.github.io/mpdcron)).
 
-use mpdpopm::config;
-use mpdpopm::config::Config;
-use mpdpopm::mpdpopm;
+use mpdpopm::{
+    config::{self, Config},
+    mpdpopm,
+};
 
-use backtrace::Backtrace;
+use anyhow::{Context, Result, bail};
 use clap::Parser;
 use tracing::{info, level_filters::LevelFilter};
 use tracing_subscriber::{EnvFilter, Layer, Registry, layer::SubscriberExt};
 
-use std::{fmt, io, path::PathBuf, sync::MutexGuard};
-
-////////////////////////////////////////////////////////////////////////////////////////////////////
-//                                 mppopmd application Error type                                 //
-////////////////////////////////////////////////////////////////////////////////////////////////////
-
-#[non_exhaustive]
-pub enum Error {
-    NoConfigArg,
-    NoConfig {
-        config: std::path::PathBuf,
-        cause: std::io::Error,
-    },
-    Filter {
-        source: tracing_subscriber::filter::FromEnvError,
-        back: Backtrace,
-    },
-    Fork {
-        errno: errno::Errno,
-        back: Backtrace,
-    },
-    PathContainsNull {
-        back: Backtrace,
-    },
-    OpenLockFile {
-        errno: errno::Errno,
-        back: Backtrace,
-    },
-    LockFile {
-        errno: errno::Errno,
-        back: Backtrace,
-    },
-    WritePid {
-        errno: errno::Errno,
-        back: Backtrace,
-    },
-    Config {
-        source: crate::config::Error,
-        back: Backtrace,
-    },
-    MpdPopm {
-        source: Box<mpdpopm::Error>,
-        back: Backtrace,
-    },
-}
-
-impl std::fmt::Display for Error {
-    #[allow(unreachable_patterns)] // the _ arm is *currently* unreachable
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        match self {
-            Error::NoConfigArg => write!(f, "No configuration file given"),
-            Error::NoConfig { config, cause } => {
-                write!(f, "Configuration error ({:?}): {}", config, cause)
-            }
-            Error::Fork { errno, back: _ } => write!(f, "When forking, got errno {}", errno),
-            Error::PathContainsNull { back: _ } => write!(f, "Path contains a null character"),
-            Error::OpenLockFile { errno, back: _ } => {
-                write!(f, "While opening lock file, got errno {}", errno)
-            }
-            Error::LockFile { errno, back: _ } => {
-                write!(f, "While locking the lock file, got errno {}", errno)
-            }
-            Error::WritePid { errno, back: _ } => {
-                write!(f, "While writing pid file, got errno {}", errno)
-            }
-            Error::Config { source, back: _ } => write!(f, "Configuration error: {}", source),
-            Error::MpdPopm { source, back: _ } => write!(f, "mpdpopm error: {}", source),
-            _ => write!(f, "Unknown mppopmd error"),
-        }
-    }
-}
-
-impl fmt::Debug for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self)
-    }
-}
-
-type Result = std::result::Result<(), Error>;
+use std::{io, path::PathBuf, sync::MutexGuard};
 
 pub struct MyMutexGuardWriter<'a>(MutexGuard<'a, std::fs::File>);
 
@@ -164,22 +87,21 @@ struct Args {
     debug: bool,
 }
 
-/// Entry point for `mppopmd'.
+/// Entry point for `mpdopmd'.
 ///
 /// Do *not* use the #[tokio::main] attribute here! If this program is asked to daemonize (the usual
 /// case), we will fork after tokio has started its thread pool, with disastrous consequences.
 /// Instead, stay synchronous until we've daemonized (or figured out that we don't need to), and
 /// only then fire-up the tokio runtime.
-fn main() -> Result {
+fn main() -> Result<()> {
     use mpdpopm::vars::VERSION;
 
     let args = Args::parse();
 
     let config = if let Some(cfgpath) = &args.config {
         match std::fs::read_to_string(cfgpath) {
-            Ok(text) => config::from_str(&text).map_err(|err| Error::Config {
-                source: err,
-                back: Backtrace::new(),
+            Ok(text) => config::from_str(&text).with_context(|| {
+                format!("Failed to parse config file at: `{}`", cfgpath.display())
             })?,
             // The config file (defaulted or not) either didn't exist, or we were unable to read its
             // contents...
@@ -187,10 +109,10 @@ fn main() -> Result {
                 // Either they did _not_, in which case they probably want to know that the config
                 // file they explicitly asked for does not exist, or there was some other problem,
                 // in which case we're out of options, anyway. Either way:
-                return Err(Error::NoConfig {
-                    config: PathBuf::from(cfgpath),
-                    cause: err,
-                });
+                bail!(
+                    "No config file could be read at: `{}`, because: {err}",
+                    cfgpath.display()
+                )
             }
         }
     } else {
@@ -208,10 +130,7 @@ fn main() -> Result {
     let filter = EnvFilter::builder()
         .with_default_directive(lf.into())
         .from_env()
-        .map_err(|err| Error::Filter {
-            source: err,
-            back: Backtrace::new(),
-        })?;
+        .context("Failed to construct env filter")?;
 
     let formatter: Box<dyn Layer<Registry> + Send + Sync> = {
         Box::new(
@@ -226,8 +145,6 @@ fn main() -> Result {
 
     info!("mppopmd {VERSION} logging at level {lf:#?}.");
     let rt = tokio::runtime::Runtime::new().unwrap();
-    rt.block_on(mpdpopm(config)).map_err(|err| Error::MpdPopm {
-        source: Box::new(err),
-        back: Backtrace::new(),
-    })
+
+    rt.block_on(mpdpopm(config)).context("Main mpdpopm failed")
 }