Skip to content

Commit e92050a

Browse files
committed
Add Cache account lookup error
- Add `AccountLookupError` and `Cache::try_account`. - Cover missing and cached account lookups in cache tests. - Use typed account lookups in required fill and portfolio paths.
1 parent b9856f3 commit e92050a

5 files changed

Lines changed: 98 additions & 36 deletions

File tree

crates/common/src/cache/error.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
// limitations under the License.
1414
// -------------------------------------------------------------------------------------------------
1515

16-
use nautilus_model::identifiers::{ClientOrderId, InstrumentId, PositionId};
16+
use nautilus_model::identifiers::{AccountId, ClientOrderId, InstrumentId, PositionId};
1717
use thiserror::Error;
1818
use ustr::Ustr;
1919

20+
/// Message used for a missing account lookup.
21+
pub const ACCOUNT_NOT_FOUND: &str = "account not found in cache";
22+
2023
/// Message used for a missing currency lookup.
2124
pub const CURRENCY_NOT_FOUND: &str = "currency not found in cache";
2225

@@ -29,6 +32,25 @@ pub const ORDER_NOT_FOUND: &str = "order not found in cache";
2932
/// Message used for a missing position lookup.
3033
pub const POSITION_NOT_FOUND: &str = "position not found in cache";
3134

35+
/// Error returned when an account cannot be resolved from a cache or store.
36+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)]
37+
pub enum AccountLookupError {
38+
/// The requested account is not present.
39+
#[error("{message}: {account_id}", message = ACCOUNT_NOT_FOUND)]
40+
NotFound {
41+
/// The account identifier that was requested.
42+
account_id: AccountId,
43+
},
44+
}
45+
46+
impl AccountLookupError {
47+
/// Returns a not-found error for `account_id`.
48+
#[must_use]
49+
pub const fn not_found(account_id: AccountId) -> Self {
50+
Self::NotFound { account_id }
51+
}
52+
}
53+
3254
/// Error returned when a currency cannot be resolved from a cache or store.
3355
#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)]
3456
pub enum CurrencyLookupError {

crates/common/src/cache/mod.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ use bytes::Bytes;
4545
pub use config::CacheConfig; // Re-export
4646
use database::{CacheDatabaseAdapter, CacheMap};
4747
pub use error::{
48-
CURRENCY_NOT_FOUND, CurrencyLookupError, INSTRUMENT_NOT_FOUND, InstrumentLookupError,
49-
ORDER_NOT_FOUND, OrderLookupError, POSITION_NOT_FOUND, PositionLookupError,
48+
ACCOUNT_NOT_FOUND, AccountLookupError, CURRENCY_NOT_FOUND, CurrencyLookupError,
49+
INSTRUMENT_NOT_FOUND, InstrumentLookupError, ORDER_NOT_FOUND, OrderLookupError,
50+
POSITION_NOT_FOUND, PositionLookupError,
5051
};
5152
use index::CacheIndex;
5253
use nautilus_core::{
@@ -5445,6 +5446,21 @@ impl Cache {
54455446
.map(|account_cell| AccountRef::new(account_cell.borrow()))
54465447
}
54475448

5449+
/// Returns a borrow of the account for the `account_id`.
5450+
///
5451+
/// # Errors
5452+
///
5453+
/// Returns [`AccountLookupError::NotFound`] when the account is not present in the cache.
5454+
pub fn try_account(
5455+
&self,
5456+
account_id: &AccountId,
5457+
) -> Result<AccountRef<'_>, AccountLookupError> {
5458+
self.accounts
5459+
.get(account_id)
5460+
.map(|account_cell| AccountRef::new(account_cell.borrow()))
5461+
.ok_or_else(|| AccountLookupError::not_found(*account_id))
5462+
}
5463+
54485464
/// Gets an exclusive write borrow of the account with the `account_id` (if found).
54495465
///
54505466
/// Requires `&mut Cache` so cache writes are reachable only by privileged crates that hold

crates/common/src/cache/tests.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ use ustr::Ustr;
6969

7070
use crate::{
7171
cache::{
72-
CURRENCY_NOT_FOUND, Cache, CacheConfig, CacheView, CurrencyLookupError,
73-
INSTRUMENT_NOT_FOUND, InstrumentLookupError, ORDER_NOT_FOUND, OrderLookupError, OrderRef,
74-
POSITION_NOT_FOUND, PositionLookupError,
72+
ACCOUNT_NOT_FOUND, AccountLookupError, CURRENCY_NOT_FOUND, Cache, CacheConfig, CacheView,
73+
CurrencyLookupError, INSTRUMENT_NOT_FOUND, InstrumentLookupError, ORDER_NOT_FOUND,
74+
OrderLookupError, OrderRef, POSITION_NOT_FOUND, PositionLookupError,
7575
database::{CacheDatabaseAdapter, CacheMap},
7676
},
7777
signal::Signal,
@@ -2903,6 +2903,30 @@ fn test_cache_add_account(mut cache: Cache) {
29032903
assert_eq!(*result.unwrap(), account);
29042904
}
29052905

2906+
#[rstest]
2907+
fn test_try_account_when_empty(cache: Cache) {
2908+
let account_id = AccountId::test_default();
2909+
2910+
let err = cache.try_account(&account_id).unwrap_err();
2911+
2912+
assert_eq!(err, AccountLookupError::not_found(account_id));
2913+
assert_eq!(
2914+
err.to_string(),
2915+
format!("{ACCOUNT_NOT_FOUND}: {account_id}")
2916+
);
2917+
}
2918+
2919+
#[rstest]
2920+
fn test_try_account_when_some(mut cache: Cache) {
2921+
let account = AccountAny::default();
2922+
let account_id = account.id();
2923+
cache.add_account(account.clone()).unwrap();
2924+
2925+
let result = cache.try_account(&account_id).unwrap();
2926+
2927+
assert_eq!(*result, account);
2928+
}
2929+
29062930
#[rstest]
29072931
fn test_cache_accounts_when_no_accounts_returns_empty(cache: Cache) {
29082932
let result = cache.accounts(&AccountId::test_default());

crates/execution/src/engine/mod.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2587,11 +2587,8 @@ impl ExecutionEngine {
25872587
return;
25882588
};
25892589

2590-
if self.cache.borrow().account(&fill.account_id).is_none() {
2591-
log::error!(
2592-
"Cannot handle leg fill: no account found for {}, {fill}",
2593-
fill.instrument_id.venue,
2594-
);
2590+
if let Err(e) = self.cache.borrow().try_account(&fill.account_id) {
2591+
log::error!("Cannot handle leg fill: {e}, {fill}");
25952592
return;
25962593
}
25972594

@@ -3096,12 +3093,12 @@ impl ExecutionEngine {
30963093

30973094
let is_margin_account = {
30983095
let cache = self.cache.borrow();
3099-
let Some(account) = cache.account(&fill.account_id) else {
3100-
log::error!(
3101-
"Cannot handle order fill: no account found for {}, {fill}",
3102-
fill.instrument_id.venue,
3103-
);
3104-
return Vec::new();
3096+
let account = match cache.try_account(&fill.account_id) {
3097+
Ok(account) => account,
3098+
Err(e) => {
3099+
log::error!("Cannot handle order fill: {e}, {fill}");
3100+
return Vec::new();
3101+
}
31053102
};
31063103

31073104
account.is_margin_account()

crates/portfolio/src/portfolio.rs

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use ahash::{AHashMap, AHashSet};
2121
use indexmap::{IndexMap, IndexSet};
2222
use nautilus_analysis::analyzer::PortfolioAnalyzer;
2323
use nautilus_common::{
24-
cache::Cache,
24+
cache::{AccountLookupError, Cache},
2525
clock::Clock,
2626
enums::LogColor,
2727
msgbus::{self, MessagingSwitchboard, TypedHandler, TypedIntoHandler},
@@ -1111,14 +1111,12 @@ impl Portfolio {
11111111

11121112
for position in &positions_open {
11131113
// Get account for THIS position
1114-
let account = if let Some(account) = cache.account(&position.account_id) {
1115-
account
1116-
} else {
1117-
log::error!(
1118-
"Cannot calculate net exposure: no account for {}",
1119-
position.account_id
1120-
);
1121-
return None;
1114+
let account = match cache.try_account(&position.account_id) {
1115+
Ok(account) => account,
1116+
Err(e) => {
1117+
log::error!("Cannot calculate net exposure: {e}");
1118+
return None;
1119+
}
11221120
};
11231121

11241122
// Validate consistent base currency across accounts
@@ -2247,11 +2245,12 @@ fn update_order(
22472245
let (instrument, orders_open) = {
22482246
let cache_ref = cache.borrow();
22492247

2250-
let account = if let Some(account) = cache_ref.account(&account_id) {
2251-
account
2252-
} else {
2253-
log::error!("Cannot update order: no account registered for {account_id}");
2254-
return;
2248+
let account = match cache_ref.try_account(&account_id) {
2249+
Ok(account) => account,
2250+
Err(e) => {
2251+
log::error!("Cannot update order: {e}");
2252+
return;
2253+
}
22552254
};
22562255

22572256
match &*account {
@@ -2325,11 +2324,15 @@ fn update_order(
23252324
};
23262325

23272326
// No cache borrow held: AccountsManager borrows cache internally for xrate lookups.
2328-
let mut working_account = if let Some(account) = cache.borrow_mut().take_account(&account_id) {
2329-
account
2330-
} else {
2331-
log::error!("Cannot update order: no account registered for {account_id}");
2332-
return;
2327+
let mut working_account = match cache.borrow_mut().take_account(&account_id) {
2328+
Some(account) => account,
2329+
None => {
2330+
log::error!(
2331+
"Cannot update order: {}",
2332+
AccountLookupError::not_found(account_id)
2333+
);
2334+
return;
2335+
}
23332336
};
23342337

23352338
if let OrderEventAny::Filled(order_filled) = event {

0 commit comments

Comments
 (0)