feat: replace anyhow with proper error variants in message stores (#279)

* Move message verification errors into their own module

* Move SessionError into the session module

* Introduce StoreError for message store related errors

* Remove redb message store as the append-only file one is superior

* Clean up file store error handling

* Robustness improvements for the MongoDB store

* Add dedicated mongodb tests

* Add test suite dedicated to file-based storage

* Remove duplication in store test suites

* Trim store test suites to focus on meaningful cases

* Remove step to run tests in CI job which is already covered by the coverage job

Author: davidsteiner

.github/workflows/ci.yml

@@ -21,8 +21,6 @@ jobs:
         run: cargo clippy -- -Dclippy::all -D warnings
       - name: Run clippy (all targets and features)
         run: cargo clippy --all-targets --all-features -- -Dclippy::all -D warnings
-      - name: Run tests
-        run: cargo test --verbose
 
   coverage:
     runs-on: ubuntu-latest

Cargo.lock

@@ -41,7 +41,6 @@ proc-macro2 = "1"
 quickcheck = "1"
 quickcheck_macros = "1"
 quote = "1"
-redb = "3.1"
 reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }
 roxmltree = "0.21"
 rust-embed = "8.7"

Cargo.toml

@@ -14,7 +14,6 @@ categories.workspace = true
 [features]
 default = ["test-utils"]
 fix44 = ["hotfix-message/fix44"]
-redb = ["dep:redb"]
 mongodb = ["dep:mongodb"]
 test-utils = []
 
@@ -31,7 +30,6 @@ chrono-tz = { workspace = true, features = ["serde"] }
 futures = { workspace = true }
 mongodb = { workspace = true, optional = true }
 rustls-pki-types = { workspace = true }
-redb = { workspace = true, optional = true }
 rustls = { workspace = true }
 rustls-native-certs = { workspace = true }
 rustls-pemfile = { workspace = true }

crates/hotfix/Cargo.toml

@@ -27,7 +27,6 @@
 
 pub mod application;
 pub mod config;
-pub(crate) mod error;
 pub mod initiator;
 pub mod message;
 pub mod message_utils;

crates/hotfix/src/lib.rs

@@ -14,6 +14,7 @@ pub mod resend_request;
 pub mod sequence_reset;
 pub mod test_request;
 pub mod verification;
+pub mod verification_error;
 
 pub use parser::RawFixMessage;
 pub use resend_request::ResendRequest;

crates/hotfix/src/message.rs

@@ -1,5 +1,5 @@
 use crate::config::SessionConfig;
-use crate::error::{CompIdType, MessageVerificationError};
+use crate::message::verification_error::{CompIdType, MessageVerificationError};
 use hotfix_message::Part;
 use hotfix_message::field_types::Timestamp;
 use hotfix_message::message::Message;
@@ -180,8 +180,9 @@ fn check_target_comp_id(
 
 #[cfg(test)]
 mod tests {
-    use super::{Message, MessageVerificationError, SessionConfig, verify_message};
-    use crate::error::CompIdType;
+    use super::{Message, SessionConfig, verify_message};
+    use crate::message::verification_error::CompIdType;
+    use crate::message::verification_error::MessageVerificationError;
     use hotfix_message::field_types::Timestamp;
     use hotfix_message::{Part, fix44};
 

crates/hotfix/src/message/verification.rs

@@ -53,9 +53,3 @@ pub enum CompIdType {
     Sender,
     Target,
 }
-
-#[derive(Debug, Error)]
-pub enum SessionError {
-    #[error("Schedule configuration is invalid: {0}")]
-    InvalidSchedule(String),
-}

crates/hotfix/src/error.rs …hotfix/src/message/verification_error.rscrates/hotfix/src/error.rs renamed to crates/hotfix/src/message/verification_error.rs crates/hotfix/src/error.rs renamed to crates/hotfix/src/message/verification_error.rs

@@ -1,4 +1,5 @@
 pub(crate) mod admin_request;
+pub mod error;
 pub(crate) mod event;
 mod info;
 mod session_handle;
@@ -26,15 +27,17 @@ use tracing::{debug, enabled, error, info, warn};
 
 use crate::Application;
 use crate::application::{InboundDecision, OutboundDecision};
-use crate::error::{CompIdType, MessageVerificationError};
 use crate::message::logout::Logout;
 use crate::message::reject::Reject;
 use crate::message::resend_request::ResendRequest;
 use crate::message::sequence_reset::SequenceReset;
 use crate::message::test_request::TestRequest;
 use crate::message::verification::verify_message;
+use crate::message::verification_error::{CompIdType, MessageVerificationError};
 use crate::message_utils::{is_admin, prepare_message_for_resend};
 use crate::session::admin_request::AdminRequest;
+pub use crate::session::info::{SessionInfo, Status};
+pub use crate::session::session_handle::SessionHandle;
 #[cfg(not(feature = "test-utils"))]
 pub(crate) use crate::session::session_ref::InternalSessionRef;
 #[cfg(feature = "test-utils")]
@@ -49,9 +52,6 @@ use hotfix_message::session_fields::{
     SessionRejectReason, TEST_REQ_ID,
 };
 
-pub use crate::session::info::{SessionInfo, Status};
-pub use crate::session::session_handle::SessionHandle;
-
 const SCHEDULE_CHECK_INTERVAL: u64 = 1;
 
 struct Session<A, I, O, S> {
@@ -365,7 +365,8 @@ where
             }
         }
 
-        self.store.increment_target_seq_number().await
+        self.store.increment_target_seq_number().await?;
+        Ok(())
     }
 
     async fn on_heartbeat(&mut self, message: &Message) -> Result<()> {
@@ -378,7 +379,8 @@ where
             self.reset_peer_timer(None);
         }
 
-        self.store.increment_target_seq_number().await
+        self.store.increment_target_seq_number().await?;
+        Ok(())
     }
 
     async fn on_test_request(&mut self, message: &Message) -> Result<()> {
@@ -515,7 +517,8 @@ where
             return Ok(());
         }
 
-        self.store.set_target_seq_number(end - 1).await
+        self.store.set_target_seq_number(end - 1).await?;
+        Ok(())
     }
 
     async fn handle_verification_error(&mut self, error: MessageVerificationError) -> Result<()> {
@@ -1129,6 +1132,7 @@ mod tests {
     use super::*;
     use crate::application::{InboundDecision, OutboundDecision};
     use crate::message::{InboundMessage, OutboundMessage};
+    use crate::store::{Result as StoreResult, StoreError};
     use chrono::{DateTime, Datelike, NaiveDate, NaiveTime, TimeDelta, Timelike};
     use chrono_tz::Tz;
     use hotfix_message::message::Message;
@@ -1168,11 +1172,11 @@ mod tests {
 
     #[async_trait::async_trait]
     impl MessageStore for TestStore {
-        async fn add(&mut self, _sequence_number: u64, _message: &[u8]) -> Result<()> {
+        async fn add(&mut self, _sequence_number: u64, _message: &[u8]) -> StoreResult<()> {
             Ok(())
         }
 
-        async fn get_slice(&self, _begin: usize, _end: usize) -> Result<Vec<Vec<u8>>> {
+        async fn get_slice(&self, _begin: usize, _end: usize) -> StoreResult<Vec<Vec<u8>>> {
             Ok(vec![])
         }
 
@@ -1184,25 +1188,25 @@ mod tests {
             self.target_seq
         }
 
-        async fn increment_sender_seq_number(&mut self) -> Result<()> {
+        async fn increment_sender_seq_number(&mut self) -> StoreResult<()> {
             self.sender_seq += 1;
             Ok(())
         }
 
-        async fn increment_target_seq_number(&mut self) -> Result<()> {
+        async fn increment_target_seq_number(&mut self) -> StoreResult<()> {
             self.target_seq += 1;
             Ok(())
         }
 
-        async fn set_target_seq_number(&mut self, seq_number: u64) -> Result<()> {
+        async fn set_target_seq_number(&mut self, seq_number: u64) -> StoreResult<()> {
             self.target_seq = seq_number;
             Ok(())
         }
 
-        async fn reset(&mut self) -> Result<()> {
+        async fn reset(&mut self) -> StoreResult<()> {
             self.reset_called.store(true, Ordering::SeqCst);
             if self.fail_reset.load(Ordering::SeqCst) {
-                bail!("simulated reset failure")
+                return Err(StoreError::Reset("simulated reset failure".into()));
             }
             self.creation_time = Utc::now();
             Ok(())

crates/hotfix/src/session.rs

@@ -0,0 +1,13 @@
+use crate::store::StoreError;
+use thiserror::Error;
+
+#[derive(Debug, Error)]
+pub enum SessionError {
+    #[error("Schedule configuration is invalid: {0}")]
+    InvalidSchedule(String),
+
+    #[error("store operation failed")]
+    Store(#[from] StoreError),
+}
+
+pub type Result<T> = std::result::Result<T, SessionError>;