Skip to main content

Create a proper ThumbnailError type and reduce use of unwrap()

ID
4d27c2c
date
2025-09-08 12:43:59+00:00
author
Alex Chan <alex@alexwlchan.net>
parent
c6ba7df
message
Create a proper `ThumbnailError` type and reduce use of `unwrap()`
changed files
5 files, 94 additions, 33 deletions

Changed files

src/create_parent_directory.rs (1216) → src/create_parent_directory.rs (1212)

diff --git a/src/create_parent_directory.rs b/src/create_parent_directory.rs
index 8be75e0..771cefe 100644
--- a/src/create_parent_directory.rs
+++ b/src/create_parent_directory.rs
@@ -1,5 +1,5 @@
 use std::fs;
-use std::io::Result;
+use std::io;
 use std::path::PathBuf;
 
 /// Create the parent directory of a given path.
@@ -9,7 +9,7 @@ use std::path::PathBuf;
 ///     create_parent_directory("path/to/images/index.html")
 ///      ~> creates "path/to/images/"
 ///
-pub fn create_parent_directory(path: &PathBuf) -> Result<()> {
+pub fn create_parent_directory(path: &PathBuf) -> io::Result<()> {
     // Quoting from the Rust docs for PathBuf.parent() [1]:
     //
     //     Returns None if the path terminates in a root or prefix,

src/create_thumbnail.rs (7868) → src/create_thumbnail.rs (7988)

diff --git a/src/create_thumbnail.rs b/src/create_thumbnail.rs
index 6650362..4ca09e6 100644
--- a/src/create_thumbnail.rs
+++ b/src/create_thumbnail.rs
@@ -1,4 +1,3 @@
-use std::io;
 use std::path::PathBuf;
 use std::process::Command;
 use std::str;
@@ -6,6 +5,7 @@ use std::str;
 use image::imageops::FilterType;
 
 use crate::create_parent_directory::create_parent_directory;
+use crate::errors::ThumbnailError;
 use crate::get_thumbnail_dimensions::{get_thumbnail_dimensions, TargetDimension};
 use crate::is_animated_gif::is_animated_gif;
 
@@ -15,21 +15,17 @@ pub fn create_thumbnail(
     path: &PathBuf,
     out_dir: &PathBuf,
     target: TargetDimension,
-) -> io::Result<PathBuf> {
-    let thumbnail_path = out_dir.join(path.file_name().unwrap());
+) -> Result<PathBuf, ThumbnailError> {
+    let file_name = path.file_name().ok_or(ThumbnailError::MissingFileName)?;
+    let thumbnail_path = out_dir.join(file_name);
     create_parent_directory(&thumbnail_path)?;
 
     // Make sure we don't overwrite the original image with a thumbnail
     if *path == thumbnail_path {
-        return Err(io::Error::new(
-            io::ErrorKind::InvalidData,
-            "Cannot write thumbnail to the same directory as the original image",
-        ));
+        return Err(ThumbnailError::SameInputOutputPath);
     }
-    assert!(*path != thumbnail_path);
 
-    let (new_width, new_height) = get_thumbnail_dimensions(&path, target)
-        .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()))?;
+    let (new_width, new_height) = get_thumbnail_dimensions(&path, target)?;
 
     if is_animated_gif(path)? {
         create_animated_gif_thumbnail(path, out_dir, new_width, new_height)
@@ -174,10 +170,20 @@ pub fn create_animated_gif_thumbnail(
     out_dir: &PathBuf,
     width: u32,
     height: u32,
-) -> io::Result<PathBuf> {
-    let file_name = gif_path.file_name().unwrap();
+) -> Result<PathBuf, ThumbnailError> {
+    let file_name = gif_path
+        .file_name()
+        .ok_or(ThumbnailError::MissingFileName)?;
+
     let thumbnail_path = out_dir.join(file_name).with_extension("mp4");
 
+    let gif_path_str = gif_path
+        .to_str()
+        .ok_or(ThumbnailError::PathConversionError)?;
+    let thumbnail_path_str = thumbnail_path
+        .to_str()
+        .ok_or(ThumbnailError::PathConversionError)?;
+
     // There's a subtlety here with ffmpeg I don't understand fully -- if
     // the width/height aren't even, it doesn't create the MP4, instead
     // failing with the error:
@@ -191,26 +197,23 @@ pub fn create_animated_gif_thumbnail(
     let output = Command::new("ffmpeg")
         .args([
             "-i",
-            gif_path.to_str().unwrap(),
+            gif_path_str,
             "-movflags",
             "faststart",
             "-pix_fmt",
             "yuv420p",
             "-vf",
             &dimension_str,
-            thumbnail_path.to_str().unwrap(),
+            thumbnail_path_str,
         ])
         .output()
-        .expect("failed to create thumbnail");
+        .map_err(|e| ThumbnailError::CommandFailed(format!("Failed to run ffmpeg: {}", e)))?;
 
     if output.status.success() {
         Ok(thumbnail_path)
     } else {
-        let error_message = format!(
-            "Unable to invoke ffmpeg!\nstderr from ffmpeg:\n{}",
-            str::from_utf8(&output.stderr).unwrap()
-        );
-        Err(io::Error::new(io::ErrorKind::Other, error_message))
+        let stderr = str::from_utf8(&output.stderr)?;
+        Err(ThumbnailError::CommandFailed(stderr.to_string()))
     }
 }
 
@@ -218,22 +221,23 @@ pub fn create_animated_gif_thumbnail(
 ///
 /// This function assumes that the original image file definitely exists.
 ///
-/// TODO: Get rid of the use of `unwrap()` in this code.
-///
 pub fn create_static_thumbnail(
     image_path: &PathBuf,
     out_dir: &PathBuf,
     width: u32,
     height: u32,
-) -> io::Result<PathBuf> {
-    let file_name = image_path.file_name().unwrap();
+) -> Result<PathBuf, ThumbnailError> {
+    let file_name = image_path
+        .file_name()
+        .ok_or(ThumbnailError::MissingFileName)?;
+
     let thumbnail_path = out_dir.join(file_name);
 
-    let img = image::open(image_path).unwrap();
+    let img = image::open(image_path).map_err(ThumbnailError::ImageOpenError)?;
 
     img.resize(width, height, FilterType::Lanczos3)
         .save(&thumbnail_path)
-        .unwrap();
+        .map_err(ThumbnailError::ImageSaveError)?;
 
     Ok(thumbnail_path)
 }

src/errors.rs (0) → src/errors.rs (1737)

diff --git a/src/errors.rs b/src/errors.rs
new file mode 100644
index 0000000..f01db6c
--- /dev/null
+++ b/src/errors.rs
@@ -0,0 +1,54 @@
+use std::fmt;
+use std::io;
+
+use image::ImageError;
+
+#[derive(Debug)]
+pub enum ThumbnailError {
+    MissingFileName,
+    ImageOpenError(ImageError),
+    ImageSaveError(ImageError),
+    CommandFailed(String),
+    Utf8Error(std::str::Utf8Error),
+    PathConversionError,
+    SameInputOutputPath,
+    IoError(std::io::Error),
+}
+
+impl fmt::Display for ThumbnailError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            ThumbnailError::MissingFileName => write!(f, "Image path is missing a file name"),
+            ThumbnailError::ImageOpenError(e) => write!(f, "Failed to open image: {}", e),
+            ThumbnailError::ImageSaveError(e) => write!(f, "Failed to save thumbnail: {}", e),
+            ThumbnailError::CommandFailed(msg) => write!(f, "ffmpeg command failed: {}", msg),
+            ThumbnailError::Utf8Error(e) => write!(f, "Failed to decode ffmpeg output: {}", e),
+            ThumbnailError::PathConversionError => write!(f, "Failed to convert path to string"),
+            ThumbnailError::SameInputOutputPath => write!(
+                f,
+                "Cannot write thumbnail to the same path as the original image"
+            ),
+            ThumbnailError::IoError(e) => write!(f, "I/O error: {}", e),
+        }
+    }
+}
+
+impl std::error::Error for ThumbnailError {}
+
+impl From<ImageError> for ThumbnailError {
+    fn from(err: ImageError) -> Self {
+        ThumbnailError::ImageOpenError(err)
+    }
+}
+
+impl From<std::str::Utf8Error> for ThumbnailError {
+    fn from(err: std::str::Utf8Error) -> Self {
+        ThumbnailError::Utf8Error(err)
+    }
+}
+
+impl From<io::Error> for ThumbnailError {
+    fn from(err: io::Error) -> Self {
+        ThumbnailError::IoError(err)
+    }
+}

src/get_thumbnail_dimensions.rs (3810) → src/get_thumbnail_dimensions.rs (3836)

diff --git a/src/get_thumbnail_dimensions.rs b/src/get_thumbnail_dimensions.rs
index 95c9f1e..ae71c1d 100644
--- a/src/get_thumbnail_dimensions.rs
+++ b/src/get_thumbnail_dimensions.rs
@@ -2,6 +2,8 @@ use std::path::PathBuf;
 
 use image::GenericImageView;
 
+use crate::errors::ThumbnailError;
+
 /// Represents the target dimensions of the thumbnail.
 ///
 /// The user can choose a max width, or a max height, but not both.
@@ -26,7 +28,7 @@ pub enum TargetDimension {
 pub fn get_thumbnail_dimensions(
     path: &PathBuf,
     target: TargetDimension,
-) -> Result<(u32, u32), image::error::ImageError> {
+) -> Result<(u32, u32), ThumbnailError> {
     let img = image::open(path)?;
 
     let (new_width, new_height) = match target {

src/main.rs (6436) → src/main.rs (6498)

diff --git a/src/main.rs b/src/main.rs
index 827fdb7..1de30d1 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6,6 +6,7 @@ use clap::{ArgGroup, Parser};
 
 mod create_parent_directory;
 mod create_thumbnail;
+mod errors;
 mod get_thumbnail_dimensions;
 mod is_animated_gif;
 
@@ -136,7 +137,7 @@ mod test_cli {
             .failure()
             .code(1)
             .stdout("")
-            .stderr("No such file or directory (os error 2)\n");
+            .stderr("Failed to open image: No such file or directory (os error 2)\n");
     }
 
     #[test]
@@ -148,7 +149,7 @@ mod test_cli {
             .failure()
             .code(1)
             .stdout("")
-            .stderr("The file extension `.\"toml\"` was not recognized as an image format\n");
+            .stderr("Failed to open image: The file extension `.\"toml\"` was not recognized as an image format\n");
     }
 
     // TODO: Improve this error message.
@@ -164,7 +165,7 @@ mod test_cli {
             .failure()
             .code(1)
             .stdout("")
-            .stderr("File exists (os error 17)\n");
+            .stderr("I/O error: File exists (os error 17)\n");
     }
 
     #[test]
@@ -176,7 +177,7 @@ mod test_cli {
             .failure()
             .code(1)
             .stdout("")
-            .stderr("Cannot write thumbnail to the same directory as the original image\n");
+            .stderr("Cannot write thumbnail to the same path as the original image\n");
     }
 
     #[test]