Implement basic error handling with anyhow

Cargo.lock

@@ -798,6 +798,7 @@ dependencies = [
 name = "image-gallery"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "axum",
  "axum-sessions",
  "axum-template",

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"] }

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::<StatusCode>() {
+            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<E> From<E> for AppError
+where
+    E: Into<anyhow::Error>,
+{
+    fn from(err: E) -> Self {
+        Self(err.into())
+    }
+}

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<ImageDir>,
     session: ReadableSession,
-) -> impl IntoResponse {
-
-    let logged_in = session.get("logged_in").unwrap_or(false);
+) -> Result<impl IntoResponse, AppError> {
+    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<AuthenticateForm>
 ) -> 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<String>,
     State(image_dir): State<ImageDir>,
     session: ReadableSession,
-) -> Result<impl IntoResponse, impl IntoResponse> {
-    if ! session.get("logged_in").unwrap_or(false) {
-        return Err(StatusCode::FORBIDDEN);
-    }
+) -> Result<impl IntoResponse, AppError> {
+    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<Exif, ()> {
-    file.rewind().map_err(|_| ())?;
-    let exifreader = exif::Reader::new();
-    exifreader.read_from_container(&mut file)
-        .map_err(|_| ())
+fn convert_image(image_path: &Path) -> Result<Vec<u8>> {
+    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<Exif> {
+    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<ImageInfo> {
+fn read_images(directory: &Path) -> Result<Vec<ImageInfo>> {
     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<ImageInfo> {
+    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(),
+    })
 }