Skip to content

Commit a49bce5

Browse files
committed
address review: refine error variants for accurate semantics
PR #369 review feedback from Copilot. Replace misleading variant reuse with domain-accurate variants so error messages match the actual failure mode. - PricingError::SqrtFailure.value: Positive -> Decimal. The sqrt is taken of a discriminant that can be negative (doc said negative but the field rejected it). - ChainError: add EmptyChain { symbol } variant for non-ATM lookups. Reserve EmptyChainAtm for atm_option_data() paths. Update get_optiondata_with_strike and its test to use EmptyChain. - OhlcvError: add MissingZipEntry { extension } for missing file in ZIP archive, and AsyncTask { reason } for spawn_blocking join failures. read_ohlcv_from_zip and read_ohlcv_from_zip_async now map to the correct variants instead of MissingColumn/RowParsing. - VolatilityError: add Chain(Box<ChainError>) variant plus From<ChainError>. AtmIvProvider for OptionChain now wraps the source as VolatilityError::Chain instead of NumericalFailure, preserving the actual cause (empty chain / ATM not found). - examples/examples_chain/creator.rs: map missing expiration to ChainError::invalid_parameters("expiration_date", "missing") instead of AtmNotFound.
1 parent 85d33ce commit a49bce5

8 files changed

Lines changed: 65 additions & 20 deletions

File tree

examples/examples_chain/src/bin/creator.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ fn main() -> Result<(), optionstratlib::error::Error> {
1919
info!("{}", option_chain);
2020

2121
let underlying_price = option_chain.underlying_price;
22-
let expiration_date =
23-
option_chain
24-
.get_expiration()
25-
.ok_or(optionstratlib::error::ChainError::AtmNotFound {
26-
symbol: option_chain.symbol.clone(),
27-
})?;
22+
let expiration_date = option_chain.get_expiration().ok_or_else(|| {
23+
optionstratlib::error::ChainError::invalid_parameters(
24+
"expiration_date",
25+
"missing on option chain",
26+
)
27+
})?;
2828
let symbol = option_chain.symbol.clone();
2929

3030
info!("Underlying Price: {}", underlying_price);

src/chains/chain.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2815,7 +2815,7 @@ impl OptionChain {
28152815
pub fn get_optiondata_with_strike(&self, price: &Positive) -> Result<&OptionData, ChainError> {
28162816
// Check for empty option chain
28172817
if self.options.is_empty() {
2818-
return Err(ChainError::EmptyChainAtm {
2818+
return Err(ChainError::EmptyChain {
28192819
symbol: self.symbol.clone(),
28202820
});
28212821
}
@@ -11951,10 +11951,14 @@ mod tests_get_strikes_and_optiondata {
1195111951
assert!(result.is_err(), "Should return error for empty chain");
1195211952

1195311953
let error = result.unwrap_err();
11954+
assert!(
11955+
matches!(error, ChainError::EmptyChain { ref symbol } if symbol == "EMPTY"),
11956+
"Should return ChainError::EmptyChain for EMPTY symbol, got: {error:?}"
11957+
);
1195411958
let error_msg = format!("{error}");
1195511959
assert!(
11956-
error_msg.contains("empty option chain"),
11957-
"Error should mention empty chain"
11960+
error_msg.contains("option chain is empty"),
11961+
"Error should mention empty chain, got: {error_msg}"
1195811962
);
1195911963
assert!(
1196011964
error_msg.contains("EMPTY"),

src/error/chains.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ use thiserror::Error;
8585
/// * `StrategyError` - Errors related to option trading strategies, including issues
8686
/// with leg validation or invalid combinations of options.
8787
///
88+
/// * `EmptyChain` - The option chain for the given symbol is empty (generic empty-chain
89+
/// condition, independent of ATM selection).
8890
/// * `EmptyChainAtm` - The option chain for the given symbol is empty and has no ATM option
89-
/// to select.
91+
/// to select. Reserved for ATM-selection paths; prefer `EmptyChain` for non-ATM lookups.
9092
/// * `AtmNotFound` - The option chain has options but none could be selected as the ATM
9193
/// contract for the given symbol.
9294
/// * `EmptyDensities` - No valid risk-neutral densities could be produced.
@@ -133,8 +135,16 @@ pub enum ChainError {
133135
#[error("Strategy error: {0}")]
134136
StrategyError(StrategyErrorKind),
135137

138+
/// The option chain for the given symbol is empty. Generic empty-chain
139+
/// condition, emitted by non-ATM lookups (e.g. strike-based selection).
140+
#[error("option chain is empty: {symbol}")]
141+
EmptyChain {
142+
/// Ticker symbol of the chain that is empty.
143+
symbol: String,
144+
},
145+
136146
/// The option chain for the given symbol is empty and has no ATM option
137-
/// to select.
147+
/// to select. Reserved for ATM-selection paths.
138148
#[error("cannot find ATM option for empty option chain: {symbol}")]
139149
EmptyChainAtm {
140150
/// Ticker symbol of the chain that is empty.

src/error/csv.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,21 @@ pub enum OhlcvError {
112112
/// Human-readable description of why the row could not be parsed.
113113
reason: String,
114114
},
115+
116+
/// No file with the expected extension was found inside the ZIP archive.
117+
#[error("no file with extension `{extension}` found in ZIP archive")]
118+
MissingZipEntry {
119+
/// Extension that was expected inside the ZIP payload (e.g. `"csv"`).
120+
extension: &'static str,
121+
},
122+
123+
/// An asynchronous background task (`tokio::task::spawn_blocking`) failed to
124+
/// complete — typically a panic or cancellation of the worker thread.
125+
#[error("async task failed: {reason}")]
126+
AsyncTask {
127+
/// Human-readable description of the underlying join failure.
128+
reason: String,
129+
},
115130
}
116131

117132
impl From<std::io::Error> for OhlcvError {

src/error/pricing.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::error::{DecimalError, GreeksError, OptionsError, PositionError};
22
use expiration_date::error::ExpirationDateError;
3-
use positive::{Positive, PositiveError};
3+
use positive::PositiveError;
4+
use rust_decimal::Decimal;
45
use thiserror::Error;
56

67
/// Error type for option pricing operations.
@@ -69,11 +70,14 @@ pub enum PricingError {
6970
},
7071

7172
/// A square-root computation inside a pricing kernel failed (the operand
72-
/// was negative or not representable).
73+
/// was negative or not representable — e.g. a negative discriminant in a
74+
/// closed-form solver).
7375
#[error("pricing sqrt failed for value {value}")]
7476
SqrtFailure {
7577
/// The value for which the square root could not be computed.
76-
value: Positive,
78+
///
79+
/// Stored as `Decimal` so negative operands are representable.
80+
value: Decimal,
7781
},
7882

7983
/// Error from Positive operations.

src/error/volatility.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,25 @@ pub enum VolatilityError {
115115
source: Box<VolatilityError>,
116116
},
117117

118+
/// A chain-layer error surfaced while retrieving volatility data
119+
/// (e.g. empty option chain, ATM lookup failure on a chain).
120+
///
121+
/// Boxed to avoid infinite enum size through the `ChainError::Volatility`
122+
/// cycle.
123+
#[error(transparent)]
124+
Chain(Box<crate::error::ChainError>),
125+
118126
/// Positive value errors
119127
#[error(transparent)]
120128
PositiveError(#[from] positive::PositiveError),
121129
}
122130

131+
impl From<crate::error::ChainError> for VolatilityError {
132+
fn from(error: crate::error::ChainError) -> Self {
133+
Self::Chain(Box::new(error))
134+
}
135+
}
136+
123137
#[cfg(test)]
124138
mod tests_volatility_errors {
125139
use super::*;

src/utils/csv.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub fn read_ohlcv_from_zip(
8787
}
8888
}
8989

90-
let csv_index = csv_index.ok_or(OhlcvError::MissingColumn { column: "csv" })?;
90+
let csv_index = csv_index.ok_or(OhlcvError::MissingZipEntry { extension: "csv" })?;
9191
let file = archive.by_index(csv_index)?;
9292
let reader = BufReader::new(file);
9393

@@ -159,8 +159,8 @@ pub async fn read_ohlcv_from_zip_async(
159159
read_ohlcv_from_zip(&zip_path, start_date.as_deref(), end_date.as_deref())
160160
})
161161
.await
162-
.map_err(|e| OhlcvError::RowParsing {
163-
reason: format!("async task error: {e}"),
162+
.map_err(|e| OhlcvError::AsyncTask {
163+
reason: e.to_string(),
164164
})?
165165
}
166166

src/volatility/traits.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,7 @@ impl AtmIvProvider for OptionChain {
110110
match self.get_atm_implied_volatility() {
111111
Ok(iv) => Ok(iv),
112112
Err(e) => Err(VolatilityError::AtmIvUnavailable {
113-
source: Box::new(VolatilityError::NumericalFailure {
114-
reason: e.to_string(),
115-
}),
113+
source: Box::new(VolatilityError::from(e)),
116114
}),
117115
}
118116
}

0 commit comments

Comments
 (0)