Merge pull request #265 from adorsys/242-enhance-error-tracing-and-lo…

…gging-across-the-codebase-e3

242 enhance error tracing and logging across the codebase e3

.github/workflows/CI.yml

@@ -1,6 +1,6 @@
 name: Rust CI
 
-on:[pull_request]
+on: [pull_request]
 
 env:
   CARGO_TERM_COLOR: always

Cargo.toml

@@ -40,6 +40,7 @@ shared = { path = "./crates/web-plugins/didcomm-messaging/shared", version = "0.
 pickup = { path = "./crates/web-plugins/didcomm-messaging/protocols/pickup", version = "0.1.0" }
 forward = { path = "./crates/web-plugins/didcomm-messaging/protocols/forward", version = "0.1.0" }
 trust-ping = { path = "./crates/web-plugins/didcomm-messaging/protocols/trust-ping", version = "0.1.0" }
+discover-features = { path = "./crates/web-plugins/didcomm-messaging/protocols/discover-features", version = "0.1.0" }
 mediator-coordination = { path = "./crates/web-plugins/didcomm-messaging/protocols/mediator-coordination", version = "0.1.0" }
 
 # Other common dependencies
@@ -62,6 +63,8 @@ url = "2.4.1"
 num-bigint = "0.4.4"
 base64 = "0.13.0"
 hex = "0.4.3"
+eyre = "0.6"
+anyhow = "1"
 subtle = "2.5.0"
 regex = "1.10.2"
 mongodb = "2.7.1"
@@ -93,6 +96,8 @@ plugin-api.workspace = true
 
 axum.workspace = true
 dotenv-flow.workspace = true
+eyre.workspace = true
+thiserror.workspace = true
 tracing.workspace = true
 lazy_static.workspace = true
 serde_json.workspace = true

crates/filesystem/src/lib.rs

@@ -61,10 +61,17 @@ impl FileSystem for StdFileSystem {
         flock(file.as_raw_fd(), FlockArg::LockExclusive)
             .map_err(|_| IoError::new(ErrorKind::Other, "Error acquiring file lock"))?;
 
-        std::fs::write(path, &content).expect("Error saving base64-encoded image to file");
+        std::fs::write(path, &content).map_err(|_| {
+            IoError::new(
+                ErrorKind::Other,
+                "Error saving base64-encoded image to file",
+            )
+        })?;
 
         // Release the lock after writing to the file
-        flock(file.as_raw_fd(), FlockArg::Unlock).expect("Error releasing file lock");
+        flock(file.as_raw_fd(), FlockArg::Unlock)
+            .map_err(|_| IoError::new(ErrorKind::Other, "Error releasing file lock"))?;
+
         Ok(())
     }
 

crates/plugin-api/Cargo.toml

@@ -6,3 +6,4 @@ edition = "2021"
 
 [dependencies]
 axum.workspace = true
+thiserror.workspace = true

crates/plugin-api/src/lib.rs

@@ -2,12 +2,16 @@ use std::{
     fmt::Debug,
     hash::{Hash, Hasher},
 };
+use thiserror::Error;
 
 use axum::Router;
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, Error, PartialEq)]
 pub enum PluginError {
-    InitError,
+    #[error("{0}")]
+    InitError(String),
+    #[error("{0}")]
+    Other(String),
 }
 
 pub trait Plugin: Sync + Send {
@@ -21,7 +25,7 @@ pub trait Plugin: Sync + Send {
     fn unmount(&self) -> Result<(), PluginError>;
 
     /// Export managed endpoints
-    fn routes(&self) -> Router;
+    fn routes(&self) -> Result<Router, PluginError>;
 }
 
 impl Eq for dyn Plugin {}

crates/web-plugins/did-endpoint/src/plugin.rs

@@ -24,14 +24,11 @@ pub(crate) struct DidEndPointState {
 }
 
 fn get_env() -> Result<DidEndpointEnv, PluginError> {
-    let storage_dirpath = std::env::var("STORAGE_DIRPATH").map_err(|_| {
-        tracing::error!("STORAGE_DIRPATH env variable required");
-        PluginError::InitError
-    })?;
+    let storage_dirpath = std::env::var("STORAGE_DIRPATH")
+        .map_err(|_| PluginError::InitError("STORAGE_DIRPATH env variable required".to_owned()))?;
 
     let server_public_domain = std::env::var("SERVER_PUBLIC_DOMAIN").map_err(|_| {
-        tracing::error!("SERVER_PUBLIC_DOMAIN env variable required");
-        PluginError::InitError
+        PluginError::InitError("SERVER_PUBLIC_DOMAIN env variable required".to_owned())
     })?;
 
     Ok(DidEndpointEnv {
@@ -62,8 +59,9 @@ impl Plugin for DidEndpoint {
                 &mut filesystem,
             )
             .map_err(|_| {
-                tracing::error!("failed to generate an initial keystore and its DID document");
-                PluginError::InitError
+                PluginError::InitError(
+                    "failed to generate an initial keystore and its DID document".to_owned(),
+                )
             })?;
         };
 
@@ -80,8 +78,10 @@ impl Plugin for DidEndpoint {
         Ok(())
     }
 
-    fn routes(&self) -> Router {
-        let state = self.state.as_ref().expect("Plugin not mounted");
-        web::routes(Arc::new(state.clone()))
+    fn routes(&self) -> Result<Router, PluginError> {
+        let state = self.state.as_ref().ok_or(PluginError::Other(
+            "missing state, plugin not mounted".to_owned(),
+        ))?;
+        Ok(web::routes(Arc::new(state.clone())))
     }
 }

crates/web-plugins/did-endpoint/src/web.rs

@@ -36,7 +36,10 @@ async fn diddoc(State(state): State<Arc<DidEndPointState>>) -> Result<Json<Value
     let did_path = Path::new(&storage_dirpath).join("did.json");
 
     match filesystem.read_to_string(&did_path).as_ref() {
-        Ok(content) => Ok(Json(serde_json::from_str(&content).unwrap())),
+        Ok(content) => Ok(Json(serde_json::from_str(&content).map_err(|_| {
+            tracing::error!("Unparseable did.json");
+            StatusCode::NOT_FOUND
+        })?)),
         Err(_) => Err(StatusCode::NOT_FOUND),
     }
 }

crates/web-plugins/didcomm-messaging/Cargo.toml

@@ -14,6 +14,7 @@ filesystem.workspace = true
 forward.workspace = true
 pickup.workspace = true
 trust-ping.workspace = true
+discover-features.workspace = true
 mediator-coordination.workspace = true
 
 mongodb.workspace = true

crates/web-plugins/didcomm-messaging/protocols/discover-features/src/errors.rs

@@ -8,7 +8,7 @@ pub enum DiscoveryError {
     #[error("No queries field in body")]
     QueryNotFound,
     #[error("query feature-type not supported try using `protocol`")]
-    FeatureNOTSupported
+    FeatureNOTSupported,
 }
 
 impl IntoResponse for DiscoveryError {

crates/web-plugins/didcomm-messaging/protocols/discover-features/src/handler.rs

@@ -1,22 +1,19 @@
-use std::{collections::HashSet, sync::Arc};
-
-use axum::response::{IntoResponse, Response};
+use crate::{
+    errors::DiscoveryError,
+    model::{Disclosures, DisclosuresContent},
+};
 use didcomm::Message;
 use serde_json::json;
 use shared::{constants::DISCOVER_FEATURE, state::AppState};
+use std::{collections::HashSet, sync::Arc};
 use uuid::Uuid;
 
-use super::{
-    errors::DiscoveryError,
-    model::{Disclosures, DisclosuresContent},
-};
-
 // handle discover feature request
 // https://didcomm.org/discover-features/2.0/
-pub fn handle_query_request(
-    message: Message,
+pub async fn handle_query_request(
     state: Arc<AppState>,
-) -> Result<Option<Message>, Response> {
+    message: Message,
+) -> Result<Option<Message>, DiscoveryError> {
     let mut disclosed_protocols: HashSet<String> = HashSet::new();
     let supported: &Vec<String>;
 
@@ -76,27 +73,28 @@ pub fn handle_query_request(
                                     }
                                 }
 
-                                None => return Err(DiscoveryError::MalformedBody.into_response()),
+                                None => return Err(DiscoveryError::MalformedBody),
                             }
                         } else {
-                            return Err(DiscoveryError::FeatureNOTSupported.into_response());
+                            return Err(DiscoveryError::FeatureNOTSupported);
                         }
                     }
-                    None => return Err(DiscoveryError::MalformedBody.into_response()),
+                    None => return Err(DiscoveryError::MalformedBody),
                 }
             }
 
             // build response body
             let msg = build_response(disclosed_protocols);
             Ok(Some(msg))
         } else {
-            return Err(DiscoveryError::QueryNotFound.into_response());
+            return Err(DiscoveryError::QueryNotFound);
         }
     } else {
         let msg = build_response(disclosed_protocols);
         Ok(Some(msg))
     }
 }
+
 fn build_response(disclosed_protocols: HashSet<String>) -> Message {
     let mut body = Disclosures::new();
     for id in disclosed_protocols.iter() {
@@ -115,6 +113,7 @@ fn build_response(disclosed_protocols: HashSet<String>) -> Message {
 
     msg
 }
+
 #[cfg(test)]
 mod test {
 
@@ -147,14 +146,17 @@ mod test {
             keystore: Arc::new(MockKeyStore::new(vec![])),
         };
 
-        let state = Arc::new(AppState::from(
-            public_domain,
-            diddoc,
-            Some(vec![
-                "https://didcomm.org/coordinate-mediation/2.0/mediate-request".to_string(),
-            ]),
-            Some(repository),
-        ));
+        let state = Arc::new(
+            AppState::from(
+                public_domain,
+                diddoc,
+                Some(vec![
+                    "https://didcomm.org/coordinate-mediation/2.0/mediate-request".to_string(),
+                ]),
+                Some(repository),
+            )
+            .unwrap(),
+        );
 
         state
     }
@@ -170,7 +172,7 @@ mod test {
 
         let message = Message::build(id, QUERY_FEATURE.to_string(), json!(body)).finalize();
         let state = setup();
-        match handle_query_request(message, state) {
+        match handle_query_request(state, message).await {
             Ok(result) => {
                 assert!(result.clone().unwrap().body.get("disclosures").is_some());
                 assert!(result
@@ -227,7 +229,7 @@ mod test {
 
         let message = Message::build(id, QUERY_FEATURE.to_string(), json!(body)).finalize();
         let state = setup();
-        match handle_query_request(message, state) {
+        match handle_query_request(state, message).await {
             Ok(result) => {
                 println!("{:#?}", result.clone().unwrap());
                 assert!(result.clone().unwrap().body.get("disclosures").is_some());
@@ -284,7 +286,7 @@ mod test {
 
         let message = Message::build(id, QUERY_FEATURE.to_string(), json!(body)).finalize();
 
-        match handle_query_request(message, state) {
+        match handle_query_request(state, message).await {
             Ok(_) => {
                 panic!("This should'nt occur");
             }

crates/web-plugins/didcomm-messaging/protocols/discover-features/src/lib.rs

@@ -1,3 +1,4 @@
 mod errors;
-pub mod handler;
 mod model;
+
+pub mod handler;

crates/web-plugins/didcomm-messaging/protocols/discover-features/src/model.rs

@@ -12,7 +12,9 @@ pub struct Disclosures {
 }
 impl Disclosures {
     pub fn new() -> Self {
-        Disclosures { disclosures: vec![] }
+        Disclosures {
+            disclosures: vec![],
+        }
     }
 }
 #[derive(Deserialize, Serialize)]

crates/web-plugins/didcomm-messaging/protocols/forward/src/web/routing.rs → crates/web-plugins/didcomm-messaging/protocols/forward/src/handler.rs

@@ -1,4 +1,4 @@
-use crate::error::ForwardError;
+use crate::ForwardError;
 use database::Repository;
 use didcomm::{AttachmentData, Message};
 use mongodb::bson::doc;
@@ -9,30 +9,9 @@ use shared::{
 };
 use std::sync::Arc;
 
-async fn checks(
-    message: &Message,
-    connection_repository: &Arc<dyn Repository<Connection>>,
-) -> Result<String, ForwardError> {
-    let next = message.body.get("next").and_then(Value::as_str);
-    match next {
-        Some(next) => next,
-        None => return Err(ForwardError::MalformedBody),
-    };
-
-    // Check if the client's did in mediator's keylist
-    let _connection = match connection_repository
-        .find_one_by(doc! {"keylist": doc!{ "$elemMatch": { "$eq": &next}}})
-        .await
-        .map_err(|_| ForwardError::InternalServerError)?
-    {
-        Some(connection) => connection,
-        None => return Err(ForwardError::UncoordinatedSender),
-    };
-
-    Ok(next.unwrap().to_string())
-}
-
-pub(crate) async fn handler(
+/// Mediator receives forwarded messages, extract the next field in the message body, and the attachments in the message
+/// then stores the attachment with the next field as key for pickup
+pub async fn mediator_forward_process(
     state: Arc<AppState>,
     message: Message,
 ) -> Result<Option<Message>, ForwardError> {
@@ -69,10 +48,31 @@ pub(crate) async fn handler(
     Ok(None)
 }
 
+async fn checks(
+    message: &Message,
+    connection_repository: &Arc<dyn Repository<Connection>>,
+) -> Result<String, ForwardError> {
+    let next = message.body.get("next").and_then(Value::as_str);
+    match next {
+        Some(next) => next,
+        None => return Err(ForwardError::MalformedBody),
+    };
+
+    // Check if the client's did in mediator's keylist
+    let _connection = match connection_repository
+        .find_one_by(doc! {"keylist": doc!{ "$elemMatch": { "$eq": &next}}})
+        .await
+        .map_err(|_| ForwardError::InternalServerError)?
+    {
+        Some(connection) => connection,
+        None => return Err(ForwardError::UncoordinatedSender),
+    };
+
+    Ok(next.unwrap().to_string())
+}
+
 #[cfg(test)]
 mod test {
-    use crate::web::handler::mediator_forward_process;
-
     use super::*;
     use did_utils::jwk::Jwk;
     use didcomm::{

crates/web-plugins/didcomm-messaging/protocols/forward/src/lib.rs

@@ -1,5 +1,5 @@
 mod error;
-pub mod web;
+pub mod handler;
 
 // Re-exports
 pub use error::ForwardError;