From 81b73396341e4402d6f916a89ae60a10d24bd0a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Klemens=20Sch=C3=B6lhorn?= Date: Sun, 12 Mar 2023 21:11:30 +0100 Subject: [PATCH] Implement basic error handling with anyhow --- Cargo.lock | 1 + Cargo.toml | 1 + src/error.rs | 26 +++++++ src/main.rs | 212 ++++++++++++++++++++++++++------------------------- 4 files changed, 136 insertions(+), 104 deletions(-) create mode 100644 src/error.rs diff --git a/Cargo.lock b/Cargo.lock index a5f8ac1..8932e8d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -798,6 +798,7 @@ dependencies = [ name = "image-gallery" version = "0.1.0" dependencies = [ + "anyhow", "axum", "axum-sessions", "axum-template", diff --git a/Cargo.toml b/Cargo.toml index 385ae16..7602c10 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" edition = "2021" [dependencies] +anyhow = "1.0.69" axum = { version = "0.6.7", features = ["form", "macros"] } axum-sessions = "0.4.1" axum-template = { version = "0.14.0", features = ["minijinja"] } diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..ed8d47a --- /dev/null +++ b/src/error.rs @@ -0,0 +1,26 @@ +use axum::{response::{IntoResponse, Response}, http::StatusCode}; + +pub struct AppError(anyhow::Error); + +// Tell axum how to convert `AppError` into a response. +impl IntoResponse for AppError { + fn into_response(self) -> Response { + let status_code = match self.0.downcast_ref::() { + Some(status_code) => *status_code, + None => StatusCode::INTERNAL_SERVER_ERROR, + }; + tracing::error!("{:#}", self.0); + status_code.into_response() + } +} + +// This enables using `?` on functions that return `Result<_, anyhow::Error>` to turn them into +// `Result<_, AppError>`. That way you don't need to do that manually. +impl From for AppError +where + E: Into, +{ + fn from(err: E) -> Self { + Self(err.into()) + } +} \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index 3548872..9f1ae22 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,19 +1,23 @@ use std::{net::SocketAddr, path::{Path, PathBuf}, ffi::OsStr, fs::File, io::{BufReader, Seek, Cursor, BufRead}, time::SystemTime, env::args_os}; +use anyhow::{Context, Result, anyhow}; use axum::{Router, routing::{get, post}, response::{IntoResponse, Redirect}, http::{StatusCode, header}, extract::{self, State}, Form, handler::Handler}; use axum_sessions::{async_session::CookieStore, SessionLayer, extractors::{ReadableSession, WritableSession}}; use axum_template::{RenderHtml, engine::Engine}; +use error::AppError; use exif::{Tag, Value, Field, In, Exif}; use image::{imageops::FilterType, DynamicImage}; use rand::Rng; use serde::{Serialize, Deserialize}; use tokio::task; use tower_http::{trace::{self, TraceLayer}, compression::CompressionLayer}; -use tracing::{Level, debug_span}; -use tracing_subscriber::{EnvFilter, layer::SubscriberExt, util::SubscriberInitExt, fmt::format::FmtSpan}; +use tracing::Level; +use tracing_subscriber::{EnvFilter, layer::SubscriberExt, util::SubscriberInitExt}; + +mod error; // TODO -// * Proper error handling +// * sort images by date // * Cache generated images (+ cache cleanup) // * Add configuration file // * Support for headlines @@ -36,7 +40,6 @@ async fn main() { let tracing_filter = EnvFilter::try_from_default_env().unwrap_or(default_tracing); let tracing_formatter = tracing_subscriber::fmt::layer() .with_target(false) - .with_span_events(FmtSpan::CLOSE) // TODO: maybe drop this .compact(); tracing_subscriber::registry() .with(tracing_filter) @@ -92,24 +95,23 @@ async fn index( engine: TemplateEngine, State(image_dir): State, session: ReadableSession, -) -> impl IntoResponse { - - let logged_in = session.get("logged_in").unwrap_or(false); +) -> Result { + let logged_in = session.get::<()>("logged_in").is_some(); if logged_in { - let images = read_images(&image_dir); + let images = read_images(&image_dir)?; - let images = images.into_iter().rev().take(30).collect(); + let images = images.into_iter().rev().collect(); - RenderHtml("index", engine, IndexTempalte { + Ok(RenderHtml("index", engine, IndexTempalte { title: "Some pictures".into(), images, - }) + })) } else { - RenderHtml("login", engine, IndexTempalte { + Ok(RenderHtml("login", engine, IndexTempalte { title: "Some pictures".into(), images: vec![], - }) + })) } } @@ -123,7 +125,7 @@ async fn authenticate( Form(form): Form ) -> Redirect { if form.password == "testle" { - session.insert("logged_in", true).ok(); + session.insert("logged_in", ()).ok(); } Redirect::to("/") } @@ -132,81 +134,68 @@ async fn converted_image( extract::Path(image): extract::Path, State(image_dir): State, session: ReadableSession, -) -> Result { - if ! session.get("logged_in").unwrap_or(false) { - return Err(StatusCode::FORBIDDEN); - } +) -> Result { + session.get::<()>("logged_in") + .ok_or(anyhow!("Trying to load image while not logged in!")) + .context(StatusCode::FORBIDDEN)?; let image_path = image_dir.join(image); - if ! image_path.exists() { - return Err(StatusCode::NOT_FOUND); - } + image_path.exists() + .then_some(()) + .ok_or(anyhow!("Requested image not found!")) + .context(StatusCode::NOT_FOUND)?; - let image_buffer = task::spawn_blocking(move || -> Result<_, StatusCode> { - let file = File::open(&image_path).unwrap(); - let mut file = BufReader::new(file); - - // Read exif data first, as it only reads a tiny amount of data - let exif = read_exif_data(&mut file); - - let mut image = debug_span!("decode_image", - image=?image_path.file_name(), - ).in_scope(|| -> Result<_, StatusCode>{ - file.rewind().map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - image::io::Reader::new(&mut file) - .with_guessed_format() - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)? - .decode() - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR) - })?; - - // Fix image orientation - if let Ok(exif) = exif { - image = fix_image_orientation(image, &exif); - } - - let small_image = debug_span!("resize_image", - image=?image_path.file_name(), - ).in_scope(|| { - image.resize(600, 600, FilterType::Lanczos3) - }); - - let mut image_buffer = Cursor::new(Vec::new()); - - debug_span!("encode_image", - image=?image_path.file_name(), - ).in_scope(|| { - small_image.write_to(&mut image_buffer, image::ImageOutputFormat::WebP) - }).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - - Ok(image_buffer) - }).await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)??; + let image_buffer = task::spawn_blocking(move || { + convert_image(&image_path) + .with_context(|| format!("Could not convert image {:?}", image_path)) + }).await??; Ok(( [ (header::CONTENT_TYPE, "image/webp"), ], - image_buffer.into_inner() + image_buffer )) } -fn read_exif_data(mut file: impl BufRead+Seek) -> Result { - file.rewind().map_err(|_| ())?; - let exifreader = exif::Reader::new(); - exifreader.read_from_container(&mut file) - .map_err(|_| ()) +fn convert_image(image_path: &Path) -> Result> { + let file = File::open(image_path)?; + let mut file = BufReader::new(file); + + // Read exif data first, as it only reads a tiny amount of data + let exif = read_exif_data(&mut file); + + file.rewind()?; + let mut image = image::io::Reader::new(&mut file) + .with_guessed_format()? + .decode()?; + + // Fix image orientation + if let Ok(exif) = exif { + image = fix_image_orientation(image, &exif); + } + + let small_image = image.resize(600, 600, FilterType::Lanczos3); + + let mut image_buffer = Cursor::new(Vec::new()); + small_image.write_to(&mut image_buffer, image::ImageOutputFormat::WebP)?; + + Ok(image_buffer.into_inner()) } -fn read_image_size(mut file: impl BufRead+Seek, exif: Option<&Exif>) -> Result<(u32, u32), ()> { - file.rewind().map_err(|_| ())?; +fn read_exif_data(mut file: impl BufRead+Seek) -> Result { + file.rewind()?; + let exifreader = exif::Reader::new(); + Ok(exifreader.read_from_container(&mut file)?) +} + +fn read_image_size(mut file: impl BufRead+Seek, exif: Option<&Exif>) -> Result<(u32, u32)> { + file.rewind()?; let (mut width, mut height) = image::io::Reader::new(&mut file) - .with_guessed_format() - .map_err(|_| ())? - .into_dimensions() - .map_err(|_| ())?; + .with_guessed_format()? + .into_dimensions()?; if let Some(exif) = exif { if let Some(orientation) = exif.get_field(Tag::Orientation, In::PRIMARY) { @@ -238,44 +227,59 @@ fn fix_image_orientation(image: DynamicImage, exif: &Exif) -> DynamicImage { } } - -fn read_images(directory: &Path) -> Vec { +fn read_images(directory: &Path) -> Result> { let mut files = vec![]; - for file in directory.read_dir().expect("read_dir call failed").flatten() { + + let directory_iterator = directory + .read_dir() + .with_context(|| format!("Could not read files in directory {:?}", directory))?.flatten(); + + for file in directory_iterator { let path = file.path(); if path.extension() == Some(OsStr::new("jpg")) { - let file = File::open(&path).unwrap(); - let mut file = BufReader::new(file); - - let exif = read_exif_data(&mut file); - - // Check if we need to flip the coordinates - let (width, height) = read_image_size(&mut file, exif.as_ref().ok()).unwrap(); - - let datetime = 'datetime: { - // First, try to read creation date from EXIF data - if let Ok(ref exif) = exif { - match exif.get_field(Tag::DateTimeOriginal, In::PRIMARY) { - Some(Field { value: Value::Ascii(value), ..}) if !value.is_empty() => { - break 'datetime String::from_utf8_lossy(&value[0]).into_owned() - } - _ => {} - }; + let image_info = match read_image_info(&path) { + Ok(image_info) => image_info, + Err(error) => { + tracing::warn!("Skipping {:?} due to error: {:#}", path, error); + continue; } - - // If that doesn't work, fall back to the file modification time - format!("{:?}", std::fs::metadata(&path).unwrap().modified().unwrap().duration_since(SystemTime::UNIX_EPOCH).unwrap()) }; - - let image_info = ImageInfo { - width, - height, - name: path.file_name().expect("invalid file path").to_string_lossy().to_string(), - }; - files.push(image_info); } } - files + + Ok(files) +} + +fn read_image_info(path: &Path) -> Result { + let file = File::open(path)?; + let mut file = BufReader::new(file); + + let exif = read_exif_data(&mut file); + + // Check if we need to flip the coordinates + let (width, height) = read_image_size(&mut file, exif.as_ref().ok()) + .context("Could not read image size")?; + + let datetime = 'datetime: { + // First, try to read creation date from EXIF data + if let Ok(ref exif) = exif { + match exif.get_field(Tag::DateTimeOriginal, In::PRIMARY) { + Some(Field { value: Value::Ascii(value), ..}) if !value.is_empty() => { + break 'datetime String::from_utf8_lossy(&value[0]).into_owned() + } + _ => {} + }; + } + + // If that doesn't work, fall back to the file modification time + format!("{:?}", std::fs::metadata(path).unwrap().modified().unwrap().duration_since(SystemTime::UNIX_EPOCH).unwrap()) + }; + + Ok(ImageInfo { + width, + height, + name: path.file_name().expect("invalid file path").to_string_lossy().to_string(), + }) }