fix: poor error handling for missing images

Previously images that could not be located were handled by throwing a full report error, which incorrectly stated it was an invalid image *type*. This changes the image handling to instead log a single-line warning directly in the image provider code, reducing the error handling required by each consumer. Resolves #146.
JakeStanger · May 20, 2023 · 87ca399 · 87ca399
1 parent 960da55
commit 87ca399
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 62 deletions.
diff --git a/src/image/gtk.rs b/src/image/gtk.rs
@@ -2,7 +2,6 @@ use super::ImageProvider;
 use crate::gtk_helpers::add_class;
 use gtk::prelude::*;
 use gtk::{Button, IconTheme, Image, Label, Orientation};
-use tracing::error;
 
 #[cfg(any(feature = "music", feature = "workspaces", feature = "clipboard"))]
 pub fn new_icon_button(input: &str, icon_theme: &IconTheme, size: i32) -> Button {
@@ -13,14 +12,13 @@ pub fn new_icon_button(input: &str, icon_theme: &IconTheme, size: i32) -> Button
         add_class(&image, "image");
 
         match ImageProvider::parse(input, icon_theme, size)
-            .and_then(|provider| provider.load_into_image(image.clone()))
+            .map(|provider| provider.load_into_image(image.clone()))
         {
-            Ok(_) => {
+            Some(_) => {
                 button.set_image(Some(&image));
                 button.set_always_show_image(true);
             }
-            Err(err) => {
-                error!("{err:?}");
+            None => {
                 button.set_label(input);
             }
         }
@@ -41,11 +39,8 @@ pub fn new_icon_label(input: &str, icon_theme: &IconTheme, size: i32) -> gtk::Bo
 
         container.add(&image);
 
-        if let Err(err) = ImageProvider::parse(input, icon_theme, size)
-            .and_then(|provider| provider.load_into_image(image))
-        {
-            error!("{err:?}");
-        }
+        ImageProvider::parse(input, icon_theme, size)
+            .map(|provider| provider.load_into_image(image));
     } else {
         let label = Label::new(Some(input));
         add_class(&label, "label");

diff --git a/src/image/provider.rs b/src/image/provider.rs
@@ -7,6 +7,7 @@ use gtk::gdk_pixbuf::Pixbuf;
 use gtk::prelude::*;
 use gtk::{IconLookupFlags, IconTheme};
 use std::path::{Path, PathBuf};
+use tracing::warn;
 
 cfg_if!(
     if #[cfg(feature = "http")] {
@@ -40,9 +41,9 @@ impl<'a> ImageProvider<'a> {
     ///
     /// Note this checks that icons exist in theme, or files exist on disk
     /// but no other check is performed.
-    pub fn parse(input: &str, theme: &'a IconTheme, size: i32) -> Result<Self> {
+    pub fn parse(input: &str, theme: &'a IconTheme, size: i32) -> Option<Self> {
         let location = Self::get_location(input, theme, size)?;
-        Ok(Self { location, size })
+        Some(Self { location, size })
     }
 
     /// Returns true if the input starts with a prefix
@@ -56,44 +57,56 @@ impl<'a> ImageProvider<'a> {
             || input.starts_with("https://")
     }
 
-    fn get_location(input: &str, theme: &'a IconTheme, size: i32) -> Result<ImageLocation<'a>> {
+    fn get_location(input: &str, theme: &'a IconTheme, size: i32) -> Option<ImageLocation<'a>> {
         let (input_type, input_name) = input
             .split_once(':')
             .map_or((None, input), |(t, n)| (Some(t), n));
 
         match input_type {
-            Some(input_type) if input_type == "icon" => Ok(ImageLocation::Icon {
+            Some(input_type) if input_type == "icon" => Some(ImageLocation::Icon {
                 name: input_name.to_string(),
                 theme,
             }),
-            Some(input_type) if input_type == "file" => Ok(ImageLocation::Local(PathBuf::from(
+            Some(input_type) if input_type == "file" => Some(ImageLocation::Local(PathBuf::from(
                 input_name[2..].to_string(),
             ))),
             #[cfg(feature = "http")]
             Some(input_type) if input_type == "http" || input_type == "https" => {
-                Ok(ImageLocation::Remote(input.parse()?))
+                input.parse().ok().map(ImageLocation::Remote)
             }
-            None if input.starts_with("steam_app_") => Ok(ImageLocation::Steam(
+            None if input.starts_with("steam_app_") => Some(ImageLocation::Steam(
                 input_name.chars().skip("steam_app_".len()).collect(),
             )),
             None if theme
                 .lookup_icon(input, size, IconLookupFlags::empty())
                 .is_some() =>
             {
-                Ok(ImageLocation::Icon {
+                Some(ImageLocation::Icon {
                     name: input_name.to_string(),
                     theme,
                 })
             }
-            Some(input_type) => Err(Report::msg(format!("Unsupported image type: {input_type}"))
-                .note("You may need to recompile with support if available")),
+            Some(input_type) => {
+                warn!(
+                    "{:?}",
+                    Report::msg(format!("Unsupported image type: {input_type}"))
+                        .note("You may need to recompile with support if available")
+                );
+                None
+            }
             None if PathBuf::from(input_name).is_file() => {
-                Ok(ImageLocation::Local(PathBuf::from(input_name)))
+                Some(ImageLocation::Local(PathBuf::from(input_name)))
+            }
+            None => {
+                if let Some(location) = get_desktop_icon_name(input_name)
+                    .map(|input| Self::get_location(&input, theme, size))
+                {
+                    location
+                } else {
+                    warn!("Failed to find image: {input}");
+                    None
+                }
             }
-            None => get_desktop_icon_name(input_name).map_or_else(
-                || Err(Report::msg(format!("Unknown image type: '{input}'"))),
-                |input| Self::get_location(&input, theme, size),
-            ),
         }
     }
 

diff --git a/src/modules/custom/image.rs b/src/modules/custom/image.rs
@@ -5,7 +5,6 @@ use crate::image::ImageProvider;
 use gtk::prelude::*;
 use gtk::Image;
 use serde::Deserialize;
-use tracing::error;
 
 #[derive(Debug, Deserialize, Clone)]
 pub struct ImageWidget {
@@ -31,12 +30,8 @@ impl CustomWidget for ImageWidget {
             let icon_theme = context.icon_theme.clone();
 
             DynamicString::new(&self.src, move |src| {
-                let res = ImageProvider::parse(&src, &icon_theme, self.size)
-                    .and_then(|image| image.load_into_image(gtk_image.clone()));
-
-                if let Err(err) = res {
-                    error!("{err:?}");
-                }
+                ImageProvider::parse(&src, &icon_theme, self.size)
+                    .map(|image| image.load_into_image(gtk_image.clone()));
 
                 Continue(true)
             });

diff --git a/src/modules/focused.rs b/src/modules/focused.rs
@@ -11,7 +11,7 @@ use gtk::Label;
 use serde::Deserialize;
 use tokio::spawn;
 use tokio::sync::mpsc::{Receiver, Sender};
-use tracing::{debug, error};
+use tracing::debug;
 
 #[derive(Debug, Deserialize, Clone)]
 pub struct FocusedModule {
@@ -113,11 +113,8 @@ impl Module<gtk::Box> for FocusedModule {
             let icon_theme = icon_theme.clone();
             context.widget_rx.attach(None, move |(name, id)| {
                 if self.show_icon {
-                    if let Err(err) = ImageProvider::parse(&id, &icon_theme, self.icon_size)
-                        .and_then(|image| image.load_into_image(icon.clone()))
-                    {
-                        error!("{err:?}");
-                    }
+                    ImageProvider::parse(&id, &icon_theme, self.icon_size)
+                        .map(|image| image.load_into_image(icon.clone()));
                 }
 
                 if self.show_title {

diff --git a/src/modules/launcher/item.rs b/src/modules/launcher/item.rs
@@ -192,16 +192,13 @@ impl ItemButton {
             let gtk_image = gtk::Image::new();
             let image =
                 ImageProvider::parse(&item.app_id.clone(), icon_theme, appearance.icon_size);
-            match image {
-                Ok(image) => {
-                    button.set_image(Some(&gtk_image));
-                    button.set_always_show_image(true);
-
-                    if let Err(err) = image.load_into_image(gtk_image) {
-                        error!("{err:?}");
-                    }
+            if let Some(image) = image {
+                button.set_image(Some(&gtk_image));
+                button.set_always_show_image(true);
+
+                if let Err(err) = image.load_into_image(gtk_image) {
+                    error!("{err:?}");
                 }
-                Err(err) => error!("{err:?}"),
             };
         }
 

diff --git a/src/modules/music/mod.rs b/src/modules/music/mod.rs
@@ -342,19 +342,15 @@ impl Module<Button> for MusicModule {
                     let new_cover = update.song.cover_path;
                     if prev_cover != new_cover {
                         prev_cover = new_cover.clone();
-                        let res = match new_cover.map(|cover_path| {
+                        let res = if let Some(image) = new_cover.and_then(|cover_path| {
                             ImageProvider::parse(&cover_path, &icon_theme, image_size)
                         }) {
-                            Some(Ok(image)) => image.load_into_image(album_image.clone()),
-                            Some(Err(err)) => {
-                                album_image.set_from_pixbuf(None);
-                                Err(err)
-                            }
-                            None => {
-                                album_image.set_from_pixbuf(None);
-                                Ok(())
-                            }
+                            image.load_into_image(album_image.clone())
+                        } else {
+                            album_image.set_from_pixbuf(None);
+                            Ok(())
                         };
+
                         if let Err(err) = res {
                             error!("{err:?}");
                         }

diff --git a/src/modules/upower.rs b/src/modules/upower.rs
@@ -180,11 +180,8 @@ impl Module<gtk::Box> for UpowerModule {
             .attach(None, move |properties: UpowerProperties| {
                 let format = format.replace("{percentage}", &properties.percentage.to_string());
                 let icon_name = String::from("icon:") + &properties.icon_name;
-                if let Err(err) = ImageProvider::parse(&icon_name, &icon_theme, 32)
-                    .and_then(|provider| provider.load_into_image(icon.clone()))
-                {
-                    error!("{err:?}");
-                }
+                ImageProvider::parse(&icon_name, &icon_theme, 32)
+                    .map(|provider| provider.load_into_image(icon.clone()));
                 label.set_markup(format.as_ref());
                 Continue(true)
             });