improve logging of mpd parse failures

src/mpd/client.rs

@@ -16,7 +16,7 @@ use log::debug;
 
 type MpdResult<T> = Result<T, MpdError>;
 
-const MAX_SUPPORTED_VERSION: Version = Version {
+const MIN_SUPPORTED_VERSION: Version = Version {
     major: 0,
     minor: 23,
     patch: 5,
@@ -128,9 +128,9 @@ impl<'name> Client<'name> {
 
         debug!(name, version = version.to_string().as_str(), handshake = buf.trim(); "MPD client initiazed");
 
-        if version > MAX_SUPPORTED_VERSION {
+        if version < MIN_SUPPORTED_VERSION {
             status_warn!(
-                "MPD version '{version}' is higher than supported. Maximum supported protocol version is '{MAX_SUPPORTED_VERSION}'. Some features may work incorrectly."
+                "MPD version '{version}' is lower than supported. Minimum supported protocol version is '{MIN_SUPPORTED_VERSION }'. Some features may work incorrectly."
             );
         }
 

src/mpd/commands/current_song.rs

@@ -2,7 +2,7 @@ use std::{collections::HashMap, time::Duration};
 
 use serde::Serialize;
 
-use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
+use crate::mpd::{errors::MpdError, FromMpd, LineHandled, ParseErrorExt};
 
 #[derive(Default, Serialize, PartialEq, Eq, Clone)]
 pub struct Song {
@@ -44,9 +44,9 @@ impl FromMpd for Song {
     fn next_internal(&mut self, key: &str, value: String) -> Result<LineHandled, MpdError> {
         match key {
             "file" => self.file = value,
-            "id" => self.id = value.parse()?,
+            "id" => self.id = value.parse().logerr(key, &value)?,
             "duration" => {
-                self.duration = Some(Duration::from_secs_f64(value.parse()?));
+                self.duration = Some(Duration::from_secs_f64(value.parse().logerr(key, &value)?));
             }
             "time" | "format" => {} // deprecated or ignored
             key => {

src/mpd/commands/list_files.rs

@@ -1,7 +1,8 @@
+use anyhow::anyhow;
 use anyhow::Context;
 use derive_more::{AsMut, AsRef, Into, IntoIterator};
 
-use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
+use crate::mpd::{errors::MpdError, FromMpd, LineHandled, ParseErrorExt};
 
 // file: 03 Diode.flac
 // size: 18183774
@@ -13,7 +14,7 @@ pub struct Listed {
     pub kind: ListingType,
     pub name: String,
     pub size: u64,
-    pub last_modified: String, // TODO timestamp?
+    pub last_modified: String,
 }
 
 #[derive(Debug, Default)]
@@ -31,7 +32,11 @@ impl FromMpd for ListFiles {
 
         self.0
             .last_mut()
-            .context("No element in accumulator while parsing ListFiles")?
+            .context(anyhow!(
+                "No element in accumulator while parsing ListFiles. Key '{}' Value :'{}'",
+                key,
+                value
+            ))?
             .next_internal(key, value)
     }
 }
@@ -47,7 +52,7 @@ impl FromMpd for Listed {
                 self.kind = ListingType::Dir;
                 self.name = value;
             }
-            "size" => self.size = value.parse()?,
+            "size" => self.size = value.parse().logerr(key, &value)?,
             "last-modified" => self.last_modified = value,
             _ => return Ok(LineHandled::No { value }),
         }

src/mpd/commands/list_mounts.rs

@@ -1,3 +1,4 @@
+use anyhow::anyhow;
 use anyhow::Context;
 use derive_more::{AsMut, AsRef, Into, IntoIterator};
 use serde::Serialize;
@@ -21,7 +22,11 @@ impl FromMpd for Mounts {
 
         self.0
             .last_mut()
-            .context("No element in accumulator while parsing Mounts")?
+            .context(anyhow!(
+                "No element in accumulator while parsing Mounts. Key '{}' Value :'{}'",
+                key,
+                value
+            ))?
             .next_internal(key, value)
     }
 }

src/mpd/commands/list_playlists.rs

@@ -1,3 +1,4 @@
+use anyhow::anyhow;
 use anyhow::Context;
 
 use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
@@ -28,7 +29,11 @@ impl FromMpd for Vec<Playlist> {
         }
 
         self.last_mut()
-            .context("No element in accumulator while parsing Playlists")?
+            .context(anyhow!(
+                "No element in accumulator while parsing Playlists. Key '{}' Value :'{}'",
+                key,
+                value
+            ))?
             .next_internal(key, value)
     }
 }

src/mpd/commands/lsinfo.rs

@@ -1,6 +1,7 @@
 use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
 
 use super::Song;
+use anyhow::anyhow;
 use anyhow::Context;
 use derive_more::{AsMut, AsRef, Into, IntoIterator};
 
@@ -29,7 +30,7 @@ impl FromMpd for Dir {
                 value
                     .split('/')
                     .last()
-                    .context("Failed to parse dir name.")?
+                    .context(anyhow!("Failed to parse dir name. Key: '{}' Value: '{}'", key, value))?
                     .clone_into(&mut self.path);
                 self.full_path = value;
             }
@@ -50,11 +51,11 @@ impl FromMpd for LsInfo {
             self.0.push(FileOrDir::Dir(Dir::default()));
         }
 
-        match self
-            .0
-            .last_mut()
-            .context("No element in accumulator while parsing LsInfo")?
-        {
+        match self.0.last_mut().context(anyhow!(
+            "No element in accumulator while parsing LsInfo. Key '{}' Value :'{}'",
+            key,
+            value
+        ))? {
             FileOrDir::Dir(dir) => dir.next_internal(key, value),
             FileOrDir::File(song) => song.next_internal(key, value),
         }

src/mpd/commands/outputs.rs

@@ -1,8 +1,9 @@
+use anyhow::anyhow;
 use anyhow::Context;
 use derive_more::{AsMut, AsRef, Into, IntoIterator};
 use serde::Serialize;
 
-use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
+use crate::mpd::{errors::MpdError, FromMpd, LineHandled, ParseErrorExt};
 
 #[derive(Debug, Serialize, Default, IntoIterator, AsRef, AsMut, Into)]
 pub struct Outputs(pub Vec<Output>);
@@ -22,15 +23,19 @@ impl FromMpd for Outputs {
 
         self.0
             .last_mut()
-            .context("No element in accumulator while parsing Outputs")?
+            .context(anyhow!(
+                "No element in accumulator while parsing Outputs. Key '{}' Value :'{}'",
+                key,
+                value
+            ))?
             .next_internal(key, value)
     }
 }
 
 impl FromMpd for Output {
     fn next_internal(&mut self, key: &str, value: String) -> Result<LineHandled, MpdError> {
         match key {
-            "outputid" => self.id = value.parse()?,
+            "outputid" => self.id = value.parse().logerr(key, &value)?,
             "outputname" => self.name = value,
             "outputenabled" => match value.as_str() {
                 "0" => self.enabled = false,

src/mpd/commands/playlist_info.rs

@@ -1,6 +1,7 @@
 use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
 
 use super::Song;
+use anyhow::anyhow;
 use anyhow::Context;
 
 impl FromMpd for Vec<Song> {
@@ -9,7 +10,11 @@ impl FromMpd for Vec<Song> {
             self.push(Song::default());
         }
         self.last_mut()
-            .context("No element in accumulator while parsing PlayListInfo")?
+            .context(anyhow!(
+                "No element in accumulator while parsing PlayListInfo. Key '{}' Value :'{}'",
+                key,
+                value
+            ))?
             .next_internal(key, value)
     }
 }

src/mpd/commands/status.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 use anyhow::anyhow;
 use serde::Serialize;
 
-use crate::mpd::{errors::MpdError, FromMpd, LineHandled};
+use crate::mpd::{errors::MpdError, FromMpd, LineHandled, ParseErrorExt};
 
 use super::Volume;
 
@@ -37,26 +37,26 @@ impl FromMpd for Status {
     fn next_internal(&mut self, key: &str, value: String) -> Result<LineHandled, MpdError> {
         match key {
             "partition" => self.partition = value,
-            "volume" => self.volume = Volume::new(value.parse()?),
+            "volume" => self.volume = Volume::new(value.parse().logerr(key, &value)?),
             "repeat" => self.repeat = value != "0",
             "random" => self.random = value != "0",
-            "single" => self.single = value.parse()?,
-            "consume" => self.consume = value.parse()?,
-            "playlist" => self.playlist = Some(value.parse()?),
+            "single" => self.single = value.parse().logerr(key, &value)?,
+            "consume" => self.consume = value.parse().logerr(key, &value)?,
+            "playlist" => self.playlist = Some(value.parse().logerr(key, &value)?),
             "playlistlength" => self.playlistlength = value.parse()?,
-            "state" => self.state = value.parse()?,
-            "song" => self.song = Some(value.parse()?),
-            "songid" => self.songid = Some(value.parse()?),
-            "nextsong" => self.nextsong = Some(value.parse()?),
-            "nextsongid" => self.nextsongid = Some(value.parse()?),
-            "elapsed" => self.elapsed = Duration::from_secs_f32(value.parse()?),
-            "duration" => self.duration = Duration::from_secs_f32(value.parse()?),
-            "bitrate" if value != "0" => self.bitrate = Some(value.parse()?),
-            "xfade" => self.xfade = Some(value.parse()?),
+            "state" => self.state = value.parse().logerr(key, &value)?,
+            "song" => self.song = Some(value.parse().logerr(key, &value)?),
+            "songid" => self.songid = Some(value.parse().logerr(key, &value)?),
+            "nextsong" => self.nextsong = Some(value.parse().logerr(key, &value)?),
+            "nextsongid" => self.nextsongid = Some(value.parse().logerr(key, &value)?),
+            "elapsed" => self.elapsed = Duration::from_secs_f32(value.parse().logerr(key, &value)?),
+            "duration" => self.duration = Duration::from_secs_f32(value.parse().logerr(key, &value)?),
+            "bitrate" if value != "0" => self.bitrate = Some(value.parse().logerr(key, &value)?),
+            "xfade" => self.xfade = Some(value.parse().logerr(key, &value)?),
             "mixrampdb" => self.mixrampdb = Some(value),
             "mixrampdelay" => self.mixrampdelay = Some(value),
             "audio" => self.audio = Some(value),
-            "updating_db" => self.updating_db = Some(value.parse()?),
+            "updating_db" => self.updating_db = Some(value.parse().logerr(key, &value)?),
             "error" => self.error = Some(value),
             "bitrate" => self.bitrate = None,
             "time" => {} // deprecated

src/mpd/mod.rs

@@ -40,3 +40,32 @@ enum LineHandled {
     Yes,
     No { value: String },
 }
+
+pub trait ParseErrorExt {
+    fn logerr(self, key: &str, value: &str) -> Self;
+}
+
+impl<T: std::str::FromStr> ParseErrorExt for Result<T, anyhow::Error> {
+    fn logerr(self, key: &str, value: &str) -> Self {
+        if self.is_err() {
+            log::error!(key, value; "Failed to parse value");
+        }
+        self
+    }
+}
+impl<T: std::str::FromStr> ParseErrorExt for Result<T, std::num::ParseFloatError> {
+    fn logerr(self, key: &str, value: &str) -> Self {
+        if self.is_err() {
+            log::error!(key, value; "Failed to parse value");
+        }
+        self
+    }
+}
+impl<T: std::str::FromStr> ParseErrorExt for Result<T, std::num::ParseIntError> {
+    fn logerr(self, key: &str, value: &str) -> Self {
+        if self.is_err() {
+            log::error!(key, value; "Failed to parse value");
+        }
+        self
+    }
+}