Refactor error handling

.pre-commit-config.yaml

@@ -0,0 +1,7 @@
+repos:
+  - repo: https://github.com/doublify/pre-commit-rust
+    rev: v1.0
+    hooks:
+    - id: fmt
+    - id: cargo-check
+    - id: clippy

Cargo.lock

@@ -216,6 +216,12 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "anyhow"
+version = "1.0.53"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "94a45b455c14666b85fc40a019e8ab9eb75e3a124e05494f5397122bc9eb06e0"
+
 [[package]]
 name = "autocfg"
 version = "1.0.1"
@@ -325,6 +331,7 @@ version = "0.1.0"
 dependencies = [
  "actix-http",
  "actix-web",
+ "anyhow",
  "chrono",
  "configparser",
  "contextswitch-types",
@@ -337,6 +344,7 @@ dependencies = [
  "rstest",
  "serde",
  "serde_json",
+ "thiserror",
  "tokio",
  "tracing",
  "tracing-actix-web",
@@ -349,7 +357,7 @@ dependencies = [
 [[package]]
 name = "contextswitch-types"
 version = "0.1.0"
-source = "git+https://github.com/dax/contextswitch-types.git#cc6db5cc18ab9d67998065004e912756d0a81e28"
+source = "git+https://github.com/dax/contextswitch-types.git#786d9980bedb32432d0830b3b951168dcb17b56b"
 dependencies = [
  "chrono",
  "serde",
@@ -1406,6 +1414,26 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "thiserror"
+version = "1.0.30"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "854babe52e4df1653706b98fcfc05843010039b406875930a70e4d9644e5c417"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "1.0.30"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "thread_local"
 version = "1.1.3"

Cargo.toml

@@ -31,6 +31,8 @@ tracing-actix-web = "=0.5.0-beta.9"
 regex = "1.5.0"
 lazy_static = "1.4.0"
 tracing-bunyan-formatter = "0.3.2"
+thiserror = "1.0.30"
+anyhow = "1.0.53"
 
 [dev-dependencies]
 reqwest = { version = "0.11.0", features = ["json"] }

src/contextswitch/api.rs

@@ -1,10 +1,42 @@
 use crate::contextswitch::taskwarrior;
 use contextswitch_types::Task;
-use std::io::Error;
+use serde_json;
+
+fn error_chain_fmt(
+    e: &impl std::error::Error,
+    f: &mut std::fmt::Formatter<'_>,
+) -> std::fmt::Result {
+    writeln!(f, "{}\n", e)?;
+    let mut current = e.source();
+    while let Some(cause) = current {
+        writeln!(f, "Caused by:\n\t{}", cause)?;
+        current = cause.source();
+    }
+    Ok(())
+}
+
+impl std::fmt::Debug for ContextswitchError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        error_chain_fmt(self, f)
+    }
+}
+
+#[derive(thiserror::Error)]
+pub enum ContextswitchError {
+    #[error("Invalid Contextswitch metadata: {metadata}")]
+    InvalidMetadataError {
+        #[source]
+        source: serde_json::Error,
+        metadata: String,
+    },
+    #[error(transparent)]
+    UnexpectedError(#[from] anyhow::Error),
+}
 
 #[tracing::instrument(level = "debug")]
-pub fn list_tasks(filters: Vec<&str>) -> Result<Vec<Task>, Error> {
+pub fn list_tasks(filters: Vec<&str>) -> Result<Vec<Task>, ContextswitchError> {
-    let tasks: Result<Vec<Task>, Error> = taskwarrior::list_tasks(filters)?
+    let tasks: Result<Vec<Task>, ContextswitchError> = taskwarrior::list_tasks(filters)
+        .map_err(|e| ContextswitchError::UnexpectedError(e.into()))?
         .iter()
         .map(Task::try_from)
         .collect();
@@ -12,7 +44,9 @@ pub fn list_tasks(filters: Vec<&str>) -> Result<Vec<Task>, Error> {
 }
 
 #[tracing::instrument(level = "debug")]
-pub async fn add_task(add_args: Vec<&str>) -> Result<Task, Error> {
+pub async fn add_task(add_args: Vec<&str>) -> Result<Task, ContextswitchError> {
-    let taskwarrior_task = taskwarrior::add_task(add_args).await?;
+    let taskwarrior_task = taskwarrior::add_task(add_args)
+        .await
+        .map_err(|e| ContextswitchError::UnexpectedError(e.into()))?;
     (&taskwarrior_task).try_into()
 }

src/contextswitch/taskwarrior.rs

@@ -1,3 +1,5 @@
+use crate::contextswitch::ContextswitchError;
+use anyhow::{anyhow, Context};
 use chrono::{DateTime, Utc};
 use configparser::ini::Ini;
 use contextswitch_types::{ContextSwitchMetadata, Task, TaskId};
@@ -6,7 +8,6 @@ use serde::{Deserialize, Serialize};
 use serde_json;
 use std::env;
 use std::fmt;
-use std::io::{Error, ErrorKind};
 use std::path::Path;
 use std::process::Command;
 use std::str;
@@ -60,12 +61,26 @@ pub struct TaskwarriorTask {
         skip_serializing_if = "Option::is_none",
         with = "contextswitch_types::opt_tw_date_format"
     )]
+    pub start: Option<DateTime<Utc>>,
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        with = "contextswitch_types::opt_tw_date_format"
+    )]
     pub end: Option<DateTime<Utc>>,
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        with = "contextswitch_types::opt_tw_date_format"
+    )]
+    pub wait: Option<DateTime<Utc>>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub parent: Option<Uuid>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub project: Option<String>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub priority: Option<contextswitch_types::Priority>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
     pub recur: Option<contextswitch_types::Recurrence>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub tags: Option<Vec<String>>,
@@ -74,16 +89,21 @@ pub struct TaskwarriorTask {
 }
 
 impl TryFrom<&TaskwarriorTask> for Task {
-    type Error = std::io::Error;
+    type Error = ContextswitchError;
 
     fn try_from(task: &TaskwarriorTask) -> Result<Self, Self::Error> {
         let cs_metadata = task.contextswitch.as_ref().map_or(
             Ok(None),
-            |cs_string| -> Result<Option<ContextSwitchMetadata>, serde_json::Error> {
+            |cs_string| -> Result<Option<ContextSwitchMetadata>, ContextswitchError> {
                 if cs_string.is_empty() || cs_string == "{}" {
                     Ok(None)
                 } else {
-                    Some(serde_json::from_str(cs_string)).transpose()
+                    Some(serde_json::from_str(cs_string))
+                        .transpose()
+                        .map_err(|e| ContextswitchError::InvalidMetadataError {
+                            source: e,
+                            metadata: cs_string.to_string(),
+                        })
                 }
             },
         )?;
@@ -96,9 +116,12 @@ impl TryFrom<&TaskwarriorTask> for Task {
             description: task.description.clone(),
             //urgency: task.urgency,
             due: task.due,
+            start: task.start,
             end: task.end,
+            wait: task.wait,
             parent: task.parent,
             project: task.project.clone(),
+            priority: task.priority,
             recur: task.recur,
             tags: task.tags.clone(),
             contextswitch: cs_metadata,
@@ -106,6 +129,20 @@ impl TryFrom<&TaskwarriorTask> for Task {
     }
 }
 
+#[derive(thiserror::Error, Debug)]
+pub enum TaskwarriorError {
+    #[error("Error while executing Taskwarrior")]
+    ExecutionError(#[from] std::io::Error),
+    #[error("Error while parsing Taskwarrior output")]
+    OutputParsingError {
+        #[source]
+        source: serde_json::Error,
+        output: String,
+    },
+    #[error(transparent)]
+    UnexpectedError(#[from] anyhow::Error),
+}
+
 fn write_default_config(data_location: &str) -> String {
     let mut taskrc = Ini::new();
     taskrc.setstr("default", "data.location", Some(data_location));
@@ -159,33 +196,39 @@ pub fn load_config(task_data_location: Option<&str>) -> String {
 }
 
 #[tracing::instrument(level = "debug")]
-pub fn list_tasks(filters: Vec<&str>) -> Result<Vec<TaskwarriorTask>, Error> {
+pub fn list_tasks(filters: Vec<&str>) -> Result<Vec<TaskwarriorTask>, TaskwarriorError> {
     let args = [filters, vec!["export"]].concat();
-    let export_output = Command::new("task").args(args).output()?;
+    let export_output = Command::new("task")
+        .args(args)
+        .output()
+        .map_err(TaskwarriorError::ExecutionError)?;
 
-    let tasks: Vec<TaskwarriorTask> = serde_json::from_slice(&export_output.stdout)?;
+    let output =
+        String::from_utf8(export_output.stdout).context("Failed to read Taskwarrior output")?;
+
+    let tasks: Vec<TaskwarriorTask> = serde_json::from_str(&output)
+        .map_err(|e| TaskwarriorError::OutputParsingError { source: e, output })?;
 
     Ok(tasks)
 }
 
 #[tracing::instrument(level = "debug")]
-pub fn get_task_by_local_id(id: &TaskwarriorTaskLocalId) -> Result<Option<TaskwarriorTask>, Error> {
+pub fn get_task_by_local_id(
+    id: &TaskwarriorTaskLocalId,
+) -> Result<Option<TaskwarriorTask>, TaskwarriorError> {
     let mut tasks: Vec<TaskwarriorTask> = list_tasks(vec![&id.to_string()])?;
     if tasks.len() > 1 {
-        return Err(Error::new(
+        return Err(TaskwarriorError::UnexpectedError(anyhow!(
-            ErrorKind::Other,
-            format!(
             "Found more than 1 task when searching for task with local ID {}",
             id
-            ),
+        )));
-        ));
     }
 
     Ok(tasks.pop())
 }
 
 #[tracing::instrument(level = "debug")]
-pub async fn add_task(add_args: Vec<&str>) -> Result<TaskwarriorTask, Error> {
+pub async fn add_task(add_args: Vec<&str>) -> Result<TaskwarriorTask, TaskwarriorError> {
     lazy_static! {
         static ref RE: Regex = Regex::new(r"Created task (?P<id>\d+).").unwrap();
         static ref LOCK: Mutex<u32> = Mutex::new(0);
@@ -194,35 +237,31 @@ pub async fn add_task(add_args: Vec<&str>) -> Result<TaskwarriorTask, Error> {
 
     let mut args = vec!["add"];
     args.extend(add_args);
-    let add_output = Command::new("task").args(args).output()?;
+    let add_output = Command::new("task")
-    let output = String::from_utf8(add_output.stdout).unwrap();
+        .args(args)
-    let task_id_capture = RE.captures(&output).ok_or_else(|| {
+        .output()
-        Error::new(
+        .map_err(TaskwarriorError::ExecutionError)?;
-            ErrorKind::Other,
+    let output =
-            format!("Cannot extract task ID from: {}", &output),
+        String::from_utf8(add_output.stdout).context("Failed to read Taskwarrior output")?;
-        )
+    let task_id_capture = RE
-    })?;
+        .captures(&output)
+        .ok_or_else(|| anyhow!("Cannot extract task ID from: {}", &output))?;
     let task_id_str = task_id_capture
         .name("id")
-        .ok_or_else(|| {
+        .ok_or_else(|| anyhow!("Cannot extract task ID value from: {}", &output))?
-            Error::new(
-                ErrorKind::Other,
-                format!("Cannot extract task ID value from: {}", &output),
-            )
-        })?
         .as_str();
 
     let task_id = TaskwarriorTaskLocalId(
         task_id_str
             .parse::<u64>()
-            .map_err(|_| Error::new(ErrorKind::Other, "Cannot parse task ID value"))?,
+            .context("Cannot parse task ID value")?,
     );
 
     let task = get_task_by_local_id(&task_id)?;
     task.ok_or_else(|| {
-        Error::new(
+        TaskwarriorError::UnexpectedError(anyhow!(
-            ErrorKind::Other,
+            "Newly created task with ID {} was not found",
-            format!("Newly created task with ID {} was not found", task_id),
+            task_id
-        )
+        ))
     })
 }

src/routes/tasks.rs

@@ -1,16 +1,31 @@
 use crate::contextswitch;
-use actix_web::{web, HttpResponse};
+use actix_web::{http::StatusCode, web, HttpResponse, ResponseError};
+use anyhow::Context;
 use contextswitch_types::{NewTask, Task};
 use serde::Deserialize;
-use std::io::Error;
 
 #[derive(Deserialize)]
 pub struct TaskQuery {
     filter: Option<String>,
 }
 
+impl ResponseError for contextswitch::ContextswitchError {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            contextswitch::ContextswitchError::InvalidMetadataError { .. } => {
+                StatusCode::INTERNAL_SERVER_ERROR
+            }
+            contextswitch::ContextswitchError::UnexpectedError(_) => {
+                StatusCode::INTERNAL_SERVER_ERROR
+            }
+        }
+    }
+}
+
 #[tracing::instrument(level = "debug", skip_all, fields(filter = %task_query.filter.as_ref().unwrap_or(&"".to_string())))]
-pub async fn list_tasks(task_query: web::Query<TaskQuery>) -> Result<HttpResponse, Error> {
+pub async fn list_tasks(
+    task_query: web::Query<TaskQuery>,
+) -> Result<HttpResponse, contextswitch::ContextswitchError> {
     let filter = task_query
         .filter
         .as_ref()
@@ -19,16 +34,18 @@ pub async fn list_tasks(task_query: web::Query<TaskQuery>) -> Result<HttpRespons
 
     Ok(HttpResponse::Ok()
         .content_type("application/json")
-        .body(serde_json::to_string(&tasks)?))
+        .body(serde_json::to_string(&tasks).context("Cannot serialize Contextswitch task")?))
 }
 
 #[tracing::instrument(level = "debug", skip_all, fields(definition = %task.definition))]
-pub async fn add_task(task: web::Json<NewTask>) -> Result<HttpResponse, Error> {
+pub async fn add_task(
+    task: web::Json<NewTask>,
+) -> Result<HttpResponse, contextswitch::ContextswitchError> {
     let task: Task = contextswitch::add_task(task.definition.split(' ').collect()).await?;
 
     Ok(HttpResponse::Ok()
         .content_type("application/json")
-        .body(serde_json::to_string(&task)?))
+        .body(serde_json::to_string(&task).context("Cannot serialize Contextswitch task")?))
 }
 
 #[tracing::instrument(level = "debug")]