From 5e8ab7856db1ced9a071dda995ea5e06e4d6c658 Mon Sep 17 00:00:00 2001 From: asonix Date: Mon, 17 Jul 2023 13:30:08 -0500 Subject: [PATCH] Extract Status errors into command-specific errors --- src/discover/exiftool.rs | 3 +-- src/discover/ffmpeg.rs | 13 +++++-------- src/discover/magick.rs | 6 ++---- src/exiftool.rs | 20 +++++++++++++++----- src/ffmpeg.rs | 19 +++++++++++++++---- src/magick.rs | 18 ++++++++++++++---- src/process.rs | 6 +++--- src/validate/exiftool.rs | 3 +-- src/validate/ffmpeg.rs | 12 ++++-------- src/validate/magick.rs | 6 ++---- 10 files changed, 62 insertions(+), 44 deletions(-) diff --git a/src/discover/exiftool.rs b/src/discover/exiftool.rs index a194827..b291454 100644 --- a/src/discover/exiftool.rs +++ b/src/discover/exiftool.rs @@ -40,8 +40,7 @@ pub(super) async fn check_reorient( #[tracing::instrument(level = "trace", skip(input))] async fn needs_reorienting(input: Bytes) -> Result { - let process = - Process::run("exiftool", &["-n", "-Orientation", "-"]).map_err(ExifError::Process)?; + let process = Process::run("exiftool", &["-n", "-Orientation", "-"])?; let mut reader = process.bytes_read(input); let mut buf = String::new(); diff --git a/src/discover/ffmpeg.rs b/src/discover/ffmpeg.rs index 82d6ede..51d4e75 100644 --- a/src/discover/ffmpeg.rs +++ b/src/discover/ffmpeg.rs @@ -6,8 +6,8 @@ use std::{collections::HashSet, sync::OnceLock}; use crate::{ ffmpeg::FfMpegError, formats::{ - AnimationFormat, ImageFormat, ImageInput, InputFile, InternalFormat, - InternalVideoFormat, VideoFormat, + AnimationFormat, ImageFormat, ImageInput, InputFile, InternalFormat, InternalVideoFormat, + VideoFormat, }, process::Process, }; @@ -220,8 +220,7 @@ where "json", input_file_str, ], - ) - .map_err(FfMpegError::Process)?; + )?; let mut output = Vec::new(); process @@ -268,8 +267,7 @@ where "compact=p=0:nk=1", input_file_str, ], - ) - .map_err(FfMpegError::Process)?; + )?; let mut output = Vec::new(); process @@ -298,8 +296,7 @@ async fn alpha_pixel_formats() -> Result, FfMpegError> { "-print_format", "json", ], - ) - .map_err(FfMpegError::Process)?; + )?; let mut output = Vec::new(); process diff --git a/src/discover/magick.rs b/src/discover/magick.rs index 88b7562..32b0de6 100644 --- a/src/discover/magick.rs +++ b/src/discover/magick.rs @@ -137,8 +137,7 @@ where let tmp_one = (f)(tmp_one).await?; tmp_one.close().await.map_err(MagickError::CloseFile)?; - let process = Process::run("magick", &["convert", "-ping", input_file_str, "INFO:"]) - .map_err(MagickError::Process)?; + let process = Process::run("magick", &["convert", "-ping", input_file_str, "INFO:"])?; let mut output = String::new(); process @@ -192,8 +191,7 @@ where let tmp_one = (f)(tmp_one).await?; tmp_one.close().await.map_err(MagickError::CloseFile)?; - let process = Process::run("magick", &["convert", "-ping", input_file_str, "JSON:"]) - .map_err(MagickError::Process)?; + let process = Process::run("magick", &["convert", "-ping", input_file_str, "JSON:"])?; let mut output = Vec::new(); process diff --git a/src/exiftool.rs b/src/exiftool.rs index cf094f2..8b107b9 100644 --- a/src/exiftool.rs +++ b/src/exiftool.rs @@ -9,19 +9,30 @@ pub(crate) enum ExifError { #[error("Error reading process output")] Read(#[source] std::io::Error), + + #[error("Invalid media file provided")] + CommandFailed(ProcessError), +} + +impl From for ExifError { + fn from(value: ProcessError) -> Self { + match value { + e @ ProcessError::Status(_, _) => Self::CommandFailed(e), + otherwise => Self::Process(otherwise), + } + } } impl ExifError { pub(crate) fn is_client_error(&self) -> bool { // if exiftool bails we probably have bad input - matches!(self, Self::Process(ProcessError::Status(_))) + matches!(self, Self::CommandFailed(_)) } } #[tracing::instrument(level = "trace", skip(input))] pub(crate) async fn needs_reorienting(input: Bytes) -> Result { - let process = - Process::run("exiftool", &["-n", "-Orientation", "-"]).map_err(ExifError::Process)?; + let process = Process::run("exiftool", &["-n", "-Orientation", "-"])?; let mut reader = process.bytes_read(input); let mut buf = String::new(); @@ -35,8 +46,7 @@ pub(crate) async fn needs_reorienting(input: Bytes) -> Result { #[tracing::instrument(level = "trace", skip(input))] pub(crate) fn clear_metadata_bytes_read(input: Bytes) -> Result { - let process = - Process::run("exiftool", &["-all=", "-", "-out", "-"]).map_err(ExifError::Process)?; + let process = Process::run("exiftool", &["-all=", "-", "-out", "-"])?; Ok(process.bytes_read(input)) } diff --git a/src/ffmpeg.rs b/src/ffmpeg.rs index 90593de..ffa2067 100644 --- a/src/ffmpeg.rs +++ b/src/ffmpeg.rs @@ -46,14 +46,26 @@ pub(crate) enum FfMpegError { #[error("Error in store")] Store(#[source] StoreError), + #[error("Invalid media file provided")] + CommandFailed(ProcessError), + #[error("Invalid file path")] Path, } +impl From for FfMpegError { + fn from(value: ProcessError) -> Self { + match value { + e @ ProcessError::Status(_, _) => Self::CommandFailed(e), + otherwise => Self::Process(otherwise), + } + } +} + impl FfMpegError { pub(crate) fn is_client_error(&self) -> bool { // Failing validation or ffmpeg bailing probably means bad input - matches!(self, Self::Process(ProcessError::Status(_))) + matches!(self, Self::CommandFailed(_)) } pub(crate) fn is_not_found(&self) -> bool { @@ -143,10 +155,9 @@ pub(crate) async fn thumbnail( format.as_ffmpeg_format(), output_file_str, ], - ) - .map_err(FfMpegError::Process)?; + )?; - process.wait().await.map_err(FfMpegError::Process)?; + process.wait().await?; tokio::fs::remove_file(input_file) .await .map_err(FfMpegError::RemoveFile)?; diff --git a/src/magick.rs b/src/magick.rs index 8a432d6..1931b9a 100644 --- a/src/magick.rs +++ b/src/magick.rs @@ -37,6 +37,9 @@ pub(crate) enum MagickError { #[error("Error in metadata discovery")] Discover(#[source] crate::discover::DiscoverError), + #[error("Invalid media file provided")] + CommandFailed(ProcessError), + #[error("Command output is empty")] Empty, @@ -44,10 +47,19 @@ pub(crate) enum MagickError { Path, } +impl From for MagickError { + fn from(value: ProcessError) -> Self { + match value { + e @ ProcessError::Status(_, _) => Self::CommandFailed(e), + otherwise => Self::Process(otherwise), + } + } +} + impl MagickError { pub(crate) fn is_client_error(&self) -> bool { // Failing validation or imagemagick bailing probably means bad input - matches!(self, Self::Process(ProcessError::Status(_))) + matches!(self, Self::CommandFailed(_)) } } @@ -91,9 +103,7 @@ where } args.push(&output_arg); - let reader = Process::run("magick", &args) - .map_err(MagickError::Process)? - .read(); + let reader = Process::run("magick", &args)?.read(); let clean_reader = crate::tmp_file::cleanup_tmpfile(reader, input_file); diff --git a/src/process.rs b/src/process.rs index 2fbd08e..333d920 100644 --- a/src/process.rs +++ b/src/process.rs @@ -53,8 +53,8 @@ pub(crate) enum ProcessError { #[error("Reached process spawn limit")] LimitReached, - #[error("Failed with status {0}")] - Status(ExitStatus), + #[error("{0} Failed with {1}")] + Status(String, ExitStatus), #[error("Unknown process error")] Other(#[source] std::io::Error), @@ -98,7 +98,7 @@ impl Process { match res { Ok(status) if status.success() => Ok(()), - Ok(status) => Err(ProcessError::Status(status)), + Ok(status) => Err(ProcessError::Status(self.command, status)), Err(e) => Err(ProcessError::Other(e)), } } diff --git a/src/validate/exiftool.rs b/src/validate/exiftool.rs index 9fb4416..2c28b10 100644 --- a/src/validate/exiftool.rs +++ b/src/validate/exiftool.rs @@ -5,8 +5,7 @@ use crate::{exiftool::ExifError, process::Process}; #[tracing::instrument(level = "trace", skip(input))] pub(crate) fn clear_metadata_bytes_read(input: Bytes) -> Result { - let process = - Process::run("exiftool", &["-all=", "-", "-out", "-"]).map_err(ExifError::Process)?; + let process = Process::run("exiftool", &["-all=", "-", "-out", "-"])?; Ok(process.bytes_read(input)) } diff --git a/src/validate/ffmpeg.rs b/src/validate/ffmpeg.rs index bbb4c14..829cc85 100644 --- a/src/validate/ffmpeg.rs +++ b/src/validate/ffmpeg.rs @@ -74,11 +74,9 @@ async fn transcode_files( output_format.ffmpeg_format(), output_path, ], - ) - .map_err(FfMpegError::Process)? + )? .wait() - .await - .map_err(FfMpegError::Process)?; + .await?; } else { Process::run( "ffmpeg", @@ -101,11 +99,9 @@ async fn transcode_files( output_format.ffmpeg_format(), output_path, ], - ) - .map_err(FfMpegError::Process)? + )? .wait() - .await - .map_err(FfMpegError::Process)?; + .await?; } Ok(()) diff --git a/src/validate/magick.rs b/src/validate/magick.rs index 942ebee..64629e4 100644 --- a/src/validate/magick.rs +++ b/src/validate/magick.rs @@ -67,14 +67,12 @@ async fn convert( "-coalesce", &output_arg, ], - ) - .map_err(MagickError::Process)? + )? } else { Process::run( "magick", &["convert", "-strip", "-auto-orient", &input_arg, &output_arg], - ) - .map_err(MagickError::Process)? + )? }; let reader = process.read();