Use a custom error type in core

Cargo.lock

@@ -1424,6 +1424,7 @@ dependencies = [
  "env_logger",
  "log",
  "pretty_assertions",
+ "thiserror",
  "uuid",
 ]
 

core/Cargo.toml

@@ -9,6 +9,7 @@ license = "MIT"
 [dependencies]
 uuid.workspace = true
 anyhow.workspace = true
+thiserror.workspace = true
 log.workspace = true
 env_logger.workspace = true
 chrono.workspace = true

core/src/error.rs

@@ -0,0 +1,9 @@
+#[derive(Debug, thiserror::Error)]
+pub enum ServerError {
+    /// There is no client with the given ClientId.
+    #[error("No such client")]
+    NoSuchClient,
+
+    #[error(transparent)]
+    Other(#[from] anyhow::Error), // source and Display delegate to anyhow::Error
+}

core/src/lib.rs

@@ -23,10 +23,12 @@
 //! * [`ServerConfig`] providing basic configuration for the server's behavior.
 //! * `client_id` and a [`Client`] providing the client metadata.
 
+mod error;
 mod inmemory;
 mod server;
 mod storage;
 
+pub use error::*;
 pub use inmemory::*;
 pub use server::*;
 pub use storage::*;

core/src/server.rs

@@ -1,3 +1,4 @@
+use crate::error::ServerError;
 use crate::storage::{Client, Snapshot, Storage, StorageTxn};
 use chrono::Utc;
 use uuid::Uuid;
@@ -110,7 +111,7 @@ impl Server {
         client_id: ClientId,
         client: Client,
         parent_version_id: VersionId,
-    ) -> anyhow::Result<GetVersionResult> {
+    ) -> Result<GetVersionResult, ServerError> {
         let mut txn = self.storage.txn()?;
 
         // If a version with parentVersionId equal to the requested parentVersionId exists, it is
@@ -147,7 +148,7 @@ impl Server {
         client: Client,
         parent_version_id: VersionId,
         history_segment: HistorySegment,
-    ) -> anyhow::Result<(AddVersionResult, SnapshotUrgency)> {
+    ) -> Result<(AddVersionResult, SnapshotUrgency), ServerError> {
         let mut txn = self.storage.txn()?;
 
         log::debug!(
@@ -206,7 +207,7 @@ impl Server {
         client: Client,
         version_id: VersionId,
         data: Vec<u8>,
-    ) -> anyhow::Result<()> {
+    ) -> Result<(), ServerError> {
         let mut txn = self.storage.txn()?;
 
         log::debug!(
@@ -290,7 +291,7 @@ impl Server {
         &self,
         client_id: ClientId,
         client: Client,
-    ) -> anyhow::Result<Option<(Uuid, Vec<u8>)>> {
+    ) -> Result<Option<(Uuid, Vec<u8>)>, ServerError> {
         let mut txn = self.storage.txn()?;
 
         Ok(if let Some(snap) = client.snapshot {
@@ -302,8 +303,8 @@ impl Server {
     }
 
     /// Convenience method to get a transaction for the embedded storage.
-    pub fn txn(&self) -> anyhow::Result<Box<dyn StorageTxn + '_>> {
+    pub fn txn(&self) -> Result<Box<dyn StorageTxn + '_>, ServerError> {
-        self.storage.txn()
+        Ok(self.storage.txn()?)
     }
 }
 

server/src/api/add_snapshot.rs

@@ -1,4 +1,6 @@
-use crate::api::{client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE};
+use crate::api::{
+    client_id_header, failure_to_ise, server_error_to_actix, ServerState, SNAPSHOT_CONTENT_TYPE,
+};
 use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result};
 use futures::StreamExt;
 use std::sync::Arc;
@@ -50,7 +52,7 @@ pub(crate) async fn service(
     // completely, to avoid blocking other storage access while that data is
     // in transit.
     let client = {
-        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
+        let mut txn = server_state.server.txn().map_err(server_error_to_actix)?;
 
         // get, or create, the client
         match txn.get_client(client_id).map_err(failure_to_ise)? {
@@ -66,7 +68,7 @@ pub(crate) async fn service(
     server_state
         .server
         .add_snapshot(client_id, client, version_id, body.to_vec())
-        .map_err(failure_to_ise)?;
+        .map_err(server_error_to_actix)?;
     Ok(HttpResponse::Ok().body(""))
 }
 

server/src/api/add_version.rs

@@ -1,6 +1,7 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
+    client_id_header, failure_to_ise, server_error_to_actix, ServerState,
-    PARENT_VERSION_ID_HEADER, SNAPSHOT_REQUEST_HEADER, VERSION_ID_HEADER,
+    HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, SNAPSHOT_REQUEST_HEADER,
+    VERSION_ID_HEADER,
 };
 use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result};
 use futures::StreamExt;
@@ -58,7 +59,7 @@ pub(crate) async fn service(
     // completely, to avoid blocking other storage access while that data is
     // in transit.
     let client = {
-        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
+        let mut txn = server_state.server.txn().map_err(server_error_to_actix)?;
 
         match txn.get_client(client_id).map_err(failure_to_ise)? {
             Some(client) => client,
@@ -73,7 +74,7 @@ pub(crate) async fn service(
     let (result, snap_urgency) = server_state
         .server
         .add_version(client_id, client, parent_version_id, body.to_vec())
-        .map_err(failure_to_ise)?;
+        .map_err(server_error_to_actix)?;
 
     Ok(match result {
         AddVersionResult::Ok(version_id) => {

server/src/api/get_child_version.rs

@@ -1,6 +1,6 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
+    client_id_header, failure_to_ise, server_error_to_actix, ServerState,
-    PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
+    HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
 };
 use actix_web::{error, get, web, HttpRequest, HttpResponse, Result};
 use std::sync::Arc;
@@ -23,7 +23,7 @@ pub(crate) async fn service(
     let parent_version_id = path.into_inner();
 
     let (client, client_id) = {
-        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
+        let mut txn = server_state.server.txn().map_err(server_error_to_actix)?;
 
         let client_id = client_id_header(&req)?;
 
@@ -37,7 +37,7 @@ pub(crate) async fn service(
     return match server_state
         .server
         .get_child_version(client_id, client, parent_version_id)
-        .map_err(failure_to_ise)?
+        .map_err(server_error_to_actix)?
     {
         GetVersionResult::Success {
             version_id,

server/src/api/get_snapshot.rs

@@ -1,5 +1,6 @@
 use crate::api::{
-    client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE, VERSION_ID_HEADER,
+    client_id_header, failure_to_ise, server_error_to_actix, ServerState, SNAPSHOT_CONTENT_TYPE,
+    VERSION_ID_HEADER,
 };
 use actix_web::{error, get, web, HttpRequest, HttpResponse, Result};
 use std::sync::Arc;
@@ -20,7 +21,7 @@ pub(crate) async fn service(
     let client_id = client_id_header(&req)?;
 
     let client = {
-        let mut txn = server_state.server.txn().map_err(failure_to_ise)?;
+        let mut txn = server_state.server.txn().map_err(server_error_to_actix)?;
         txn.get_client(client_id)
             .map_err(failure_to_ise)?
             .ok_or_else(|| error::ErrorNotFound("no such client"))?
@@ -29,7 +30,7 @@ pub(crate) async fn service(
     if let Some((version_id, data)) = server_state
         .server
         .get_snapshot(client_id, client)
-        .map_err(failure_to_ise)?
+        .map_err(server_error_to_actix)?
     {
         Ok(HttpResponse::Ok()
             .content_type(SNAPSHOT_CONTENT_TYPE)

server/src/api/mod.rs

@@ -1,5 +1,5 @@
 use actix_web::{error, http::StatusCode, web, HttpRequest, Result, Scope};
-use taskchampion_sync_server_core::{ClientId, Server};
+use taskchampion_sync_server_core::{ClientId, Server, ServerError};
 
 mod add_snapshot;
 mod add_version;
@@ -43,6 +43,14 @@ fn failure_to_ise(err: anyhow::Error) -> impl actix_web::ResponseError {
     error::InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR)
 }
 
+/// Convert a ServerError to an Actix error
+fn server_error_to_actix(err: ServerError) -> actix_web::Error {
+    match err {
+        ServerError::NoSuchClient => error::ErrorNotFound(err),
+        ServerError::Other(err) => error::ErrorInternalServerError(err),
+    }
+}
+
 /// Get the client id
 fn client_id_header(req: &HttpRequest) -> Result<ClientId> {
     fn badrequest() -> error::Error {