From 9004ecaadf688a53be0062d65170faaccbef9a59 Mon Sep 17 00:00:00 2001 From: asonix Date: Mon, 10 Jul 2023 15:29:41 -0500 Subject: [PATCH] Better classify process related errors --- src/error.rs | 24 ++-- src/exiftool.rs | 33 +++++- src/ffmpeg.rs | 310 +++++++++++++++++++++++++++++++++--------------- src/magick.rs | 219 +++++++++++++++++++++++++--------- src/process.rs | 51 ++++++-- 5 files changed, 454 insertions(+), 183 deletions(-) diff --git a/src/error.rs b/src/error.rs index 273aad5..7eb50d7 100644 --- a/src/error.rs +++ b/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, diff --git a/src/exiftool.rs b/src/exiftool.rs index 2d5ca9d..cf094f2 100644 --- a/src/exiftool.rs +++ b/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 { - let process = Process::run("exiftool", &["-n", "-Orientation", "-"])?; +pub(crate) async fn needs_reorienting(input: Bytes) -> Result { + 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 { - let process = Process::run("exiftool", &["-all=", "-", "-out", "-"])?; +pub(crate) fn clear_metadata_bytes_read(input: Bytes) -> Result { + let process = + Process::run("exiftool", &["-all=", "-", "-out", "-"]).map_err(ExifError::Process)?; Ok(process.bytes_read(input)) } diff --git a/src/ffmpeg.rs b/src/ffmpeg.rs index a878220..71eca76 100644 --- a/src/ffmpeg.rs +++ b/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, +} + +#[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 { + ) -> Result { 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, Error> { +) -> Result, 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( store: &S, identifier: &S::Identifier, -) -> Result, Error> { +) -> Result, 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, Error> { +pub(crate) async fn details_bytes(input: Bytes) -> Result, 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, -} - -#[derive(serde::Deserialize)] -struct PixelFormat { - name: String, - flags: Flags, -} - -#[derive(serde::Deserialize)] -struct Flags { - alpha: usize, -} - -async fn alpha_pixel_formats() -> Result, Error> { +async fn alpha_pixel_formats() -> Result, FfMpegError> { let process = Process::run( "ffprobe", &[ @@ -400,12 +472,17 @@ async fn alpha_pixel_formats() -> Result, 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: F) -> Result, Error> +async fn details_file(f: F) -> Result, FfMpegError> where F: FnOnce(crate::file::File) -> Fut, - Fut: std::future::Future>, + Fut: std::future::Future>, { 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, Error> { +fn parse_details(output: DetailsOutput) -> Result, FfMpegError> { tracing::debug!("OUTPUT: {:?}", output); let [stream] = output.streams; @@ -499,7 +585,7 @@ fn parse_details(output: DetailsOutput) -> Result, 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 { +async fn pixel_format(input_file: &str) -> Result { let process = Process::run( "ffprobe", &[ @@ -548,10 +634,15 @@ async fn pixel_format(input_file: &str) -> Result { "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 { pub(crate) async fn transcode_bytes( input: Bytes, transcode_options: TranscodeOptions, -) -> Result { +) -> Result { 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> = 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( from: S::Identifier, input_format: VideoFormat, format: ThumbnailFormat, -) -> Result { +) -> Result { 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( 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); diff --git a/src/magick.rs b/src/magick.rs index 8485bff..5e3eea5 100644 --- a/src/magick.rs +++ b/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 { 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 { +) -> Result { 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, -) -> Result { +) -> Result { 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 = serde_json::from_slice(&bytes)?; + let details_output: Vec = + 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( store: S, identifier: S::Identifier, hint: Option, -) -> Result { +) -> Result { 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( "-".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 = serde_json::from_slice(&output)?; + let details_output: Vec = + 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 { - let process = Process::run("magick", &["convert", "-ping", path_str, "JSON:"])?; +pub(crate) async fn details_file(path_str: &str) -> Result { + 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 = serde_json::from_slice(&output)?; + let details_output: Vec = + 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) -> Result { +fn parse_details(details_output: Vec) -> Result { 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) -> Result { .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) -> Result { "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) -> Result { }) } -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, format: ImageFormat) -> std::io::Result { +fn process_image(process_args: Vec, format: ImageFormat) -> Result { 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( @@ -349,31 +434,47 @@ pub(crate) fn process_image_store_read( identifier: S::Identifier, args: Vec, format: ImageFormat, -) -> std::io::Result { - Ok(process_image(args, format)?.store_read(store, identifier)) +) -> Result { + Ok(process_image(args, format) + .map_err(MagickError::Process)? + .store_read(store, identifier)) } pub(crate) fn process_image_async_read( async_read: A, args: Vec, format: ImageFormat, -) -> std::io::Result { - Ok(process_image(args, format)?.pipe_async_read(async_read)) +) -> Result { + 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 { + pub(crate) fn validate_input(&self) -> Result { 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) diff --git a/src/process.rs b/src/process.rs index d2798e3..1ef491f 100644 --- a/src/process.rs +++ b/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 { - 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 { + 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 { + fn spawn(cmd: &mut Command) -> std::io::Result { 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 {