First steps with hopefully better error handling

crates/dav/Cargo.toml

@@ -22,3 +22,4 @@ serde = { version = "1.0.188", features = ["serde_derive", "derive"] }
 serde_json = "1.0.105"
 tokio = { version = "1.32.0", features = ["sync", "full"] }
 async-trait = "0.1.73"
+thiserror = "1.0.48"

crates/dav/src/error.rs

@@ -1,24 +1,33 @@
 use actix_web::{http::StatusCode, HttpResponse};
-use derive_more::{Display, Error};
+use thiserror::Error;
 
-#[derive(Debug, Display, Error)]
+use crate::routes::propfind;
+
+#[derive(Debug, Error)]
 pub enum Error {
-    #[display(fmt = "Internal server error")]
-    InternalError,
-    #[display(fmt = "Not found")]
+    #[error("Not found")]
     NotFound,
-    #[display(fmt = "Bad request")]
+    #[error("Bad request")]
     BadRequest,
+    #[error("Unauthorized")]
     Unauthorized,
+    #[error("Internal server error :(")]
+    InternalError,
+    #[error(transparent)]
+    PropfindError(#[from] propfind::Error),
+    #[error("Internal server error")]
+    Other(#[from] anyhow::Error),
 }
 
 impl actix_web::error::ResponseError for Error {
     fn status_code(&self) -> StatusCode {
-        match *self {
+        match self {
             Self::InternalError => StatusCode::INTERNAL_SERVER_ERROR,
+            Self::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
             Self::NotFound => StatusCode::NOT_FOUND,
             Self::BadRequest => StatusCode::BAD_REQUEST,
             Self::Unauthorized => StatusCode::UNAUTHORIZED,
+            Self::PropfindError(e) => e.status_code(),
         }
     }
 

crates/dav/src/routes/event.rs

@@ -1,8 +1,27 @@
-use crate::{CalDavContext, Error};
+use crate::CalDavContext;
+use actix_web::http::StatusCode;
 use actix_web::web::{Data, Path};
-use actix_web::HttpResponse;
+use actix_web::{HttpResponse, ResponseError};
 use rustical_auth::{AuthInfoExtractor, CheckAuthentication};
 use rustical_store::calendar::CalendarStore;
+use thiserror::Error;
+
+#[derive(Debug, Error)]
+pub enum Error {
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
+impl ResponseError for Error {
+    fn status_code(&self) -> actix_web::http::StatusCode {
+        match self {
+            Self::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
+    fn error_response(&self) -> HttpResponse<actix_web::body::BoxBody> {
+        HttpResponse::build(self.status_code()).body(self.to_string())
+    }
+}
 
 pub async fn delete_event<A: CheckAuthentication, C: CalendarStore>(
     context: Data<CalDavContext<C>>,
@@ -15,13 +34,7 @@ pub async fn delete_event<A: CheckAuthentication, C: CalendarStore>(
     if cid.ends_with(".ics") {
         cid.truncate(cid.len() - 4);
     }
-    context
-        .store
-        .write()
-        .await
-        .delete_event(&cid, &uid)
-        .await
-        .map_err(|_e| Error::InternalError)?;
+    context.store.write().await.delete_event(&cid, &uid).await?;
 
     Ok(HttpResponse::Ok().body(""))
 }
@@ -36,13 +49,7 @@ pub async fn get_event<A: CheckAuthentication, C: CalendarStore>(
     if uid.ends_with(".ics") {
         uid.truncate(uid.len() - 4);
     }
-    let event = context
-        .store
-        .read()
-        .await
-        .get_event(&cid, &uid)
-        .await
-        .map_err(|_e| Error::NotFound)?;
+    let event = context.store.read().await.get_event(&cid, &uid).await?;
 
     Ok(HttpResponse::Ok()
         .insert_header(("ETag", event.get_etag()))
@@ -66,8 +73,7 @@ pub async fn put_event<A: CheckAuthentication, C: CalendarStore>(
         .write()
         .await
         .upsert_event(cid, uid, body)
-        .await
-        .map_err(|_e| Error::InternalError)?;
+        .await?;
 
     Ok(HttpResponse::Ok().body(""))
 }

crates/dav/src/routes/propfind.rs

@@ -1,18 +1,43 @@
 use crate::depth_extractor::Depth;
-use crate::error::Error;
 use crate::namespace::Namespace;
 use crate::resource::{HandlePropfind, Resource};
 use crate::xml_snippets::generate_multistatus;
 use crate::CalDavContext;
 use actix_web::http::header::ContentType;
+use actix_web::http::StatusCode;
 use actix_web::web::{Data, Path};
 use actix_web::{HttpRequest, HttpResponse};
-use anyhow::{anyhow, Result};
+use anyhow::Result;
 use quick_xml::events::BytesText;
 use rustical_auth::{AuthInfoExtractor, CheckAuthentication};
 use rustical_store::calendar::CalendarStore;
+use thiserror::Error;
 
-fn parse_propfind(body: &str) -> Result<Vec<&str>> {
+#[derive(Debug, Error)]
+pub enum Error {
+    #[error("invalid propfind request: {0}")]
+    InvalidPropfind(&'static str),
+    #[error("input is not valid xml")]
+    ParsingError(#[from] roxmltree::Error),
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
+
+impl actix_web::error::ResponseError for Error {
+    fn status_code(&self) -> actix_web::http::StatusCode {
+        match self {
+            Self::InvalidPropfind(_) => StatusCode::BAD_REQUEST,
+            Self::ParsingError(_) => StatusCode::BAD_REQUEST,
+            Self::Other(_) => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
+
+    fn error_response(&self) -> HttpResponse<actix_web::body::BoxBody> {
+        HttpResponse::build(self.status_code()).body(self.to_string())
+    }
+}
+
+fn parse_propfind(body: &str) -> Result<Vec<&str>, Error> {
     if body.is_empty() {
         // if body is empty, allprops must be returned (RFC 4918)
         return Ok(vec!["allprops"]);
@@ -21,7 +46,7 @@ fn parse_propfind(body: &str) -> Result<Vec<&str>> {
 
     let propfind_node = doc.root_element();
     if propfind_node.tag_name().name() != "propfind" {
-        return Err(anyhow!("invalid tag"));
+        return Err(Error::InvalidPropfind("root tag is not <propfind>"));
     }
 
     let prop_node = if let Some(el) = propfind_node.first_element_child() {
@@ -30,15 +55,15 @@ fn parse_propfind(body: &str) -> Result<Vec<&str>> {
         return Ok(Vec::new());
     };
 
-    let props = match prop_node.tag_name().name() {
+    match prop_node.tag_name().name() {
         "prop" => Ok(prop_node
             .children()
             .map(|node| node.tag_name().name())
             .collect()),
-        _ => Err(anyhow!("invalid prop tag")),
-    };
-    dbg!(body, &props);
-    props
+        _ => Err(Error::InvalidPropfind(
+            "invalid tag in <propfind>, expected <prop>",
+        )),
+    }
 }
 
 pub async fn route_propfind<A: CheckAuthentication, R: Resource, C: CalendarStore>(
@@ -49,7 +74,7 @@ pub async fn route_propfind<A: CheckAuthentication, R: Resource, C: CalendarStor
     auth: AuthInfoExtractor<A>,
     depth: Depth,
 ) -> Result<HttpResponse, Error> {
-    let props = parse_propfind(&body).map_err(|_e| Error::BadRequest)?;
+    let props = parse_propfind(&body)?;
     // let req_path = req.path().to_string();
     let auth_info = auth.inner;
 
@@ -59,24 +84,13 @@ pub async fn route_propfind<A: CheckAuthentication, R: Resource, C: CalendarStor
         path.into_inner(),
         context.prefix.to_string(),
     )
-    .await
-    .map_err(|_e| Error::InternalError)?;
+    .await?;
 
-    let mut responses = vec![resource
-        .propfind(props.clone())
-        .map_err(|_e| Error::InternalError)?];
+    let mut responses = vec![resource.propfind(props.clone())?];
 
     if depth != Depth::Zero {
-        for member in resource
-            .get_members()
-            .await
-            .map_err(|_e| Error::InternalError)?
-        {
-            responses.push(
-                member
-                    .propfind(props.clone())
-                    .map_err(|_e| Error::InternalError)?,
-            );
+        for member in resource.get_members().await? {
+            responses.push(member.propfind(props.clone())?);
         }
     }
 
@@ -90,8 +104,7 @@ pub async fn route_propfind<A: CheckAuthentication, R: Resource, C: CalendarStor
             }
             Ok(())
         },
-    )
-    .map_err(|_e| Error::InternalError)?;
+    )?;
 
     Ok(HttpResponse::MultiStatus()
         .content_type(ContentType::xml())