Fix mutex poisoning and optimize metrics with dashmap, add derived traits to McpError

Co-authored-by: 0xrinegade <[email protected]>

Author: Copilot

Cargo.toml

@@ -17,6 +17,7 @@ tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["json", "env-filter"] }
 uuid = { version = "1.0", features = ["v4"] }
 once_cell = "1.19"
+dashmap = "6.1"
 solana-client = "1.17"
 solana-sdk = "1.17"
 solana-account-decoder = "1.17"

src/error.rs

@@ -7,7 +7,7 @@ use uuid::Uuid;
 /// This module defines a hierarchy of error types that provide
 /// rich context for debugging and monitoring while maintaining
 /// security by avoiding sensitive data exposure.
-#[derive(Error, Debug)]
+#[derive(Error, Debug, Clone, PartialEq)]
 pub enum McpError {
     /// Client-side errors (invalid input, malformed requests)
     #[error("Client error: {message}")]
@@ -23,7 +23,7 @@ pub enum McpError {
         message: String,
         request_id: Option<Uuid>,
         method: Option<String>,
-        source: Option<Box<dyn std::error::Error + Send + Sync>>,
+        source_message: Option<String>, // Store source error as string for Clone/PartialEq
     },
 
     /// RPC-specific errors (Solana client failures)
@@ -33,7 +33,7 @@ pub enum McpError {
         request_id: Option<Uuid>,
         method: Option<String>,
         rpc_url: Option<String>,
-        source: Option<Box<dyn std::error::Error + Send + Sync>>,
+        source_message: Option<String>, // Store source error as string for Clone/PartialEq
     },
 
     /// Validation errors (invalid parameters, security checks)
@@ -79,7 +79,7 @@ impl McpError {
             message: message.into(),
             request_id: None,
             method: None,
-            source: None,
+            source_message: None,
         }
     }
 
@@ -90,7 +90,7 @@ impl McpError {
             request_id: None,
             method: None,
             rpc_url: None,
-            source: None,
+            source_message: None,
         }
     }
 
@@ -176,9 +176,10 @@ impl McpError {
 
     /// Adds source error context
     pub fn with_source(mut self, source: Box<dyn std::error::Error + Send + Sync>) -> Self {
+        let source_message = source.to_string();
         match &mut self {
-            McpError::Server { source: ref mut s, .. } => *s = Some(source),
-            McpError::Rpc { source: ref mut s, .. } => *s = Some(source),
+            McpError::Server { source_message: ref mut s, .. } => *s = Some(source_message),
+            McpError::Rpc { source_message: ref mut s, .. } => *s = Some(source_message),
             _ => {}, // Other error types don't have source fields
         }
         self
@@ -253,19 +254,27 @@ impl McpError {
                     log_data.insert("parameter".to_string(), Value::String(param.clone()));
                 }
             },
-            McpError::Rpc { rpc_url, .. } => {
+            McpError::Rpc { rpc_url, source_message, .. } => {
                 if let Some(url) = rpc_url {
                     // Sanitize URL for logging
                     let sanitized = crate::validation::sanitize_for_logging(url);
                     log_data.insert("rpc_url".to_string(), Value::String(sanitized));
                 }
+                if let Some(source_msg) = source_message {
+                    log_data.insert("source_error".to_string(), Value::String(source_msg.clone()));
+                }
             },
             McpError::Network { endpoint, .. } => {
                 if let Some(ep) = endpoint {
                     let sanitized = crate::validation::sanitize_for_logging(ep);
                     log_data.insert("endpoint".to_string(), Value::String(sanitized));
                 }
             },
+            McpError::Server { source_message, .. } => {
+                if let Some(source_msg) = source_message {
+                    log_data.insert("source_error".to_string(), Value::String(source_msg.clone()));
+                }
+            },
             _ => {}
         }
 
@@ -352,4 +361,26 @@ mod tests {
         assert!(log_value.get("method").is_some());
         assert!(log_value.get("rpc_url").is_some());
     }
+
+    #[test]
+    fn test_derived_traits() {
+        let request_id = Uuid::new_v4();
+        let error1 = McpError::validation("Invalid pubkey format")
+            .with_request_id(request_id)
+            .with_method("getBalance")
+            .with_parameter("pubkey");
+
+        // Test Clone
+        let error2 = error1.clone();
+        assert_eq!(error1.request_id(), error2.request_id());
+        assert_eq!(error1.method(), error2.method());
+        assert_eq!(error1.error_type(), error2.error_type());
+
+        // Test PartialEq
+        assert_eq!(error1, error2);
+
+        // Test that different errors are not equal
+        let error3 = McpError::client("Different error");
+        assert_ne!(error1, error3);
+    }
 }

src/logging.rs

@@ -8,8 +8,7 @@ use tracing_subscriber::{
 use uuid::Uuid;
 use serde_json::Value;
 use std::sync::atomic::{AtomicU64, Ordering};
-use std::collections::HashMap;
-use std::sync::Mutex;
+use dashmap::DashMap;
 
 /// Metrics collection for monitoring RPC call outcomes
 #[derive(Debug, Default)]
@@ -19,9 +18,9 @@ pub struct Metrics {
     /// Number of successful RPC calls
     pub successful_calls: AtomicU64,
     /// Number of failed RPC calls by error type
-    pub failed_calls_by_type: Mutex<HashMap<String, u64>>,
+    pub failed_calls_by_type: DashMap<String, u64>,
     /// Number of failed RPC calls by method
-    pub failed_calls_by_method: Mutex<HashMap<String, u64>>,
+    pub failed_calls_by_method: DashMap<String, u64>,
 }
 
 impl Metrics {
@@ -37,21 +36,27 @@ impl Metrics {
 
     /// Increment failed calls counter by error type
     pub fn increment_failed_calls(&self, error_type: &str, method: Option<&str>) {
-        // Increment by error type
-        let mut failed_by_type = self.failed_calls_by_type.lock().unwrap();
-        *failed_by_type.entry(error_type.to_string()).or_insert(0) += 1;
+        // Increment by error type using dashmap for concurrent access
+        *self.failed_calls_by_type.entry(error_type.to_string()).or_insert(0) += 1;
 
         // Increment by method if available
         if let Some(method) = method {
-            let mut failed_by_method = self.failed_calls_by_method.lock().unwrap();
-            *failed_by_method.entry(method.to_string()).or_insert(0) += 1;
+            *self.failed_calls_by_method.entry(method.to_string()).or_insert(0) += 1;
         }
     }
 
     /// Get current metrics as JSON value
     pub fn to_json(&self) -> Value {
-        let failed_by_type = self.failed_calls_by_type.lock().unwrap().clone();
-        let failed_by_method = self.failed_calls_by_method.lock().unwrap().clone();
+        // Convert DashMap to HashMap for JSON serialization
+        let failed_by_type: std::collections::HashMap<String, u64> = self.failed_calls_by_type
+            .iter()
+            .map(|entry| (entry.key().clone(), *entry.value()))
+            .collect();
+            
+        let failed_by_method: std::collections::HashMap<String, u64> = self.failed_calls_by_method
+            .iter()
+            .map(|entry| (entry.key().clone(), *entry.value()))
+            .collect();
 
         serde_json::json!({
             "total_calls": self.total_calls.load(Ordering::Relaxed),
@@ -65,8 +70,8 @@ impl Metrics {
     pub fn reset(&self) {
         self.total_calls.store(0, Ordering::Relaxed);
         self.successful_calls.store(0, Ordering::Relaxed);
-        self.failed_calls_by_type.lock().unwrap().clear();
-        self.failed_calls_by_method.lock().unwrap().clear();
+        self.failed_calls_by_type.clear();
+        self.failed_calls_by_method.clear();
     }
 }
 
@@ -378,11 +383,9 @@ mod tests {
         assert_eq!(metrics.total_calls.load(Ordering::Relaxed), 1);
         assert_eq!(metrics.successful_calls.load(Ordering::Relaxed), 1);
 
-        let failed_by_type = metrics.failed_calls_by_type.lock().unwrap();
-        assert_eq!(failed_by_type.get("validation"), Some(&1));
-        
-        let failed_by_method = metrics.failed_calls_by_method.lock().unwrap();
-        assert_eq!(failed_by_method.get("getBalance"), Some(&1));
+        // Test dashmap access
+        assert_eq!(metrics.failed_calls_by_type.get("validation").map(|v| *v), Some(1));
+        assert_eq!(metrics.failed_calls_by_method.get("getBalance").map(|v| *v), Some(1));
     }
 
     #[test]