Better classify process related errors

src/error.rs

@@ -66,8 +66,14 @@ pub(crate) enum UploadError {
     #[error("Error in store")]
     Store(#[source] crate::store::StoreError),
 
-    #[error("Error parsing image details")]
-    ParseDetails(#[from] crate::magick::ParseDetailsError),
+    #[error("Error in ffmpeg")]
+    Ffmpeg(#[from] crate::ffmpeg::FfMpegError),
+
+    #[error("Error in imagemagick")]
+    Magick(#[from] crate::magick::MagickError),
+
+    #[error("Error in exiftool")]
+    Exiftool(#[from] crate::exiftool::ExifError),
 
     #[error("Provided process path is invalid")]
     ParsePath,
@@ -96,12 +102,6 @@ pub(crate) enum UploadError {
     #[error("Gif uploads are not enabled")]
     SilentVideoDisabled,
 
-    #[error("Invalid media dimensions")]
-    Dimensions,
-
-    #[error("Too many frames")]
-    Frames,
-
     #[error("Unable to download image, bad response {0}")]
     Download(actix_web::http::StatusCode),
 
@@ -111,9 +111,6 @@ pub(crate) enum UploadError {
     #[error("Unable to send request, {0}")]
     SendRequest(String),
 
-    #[error("Error converting Path to String")]
-    Path,
-
     #[error("Tried to save an image with an already-taken name")]
     DuplicateAlias,
 
@@ -175,7 +172,12 @@ impl ResponseError for Error {
                 | UploadError::UnsupportedProcessExtension
                 | UploadError::SilentVideoDisabled,
             ) => StatusCode::BAD_REQUEST,
+            Some(UploadError::Magick(e)) if e.is_client_error() => StatusCode::BAD_REQUEST,
+            Some(UploadError::Ffmpeg(e)) if e.is_client_error() => StatusCode::BAD_REQUEST,
+            Some(UploadError::Exiftool(e)) if e.is_client_error() => StatusCode::BAD_REQUEST,
             Some(UploadError::MissingAlias) => StatusCode::NOT_FOUND,
+            Some(UploadError::Magick(e)) if e.is_not_found() => StatusCode::NOT_FOUND,
+            Some(UploadError::Ffmpeg(e)) if e.is_not_found() => StatusCode::NOT_FOUND,
             Some(UploadError::InvalidToken) => StatusCode::FORBIDDEN,
             Some(UploadError::Range) => StatusCode::RANGE_NOT_SATISFIABLE,
             _ => StatusCode::INTERNAL_SERVER_ERROR,

src/exiftool.rs

@@ -1,21 +1,42 @@
-use crate::process::Process;
+use crate::process::{Process, ProcessError};
 use actix_web::web::Bytes;
 use tokio::io::{AsyncRead, AsyncReadExt};
 
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum ExifError {
+    #[error("Error in process")]
+    Process(#[source] ProcessError),
+
+    #[error("Error reading process output")]
+    Read(#[source] std::io::Error),
+}
+
+impl ExifError {
+    pub(crate) fn is_client_error(&self) -> bool {
+        // if exiftool bails we probably have bad input
+        matches!(self, Self::Process(ProcessError::Status(_)))
+    }
+}
+
 #[tracing::instrument(level = "trace", skip(input))]
-pub(crate) async fn needs_reorienting(input: Bytes) -> std::io::Result<bool> {
-    let process = Process::run("exiftool", &["-n", "-Orientation", "-"])?;
+pub(crate) async fn needs_reorienting(input: Bytes) -> Result<bool, ExifError> {
+    let process =
+        Process::run("exiftool", &["-n", "-Orientation", "-"]).map_err(ExifError::Process)?;
     let mut reader = process.bytes_read(input);
 
     let mut buf = String::new();
-    reader.read_to_string(&mut buf).await?;
+    reader
+        .read_to_string(&mut buf)
+        .await
+        .map_err(ExifError::Read)?;
 
     Ok(!buf.is_empty())
 }
 
 #[tracing::instrument(level = "trace", skip(input))]
-pub(crate) fn clear_metadata_bytes_read(input: Bytes) -> std::io::Result<impl AsyncRead + Unpin> {
-    let process = Process::run("exiftool", &["-all=", "-", "-out", "-"])?;
+pub(crate) fn clear_metadata_bytes_read(input: Bytes) -> Result<impl AsyncRead + Unpin, ExifError> {
+    let process =
+        Process::run("exiftool", &["-all=", "-", "-out", "-"]).map_err(ExifError::Process)?;
 
     Ok(process.bytes_read(input))
 }

src/ffmpeg.rs

@@ -3,9 +3,8 @@ mod tests;
 
 use crate::{
     config::{AudioCodec, ImageFormat, MediaConfiguration, VideoCodec},
-    error::{Error, UploadError},
-    magick::{Details, ParseDetailsError, ValidInputType},
-    process::Process,
+    magick::{Details, ParseDetailsError, ValidInputType, ValidateDetailsError},
+    process::{Process, ProcessError},
     store::{Store, StoreError},
 };
 use actix_web::web::Bytes;
@@ -28,6 +27,108 @@ enum TranscodeOutputOptions {
     },
 }
 
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum VideoFormat {
+    Gif,
+    Mp4,
+    Webm,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum OutputFormat {
+    Mp4,
+    Webm,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum ThumbnailFormat {
+    Jpeg,
+    // Webp,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum FileFormat {
+    Image(ImageFormat),
+    Video(VideoFormat),
+}
+
+#[derive(serde::Deserialize)]
+struct PixelFormatOutput {
+    pixel_formats: Vec<PixelFormat>,
+}
+
+#[derive(serde::Deserialize)]
+struct PixelFormat {
+    name: String,
+    flags: Flags,
+}
+
+#[derive(serde::Deserialize)]
+struct Flags {
+    alpha: usize,
+}
+
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum FfMpegError {
+    #[error("Error in ffmpeg process")]
+    Process(#[source] ProcessError),
+
+    #[error("Error reading output")]
+    Read(#[source] std::io::Error),
+
+    #[error("Error writing bytes")]
+    Write(#[source] std::io::Error),
+
+    #[error("Invalid output format")]
+    Json(#[source] serde_json::Error),
+
+    #[error("Error creating parent directory")]
+    CreateDir(#[source] crate::store::file_store::FileError),
+
+    #[error("Error reading file to stream")]
+    ReadFile(#[source] crate::store::file_store::FileError),
+
+    #[error("Error opening file")]
+    OpenFile(#[source] std::io::Error),
+
+    #[error("Error creating file")]
+    CreateFile(#[source] std::io::Error),
+
+    #[error("Error closing file")]
+    CloseFile(#[source] std::io::Error),
+
+    #[error("Error removing file")]
+    RemoveFile(#[source] std::io::Error),
+
+    #[error("Error parsing details")]
+    Details(#[source] ParseDetailsError),
+
+    #[error("Media details are invalid")]
+    ValidateDetails(#[source] ValidateDetailsError),
+
+    #[error("Error in store")]
+    Store(#[source] StoreError),
+
+    #[error("Invalid file path")]
+    Path,
+}
+
+impl FfMpegError {
+    pub(crate) fn is_client_error(&self) -> bool {
+        // Failing validation or ffmpeg bailing probably means bad input
+        matches!(self, Self::ValidateDetails(_))
+            || matches!(self, Self::Process(ProcessError::Status(_)))
+    }
+
+    pub(crate) fn is_not_found(&self) -> bool {
+        if let Self::Store(e) = self {
+            return e.is_not_found();
+        }
+
+        false
+    }
+}
+
 impl TranscodeOptions {
     pub(crate) fn new(
         media: &MediaConfiguration,
@@ -98,7 +199,7 @@ impl TranscodeOptions {
         input_path: &str,
         output_path: &str,
         alpha: bool,
-    ) -> Result<Process, std::io::Error> {
+    ) -> Result<Process, ProcessError> {
         match self.output {
             TranscodeOutputOptions::Gif => Process::run("ffmpeg", &[
                 "-hide_banner",
@@ -194,31 +295,6 @@ impl TranscodeOutputOptions {
     }
 }
 
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum VideoFormat {
-    Gif,
-    Mp4,
-    Webm,
-}
-
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum OutputFormat {
-    Mp4,
-    Webm,
-}
-
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum ThumbnailFormat {
-    Jpeg,
-    // Webp,
-}
-
-#[derive(Clone, Copy, Debug)]
-pub(crate) enum FileFormat {
-    Image(ImageFormat),
-    Video(VideoFormat),
-}
-
 impl ValidInputType {
     pub(crate) fn to_file_format(self) -> FileFormat {
         match self {
@@ -342,9 +418,12 @@ const FORMAT_MAPPINGS: &[(&str, VideoFormat)] = &[
 
 pub(crate) async fn input_type_bytes(
     input: Bytes,
-) -> Result<Option<(Details, ValidInputType)>, Error> {
+) -> Result<Option<(Details, ValidInputType)>, FfMpegError> {
     if let Some(details) = details_bytes(input).await? {
-        let input_type = details.validate_input()?;
+        let input_type = details
+            .validate_input()
+            .map_err(FfMpegError::ValidateDetails)?;
+
         return Ok(Some((details, input_type)));
     }
 
@@ -354,40 +433,33 @@ pub(crate) async fn input_type_bytes(
 pub(crate) async fn details_store<S: Store>(
     store: &S,
     identifier: &S::Identifier,
-) -> Result<Option<Details>, Error> {
+) -> Result<Option<Details>, FfMpegError> {
     details_file(move |mut tmp_one| async move {
-        let stream = store.to_stream(identifier, None, None).await?;
-        tmp_one.write_from_stream(stream).await?;
+        let stream = store
+            .to_stream(identifier, None, None)
+            .await
+            .map_err(FfMpegError::Store)?;
+        tmp_one
+            .write_from_stream(stream)
+            .await
+            .map_err(FfMpegError::Write)?;
         Ok(tmp_one)
     })
     .await
 }
 
-pub(crate) async fn details_bytes(input: Bytes) -> Result<Option<Details>, Error> {
+pub(crate) async fn details_bytes(input: Bytes) -> Result<Option<Details>, FfMpegError> {
     details_file(move |mut tmp_one| async move {
-        tmp_one.write_from_bytes(input).await?;
+        tmp_one
+            .write_from_bytes(input)
+            .await
+            .map_err(FfMpegError::Write)?;
         Ok(tmp_one)
     })
     .await
 }
 
-#[derive(serde::Deserialize)]
-struct PixelFormatOutput {
-    pixel_formats: Vec<PixelFormat>,
-}
-
-#[derive(serde::Deserialize)]
-struct PixelFormat {
-    name: String,
-    flags: Flags,
-}
-
-#[derive(serde::Deserialize)]
-struct Flags {
-    alpha: usize,
-}
-
-async fn alpha_pixel_formats() -> Result<HashSet<String>, Error> {
+async fn alpha_pixel_formats() -> Result<HashSet<String>, FfMpegError> {
     let process = Process::run(
         "ffprobe",
         &[
@@ -400,12 +472,17 @@ async fn alpha_pixel_formats() -> Result<HashSet<String>, Error> {
             "-print_format",
             "json",
         ],
-    )?;
+    )
+    .map_err(FfMpegError::Process)?;
 
     let mut output = Vec::new();
-    process.read().read_to_end(&mut output).await?;
+    process
+        .read()
+        .read_to_end(&mut output)
+        .await
+        .map_err(FfMpegError::Read)?;
 
-    let formats: PixelFormatOutput = serde_json::from_slice(&output)?;
+    let formats: PixelFormatOutput = serde_json::from_slice(&output).map_err(FfMpegError::Json)?;
 
     Ok(parse_pixel_formats(formats))
 }
@@ -443,20 +520,22 @@ struct Format {
 }
 
 #[tracing::instrument(skip(f))]
-async fn details_file<F, Fut>(f: F) -> Result<Option<Details>, Error>
+async fn details_file<F, Fut>(f: F) -> Result<Option<Details>, FfMpegError>
 where
     F: FnOnce(crate::file::File) -> Fut,
-    Fut: std::future::Future<Output = Result<crate::file::File, Error>>,
+    Fut: std::future::Future<Output = Result<crate::file::File, FfMpegError>>,
 {
     let input_file = crate::tmp_file::tmp_file(None);
-    let input_file_str = input_file.to_str().ok_or(UploadError::Path)?;
+    let input_file_str = input_file.to_str().ok_or(FfMpegError::Path)?;
     crate::store::file_store::safe_create_parent(&input_file)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::CreateDir)?;
 
-    let tmp_one = crate::file::File::create(&input_file).await?;
+    let tmp_one = crate::file::File::create(&input_file)
+        .await
+        .map_err(FfMpegError::CreateFile)?;
     let tmp_one = (f)(tmp_one).await?;
-    tmp_one.close().await?;
+    tmp_one.close().await.map_err(FfMpegError::CloseFile)?;
 
     let process = Process::run(
         "ffprobe",
@@ -474,18 +553,25 @@ where
             "json",
             input_file_str,
         ],
-    )?;
+    )
+    .map_err(FfMpegError::Process)?;
 
     let mut output = Vec::new();
-    process.read().read_to_end(&mut output).await?;
-    tokio::fs::remove_file(input_file_str).await?;
+    process
+        .read()
+        .read_to_end(&mut output)
+        .await
+        .map_err(FfMpegError::Read)?;
+    tokio::fs::remove_file(input_file_str)
+        .await
+        .map_err(FfMpegError::RemoveFile)?;
 
-    let output: DetailsOutput = serde_json::from_slice(&output)?;
+    let output: DetailsOutput = serde_json::from_slice(&output).map_err(FfMpegError::Json)?;
 
     parse_details(output)
 }
 
-fn parse_details(output: DetailsOutput) -> Result<Option<Details>, Error> {
+fn parse_details(output: DetailsOutput) -> Result<Option<Details>, FfMpegError> {
     tracing::debug!("OUTPUT: {:?}", output);
 
     let [stream] = output.streams;
@@ -499,7 +585,7 @@ fn parse_details(output: DetailsOutput) -> Result<Option<Details>, Error> {
                 stream.nb_read_frames.as_deref(),
                 *v,
             )
-            .map_err(Error::from);
+            .map_err(FfMpegError::Details);
         }
     }
 
@@ -534,7 +620,7 @@ fn parse_details_inner(
     }))
 }
 
-async fn pixel_format(input_file: &str) -> Result<String, Error> {
+async fn pixel_format(input_file: &str) -> Result<String, FfMpegError> {
     let process = Process::run(
         "ffprobe",
         &[
@@ -548,10 +634,15 @@ async fn pixel_format(input_file: &str) -> Result<String, Error> {
             "compact=p=0:nk=1",
             input_file,
         ],
-    )?;
+    )
+    .map_err(FfMpegError::Process)?;
 
     let mut output = Vec::new();
-    process.read().read_to_end(&mut output).await?;
+    process
+        .read()
+        .read_to_end(&mut output)
+        .await
+        .map_err(FfMpegError::Read)?;
     Ok(String::from_utf8_lossy(&output).trim().to_string())
 }
 
@@ -559,22 +650,27 @@ async fn pixel_format(input_file: &str) -> Result<String, Error> {
 pub(crate) async fn transcode_bytes(
     input: Bytes,
     transcode_options: TranscodeOptions,
-) -> Result<impl AsyncRead + Unpin, Error> {
+) -> Result<impl AsyncRead + Unpin, FfMpegError> {
     let input_file = crate::tmp_file::tmp_file(Some(transcode_options.input_file_extension()));
-    let input_file_str = input_file.to_str().ok_or(UploadError::Path)?;
+    let input_file_str = input_file.to_str().ok_or(FfMpegError::Path)?;
     crate::store::file_store::safe_create_parent(&input_file)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::CreateDir)?;
 
     let output_file = crate::tmp_file::tmp_file(Some(transcode_options.output_file_extension()));
-    let output_file_str = output_file.to_str().ok_or(UploadError::Path)?;
+    let output_file_str = output_file.to_str().ok_or(FfMpegError::Path)?;
     crate::store::file_store::safe_create_parent(&output_file)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::CreateDir)?;
 
-    let mut tmp_one = crate::file::File::create(&input_file).await?;
-    tmp_one.write_from_bytes(input).await?;
-    tmp_one.close().await?;
+    let mut tmp_one = crate::file::File::create(&input_file)
+        .await
+        .map_err(FfMpegError::CreateFile)?;
+    tmp_one
+        .write_from_bytes(input)
+        .await
+        .map_err(FfMpegError::Write)?;
+    tmp_one.close().await.map_err(FfMpegError::CloseFile)?;
 
     let alpha = if transcode_options.supports_alpha() {
         static ALPHA_PIXEL_FORMATS: OnceCell<HashSet<String>> = OnceCell::new();
@@ -594,16 +690,22 @@ pub(crate) async fn transcode_bytes(
         false
     };
 
-    let process = transcode_options.execute(input_file_str, output_file_str, alpha)?;
+    let process = transcode_options
+        .execute(input_file_str, output_file_str, alpha)
+        .map_err(FfMpegError::Process)?;
 
-    process.wait().await?;
-    tokio::fs::remove_file(input_file).await?;
+    process.wait().await.map_err(FfMpegError::Process)?;
+    tokio::fs::remove_file(input_file)
+        .await
+        .map_err(FfMpegError::RemoveFile)?;
 
-    let tmp_two = crate::file::File::open(&output_file).await?;
+    let tmp_two = crate::file::File::open(&output_file)
+        .await
+        .map_err(FfMpegError::OpenFile)?;
     let stream = tmp_two
         .read_to_stream(None, None)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::ReadFile)?;
     let reader = tokio_util::io::StreamReader::new(stream);
     let clean_reader = crate::tmp_file::cleanup_tmpfile(reader, output_file);
 
@@ -616,24 +718,31 @@ pub(crate) async fn thumbnail<S: Store>(
     from: S::Identifier,
     input_format: VideoFormat,
     format: ThumbnailFormat,
-) -> Result<impl AsyncRead + Unpin, Error> {
+) -> Result<impl AsyncRead + Unpin, FfMpegError> {
     let input_file = crate::tmp_file::tmp_file(Some(input_format.to_file_extension()));
-    let input_file_str = input_file.to_str().ok_or(UploadError::Path)?;
+    let input_file_str = input_file.to_str().ok_or(FfMpegError::Path)?;
     crate::store::file_store::safe_create_parent(&input_file)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::CreateDir)?;
 
     let output_file = crate::tmp_file::tmp_file(Some(format.to_file_extension()));
-    let output_file_str = output_file.to_str().ok_or(UploadError::Path)?;
+    let output_file_str = output_file.to_str().ok_or(FfMpegError::Path)?;
     crate::store::file_store::safe_create_parent(&output_file)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::CreateDir)?;
 
-    let mut tmp_one = crate::file::File::create(&input_file).await?;
+    let mut tmp_one = crate::file::File::create(&input_file)
+        .await
+        .map_err(FfMpegError::CreateFile)?;
+    let stream = store
+        .to_stream(&from, None, None)
+        .await
+        .map_err(FfMpegError::Store)?;
     tmp_one
-        .write_from_stream(store.to_stream(&from, None, None).await?)
-        .await?;
-    tmp_one.close().await?;
+        .write_from_stream(stream)
+        .await
+        .map_err(FfMpegError::Write)?;
+    tmp_one.close().await.map_err(FfMpegError::CloseFile)?;
 
     let process = Process::run(
         "ffmpeg",
@@ -651,16 +760,21 @@ pub(crate) async fn thumbnail<S: Store>(
             format.as_ffmpeg_format(),
             output_file_str,
         ],
-    )?;
+    )
+    .map_err(FfMpegError::Process)?;
 
-    process.wait().await?;
-    tokio::fs::remove_file(input_file).await?;
+    process.wait().await.map_err(FfMpegError::Process)?;
+    tokio::fs::remove_file(input_file)
+        .await
+        .map_err(FfMpegError::RemoveFile)?;
 
-    let tmp_two = crate::file::File::open(&output_file).await?;
+    let tmp_two = crate::file::File::open(&output_file)
+        .await
+        .map_err(FfMpegError::OpenFile)?;
     let stream = tmp_two
         .read_to_stream(None, None)
         .await
-        .map_err(StoreError::from)?;
+        .map_err(FfMpegError::ReadFile)?;
     let reader = tokio_util::io::StreamReader::new(stream);
     let clean_reader = crate::tmp_file::cleanup_tmpfile(reader, output_file);
 

src/magick.rs

@@ -3,16 +3,67 @@ mod tests;
 
 use crate::{
     config::{ImageFormat, VideoCodec},
-    error::{Error, UploadError},
-    process::Process,
+    process::{Process, ProcessError},
     repo::Alias,
-    store::{Store, StoreError},
+    store::Store,
 };
 use actix_web::web::Bytes;
-use tokio::{
-    io::{AsyncRead, AsyncReadExt},
-    process::Command,
-};
+use tokio::io::{AsyncRead, AsyncReadExt};
+
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum MagickError {
+    #[error("Error in imagemagick process")]
+    Process(#[source] ProcessError),
+
+    #[error("Error parsing details")]
+    ParseDetails(#[source] ParseDetailsError),
+
+    #[error("Media details are invalid")]
+    ValidateDetails(#[source] ValidateDetailsError),
+
+    #[error("Invalid output format")]
+    Json(#[source] serde_json::Error),
+
+    #[error("Error reading bytes")]
+    Read(#[source] std::io::Error),
+
+    #[error("Error writing bytes")]
+    Write(#[source] std::io::Error),
+
+    #[error("Error creating file")]
+    CreateFile(#[source] std::io::Error),
+
+    #[error("Error creating directory")]
+    CreateDir(#[source] crate::store::file_store::FileError),
+
+    #[error("Error reading file")]
+    Store(#[source] crate::store::StoreError),
+
+    #[error("Error closing file")]
+    CloseFile(#[source] std::io::Error),
+
+    #[error("Error removing file")]
+    RemoveFile(#[source] std::io::Error),
+
+    #[error("Invalid file path")]
+    Path,
+}
+
+impl MagickError {
+    pub(crate) fn is_client_error(&self) -> bool {
+        // Failing validation or imagemagick bailing probably means bad input
+        matches!(self, Self::ValidateDetails(_))
+            || matches!(self, Self::Process(ProcessError::Status(_)))
+    }
+
+    pub(crate) fn is_not_found(&self) -> bool {
+        if let Self::Store(e) = self {
+            return e.is_not_found();
+        }
+
+        false
+    }
+}
 
 pub(crate) fn details_hint(alias: &Alias) -> Option<ValidInputType> {
     let ext = alias.extension()?;
@@ -138,7 +189,7 @@ pub(crate) struct Details {
 pub(crate) fn convert_bytes_read(
     input: Bytes,
     format: ImageFormat,
-) -> std::io::Result<impl AsyncRead + Unpin> {
+) -> Result<impl AsyncRead + Unpin, MagickError> {
     let process = Process::run(
         "magick",
         &[
@@ -148,7 +199,8 @@ pub(crate) fn convert_bytes_read(
             "-strip",
             format!("{}:-", format.as_magick_format()).as_str(),
         ],
-    )?;
+    )
+    .map_err(MagickError::Process)?;
 
     Ok(process.bytes_read(input))
 }
@@ -157,17 +209,22 @@ pub(crate) fn convert_bytes_read(
 pub(crate) async fn details_bytes(
     input: Bytes,
     hint: Option<ValidInputType>,
-) -> Result<Details, Error> {
+) -> Result<Details, MagickError> {
     if let Some(hint) = hint.and_then(|hint| hint.video_hint()) {
         let input_file = crate::tmp_file::tmp_file(Some(hint));
-        let input_file_str = input_file.to_str().ok_or(UploadError::Path)?;
+        let input_file_str = input_file.to_str().ok_or(MagickError::Path)?;
         crate::store::file_store::safe_create_parent(&input_file)
             .await
-            .map_err(StoreError::from)?;
+            .map_err(MagickError::CreateDir)?;
 
-        let mut tmp_one = crate::file::File::create(&input_file).await?;
-        tmp_one.write_from_bytes(input).await?;
-        tmp_one.close().await?;
+        let mut tmp_one = crate::file::File::create(&input_file)
+            .await
+            .map_err(MagickError::CreateFile)?;
+        tmp_one
+            .write_from_bytes(input)
+            .await
+            .map_err(MagickError::Write)?;
+        tmp_one.close().await.map_err(MagickError::CloseFile)?;
 
         return details_file(input_file_str).await;
     }
@@ -178,16 +235,21 @@ pub(crate) async fn details_bytes(
         "-".to_owned()
     };
 
-    let process = Process::run("magick", &["convert", "-ping", &last_arg, "JSON:"])?;
+    let process = Process::run("magick", &["convert", "-ping", &last_arg, "JSON:"])
+        .map_err(MagickError::Process)?;
 
     let mut reader = process.bytes_read(input);
 
     let mut bytes = Vec::new();
-    reader.read_to_end(&mut bytes).await?;
+    reader
+        .read_to_end(&mut bytes)
+        .await
+        .map_err(MagickError::Read)?;
 
-    let details_output: Vec<DetailsOutput> = serde_json::from_slice(&bytes)?;
+    let details_output: Vec<DetailsOutput> =
+        serde_json::from_slice(&bytes).map_err(MagickError::Json)?;
 
-    parse_details(details_output)
+    parse_details(details_output).map_err(MagickError::ParseDetails)
 }
 
 #[derive(Debug, serde::Deserialize)]
@@ -212,19 +274,26 @@ pub(crate) async fn details_store<S: Store + 'static>(
     store: S,
     identifier: S::Identifier,
     hint: Option<ValidInputType>,
-) -> Result<Details, Error> {
+) -> Result<Details, MagickError> {
     if let Some(hint) = hint.and_then(|hint| hint.video_hint()) {
         let input_file = crate::tmp_file::tmp_file(Some(hint));
-        let input_file_str = input_file.to_str().ok_or(UploadError::Path)?;
+        let input_file_str = input_file.to_str().ok_or(MagickError::Path)?;
         crate::store::file_store::safe_create_parent(&input_file)
             .await
-            .map_err(StoreError::from)?;
+            .map_err(MagickError::CreateDir)?;
 
-        let mut tmp_one = crate::file::File::create(&input_file).await?;
+        let mut tmp_one = crate::file::File::create(&input_file)
+            .await
+            .map_err(MagickError::CreateFile)?;
+        let stream = store
+            .to_stream(&identifier, None, None)
+            .await
+            .map_err(MagickError::Store)?;
         tmp_one
-            .write_from_stream(store.to_stream(&identifier, None, None).await?)
-            .await?;
-        tmp_one.close().await?;
+            .write_from_stream(stream)
+            .await
+            .map_err(MagickError::Write)?;
+        tmp_one.close().await.map_err(MagickError::CloseFile)?;
 
         return details_file(input_file_str).await;
     }
@@ -235,31 +304,43 @@ pub(crate) async fn details_store<S: Store + 'static>(
         "-".to_owned()
     };
 
-    let process = Process::run("magick", &["convert", "-ping", &last_arg, "JSON:"])?;
+    let process = Process::run("magick", &["convert", "-ping", &last_arg, "JSON:"])
+        .map_err(MagickError::Process)?;
 
     let mut reader = process.store_read(store, identifier);
 
     let mut output = Vec::new();
-    reader.read_to_end(&mut output).await?;
+    reader
+        .read_to_end(&mut output)
+        .await
+        .map_err(MagickError::Read)?;
 
-    let details_output: Vec<DetailsOutput> = serde_json::from_slice(&output)?;
+    let details_output: Vec<DetailsOutput> =
+        serde_json::from_slice(&output).map_err(MagickError::Json)?;
 
-    parse_details(details_output)
+    parse_details(details_output).map_err(MagickError::ParseDetails)
 }
 
 #[tracing::instrument]
-pub(crate) async fn details_file(path_str: &str) -> Result<Details, Error> {
-    let process = Process::run("magick", &["convert", "-ping", path_str, "JSON:"])?;
+pub(crate) async fn details_file(path_str: &str) -> Result<Details, MagickError> {
+    let process = Process::run("magick", &["convert", "-ping", path_str, "JSON:"])
+        .map_err(MagickError::Process)?;
 
     let mut reader = process.read();
 
     let mut output = Vec::new();
-    reader.read_to_end(&mut output).await?;
-    tokio::fs::remove_file(path_str).await?;
+    reader
+        .read_to_end(&mut output)
+        .await
+        .map_err(MagickError::Read)?;
+    tokio::fs::remove_file(path_str)
+        .await
+        .map_err(MagickError::RemoveFile)?;
 
-    let details_output: Vec<DetailsOutput> = serde_json::from_slice(&output)?;
+    let details_output: Vec<DetailsOutput> =
+        serde_json::from_slice(&output).map_err(MagickError::Json)?;
 
-    parse_details(details_output)
+    parse_details(details_output).map_err(MagickError::ParseDetails)
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -277,11 +358,11 @@ pub(crate) enum ParseDetailsError {
     ParseFrames(String),
 }
 
-fn parse_details(details_output: Vec<DetailsOutput>) -> Result<Details, Error> {
+fn parse_details(details_output: Vec<DetailsOutput>) -> Result<Details, ParseDetailsError> {
     let frames = details_output.len();
 
     if frames == 0 {
-        return Err(ParseDetailsError::NoFrames.into());
+        return Err(ParseDetailsError::NoFrames);
     }
 
     let width = details_output
@@ -302,7 +383,7 @@ fn parse_details(details_output: Vec<DetailsOutput>) -> Result<Details, Error> {
         .iter()
         .all(|details| details.image.format == format)
     {
-        return Err(ParseDetailsError::MixedFormats.into());
+        return Err(ParseDetailsError::MixedFormats);
     }
 
     let mime_type = match format {
@@ -314,7 +395,7 @@ fn parse_details(details_output: Vec<DetailsOutput>) -> Result<Details, Error> {
         "JXL" => image_jxl(),
         "PNG" => mime::IMAGE_PNG,
         "WEBP" => image_webp(),
-        e => return Err(ParseDetailsError::Unsupported(String::from(e)).into()),
+        e => return Err(ParseDetailsError::Unsupported(String::from(e))),
     };
 
     Ok(Details {
@@ -325,23 +406,27 @@ fn parse_details(details_output: Vec<DetailsOutput>) -> Result<Details, Error> {
     })
 }
 
-pub(crate) async fn input_type_bytes(input: Bytes) -> Result<(Details, ValidInputType), Error> {
+pub(crate) async fn input_type_bytes(
+    input: Bytes,
+) -> Result<(Details, ValidInputType), MagickError> {
     let details = details_bytes(input, None).await?;
-    let input_type = details.validate_input()?;
+    let input_type = details
+        .validate_input()
+        .map_err(MagickError::ValidateDetails)?;
     Ok((details, input_type))
 }
 
-fn process_image(args: Vec<String>, format: ImageFormat) -> std::io::Result<Process> {
+fn process_image(process_args: Vec<String>, format: ImageFormat) -> Result<Process, ProcessError> {
     let command = "magick";
     let convert_args = ["convert", "-"];
     let last_arg = format!("{}:-", format.as_magick_format());
 
-    Process::spawn(
-        Command::new(command)
-            .args(convert_args)
-            .args(args)
-            .arg(last_arg),
-    )
+    let mut args = Vec::with_capacity(process_args.len() + 3);
+    args.extend_from_slice(&convert_args[..]);
+    args.extend(process_args.iter().map(|s| s.as_str()));
+    args.push(&last_arg);
+
+    Process::run(command, &args)
 }
 
 pub(crate) fn process_image_store_read<S: Store + 'static>(
@@ -349,31 +434,47 @@ pub(crate) fn process_image_store_read<S: Store + 'static>(
     identifier: S::Identifier,
     args: Vec<String>,
     format: ImageFormat,
-) -> std::io::Result<impl AsyncRead + Unpin> {
-    Ok(process_image(args, format)?.store_read(store, identifier))
+) -> Result<impl AsyncRead + Unpin, MagickError> {
+    Ok(process_image(args, format)
+        .map_err(MagickError::Process)?
+        .store_read(store, identifier))
 }
 
 pub(crate) fn process_image_async_read<A: AsyncRead + Unpin + 'static>(
     async_read: A,
     args: Vec<String>,
     format: ImageFormat,
-) -> std::io::Result<impl AsyncRead + Unpin> {
-    Ok(process_image(args, format)?.pipe_async_read(async_read))
+) -> Result<impl AsyncRead + Unpin, MagickError> {
+    Ok(process_image(args, format)
+        .map_err(MagickError::Process)?
+        .pipe_async_read(async_read))
+}
+
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum ValidateDetailsError {
+    #[error("Exceeded maximum dimensions")]
+    ExceededDimensions,
+
+    #[error("Exceeded maximum frame count")]
+    TooManyFrames,
+
+    #[error("Unsupported media type: {0}")]
+    UnsupportedMediaType(String),
 }
 
 impl Details {
     #[tracing::instrument(level = "debug", name = "Validating input type")]
-    pub(crate) fn validate_input(&self) -> Result<ValidInputType, Error> {
+    pub(crate) fn validate_input(&self) -> Result<ValidInputType, ValidateDetailsError> {
         if self.width > crate::CONFIG.media.max_width
             || self.height > crate::CONFIG.media.max_height
             || self.width * self.height > crate::CONFIG.media.max_area
         {
-            return Err(UploadError::Dimensions.into());
+            return Err(ValidateDetailsError::ExceededDimensions);
         }
 
         if let Some(frames) = self.frames {
             if frames > crate::CONFIG.media.max_frame_count {
-                return Err(UploadError::Frames.into());
+                return Err(ValidateDetailsError::TooManyFrames);
             }
         }
 
@@ -386,7 +487,11 @@ impl Details {
             (mime::IMAGE, subtype) if subtype.as_str() == "jxl" => ValidInputType::Jxl,
             (mime::IMAGE, mime::PNG) => ValidInputType::Png,
             (mime::IMAGE, subtype) if subtype.as_str() == "webp" => ValidInputType::Webp,
-            _ => return Err(ParseDetailsError::Unsupported(self.mime_type.to_string()).into()),
+            _ => {
+                return Err(ValidateDetailsError::UnsupportedMediaType(
+                    self.mime_type.to_string(),
+                ))
+            }
         };
 
         Ok(input_type)

src/process.rs

@@ -4,7 +4,7 @@ use actix_web::web::Bytes;
 use std::{
     future::Future,
     pin::Pin,
-    process::Stdio,
+    process::{ExitStatus, Stdio},
     task::{Context, Poll},
 };
 use tokio::{
@@ -41,27 +41,56 @@ pin_project_lite::pin_project! {
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum ProcessError {
+    #[error("Required commend {0} not found")]
+    NotFound(String),
+
+    #[error("Reached process spawn limit")]
+    LimitReached,
+
+    #[error("Failed with status {0}")]
+    Status(ExitStatus),
+
+    #[error("Unknown process error")]
+    Other(#[source] std::io::Error),
+}
+
 impl Process {
-    pub(crate) fn run(command: &str, args: &[&str]) -> std::io::Result<Self> {
-        tracing::trace_span!(parent: None, "Create command")
-            .in_scope(|| Self::spawn(Command::new(command).args(args)))
+    pub(crate) fn run(command: &str, args: &[&str]) -> Result<Self, ProcessError> {
+        let res = tracing::trace_span!(parent: None, "Create command")
+            .in_scope(|| Self::spawn(Command::new(command).args(args)));
+
+        match res {
+            Ok(this) => Ok(this),
+            Err(e) => match e.kind() {
+                std::io::ErrorKind::NotFound => Err(ProcessError::NotFound(command.to_string())),
+                std::io::ErrorKind::WouldBlock => Err(ProcessError::LimitReached),
+                _ => Err(ProcessError::Other(e)),
+            },
+        }
     }
 
-    pub(crate) fn spawn(cmd: &mut Command) -> std::io::Result<Self> {
+    fn spawn(cmd: &mut Command) -> std::io::Result<Self> {
         tracing::trace_span!(parent: None, "Spawn command").in_scope(|| {
-            let cmd = cmd.stdin(Stdio::piped()).stdout(Stdio::piped());
+            let cmd = cmd
+                .stdin(Stdio::piped())
+                .stdout(Stdio::piped())
+                .kill_on_drop(true);
 
             cmd.spawn().map(|child| Process { child })
         })
     }
 
     #[tracing::instrument(skip(self))]
-    pub(crate) async fn wait(mut self) -> std::io::Result<()> {
-        let status = self.child.wait().await?;
-        if !status.success() {
-            return Err(std::io::Error::new(std::io::ErrorKind::Other, &StatusError));
+    pub(crate) async fn wait(mut self) -> Result<(), ProcessError> {
+        let res = self.child.wait().await;
+
+        match res {
+            Ok(status) if status.success() => Ok(()),
+            Ok(status) => Err(ProcessError::Status(status)),
+            Err(e) => Err(ProcessError::Other(e)),
         }
-        Ok(())
     }
 
     pub(crate) fn bytes_read(self, input: Bytes) -> impl AsyncRead + Unpin {