diff --git a/diskann-benchmark-core/src/build/graph/multi.rs b/diskann-benchmark-core/src/build/graph/multi.rs index f5eed11bc..f47f00fb4 100644 --- a/diskann-benchmark-core/src/build/graph/multi.rs +++ b/diskann-benchmark-core/src/build/graph/multi.rs @@ -6,7 +6,7 @@ use std::{ops::Range, sync::Arc}; use diskann::{ - ANNError, ANNErrorKind, ANNResult, + ANNError, ANNResult, graph::{self, glue}, provider, }; @@ -95,14 +95,11 @@ where } } - ANNError::message( - ANNErrorKind::Opaque, - OutOfBounds { - max: self.data.nrows(), - start: range.start, - end: range.end, - }, - ) + ANNError::message(OutOfBounds { + max: self.data.nrows(), + start: range.start, + end: range.end, + }) })? .to_owned(); diff --git a/diskann-benchmark-core/src/build/ids.rs b/diskann-benchmark-core/src/build/ids.rs index 5decc10fa..e8035c26f 100644 --- a/diskann-benchmark-core/src/build/ids.rs +++ b/diskann-benchmark-core/src/build/ids.rs @@ -7,8 +7,8 @@ use std::marker::PhantomData; -use diskann::{ANNError, ANNErrorKind, ANNResult}; -use diskann_utils::future::AsyncFriendly; +use diskann::{ANNError, ANNResult}; +use diskann_utils::{future::AsyncFriendly, lazy_format}; /// Convert an implicit data index to an external ID. /// @@ -77,7 +77,7 @@ where T: TryFrom + AsyncFriendly, { fn to_id(&self, i: usize) -> ANNResult { - T::try_from(i).map_err(ANNError::opaque) + T::try_from(i).map_err(ANNError::new) } } @@ -115,14 +115,13 @@ where { fn to_id(&self, i: usize) -> ANNResult { self.0.get(i).cloned().ok_or_else(|| { - ANNError::message( - ANNErrorKind::Opaque, - format!( - "tried to index a slice of length {} at index {}", - self.0.len(), - i - ), - ) + let len = self.0.len(); + ANNError::message(lazy_format!( + move, + "tried to index a slice of length {} at index {}", + len, + i + )) }) } } @@ -167,14 +166,13 @@ macro_rules! impl_range { impl ToId<$T> for Range<$T> { fn to_id(&self, i: usize) -> ANNResult<$T> { self.0.clone().nth(i).ok_or_else(|| { - ANNError::message( - ANNErrorKind::Opaque, - format!( - "tried to index a range of length {} at index {}", - self.0.len(), - i - ), - ) + let len = self.0.len(); + ANNError::message(lazy_format!( + move, + "tried to index a range of length {} at index {}", + len, + i + )) }) } } diff --git a/diskann-benchmark-core/src/search/graph/strategy.rs b/diskann-benchmark-core/src/search/graph/strategy.rs index 932f8d27b..edcfc4d02 100644 --- a/diskann-benchmark-core/src/search/graph/strategy.rs +++ b/diskann-benchmark-core/src/search/graph/strategy.rs @@ -5,7 +5,6 @@ use std::{fmt::Debug, sync::Arc}; -use diskann::ANNError; use thiserror::Error; /// A dynamic strategy (e.g. `diskann::graph::glue::SearchStrategy`) manager for built-in @@ -168,12 +167,7 @@ impl Error { } } -impl From for ANNError { - #[track_caller] - fn from(error: Error) -> ANNError { - ANNError::opaque(error) - } -} +diskann::convert_error!(Error); /// Error for an incorrect number of strategies. /// @@ -228,6 +222,8 @@ impl std::error::Error for LengthIncompatible {} mod tests { use super::*; + use diskann::ANNError; + // Simple test strategy type #[derive(Debug, Clone, PartialEq, Eq)] struct TestStrategy(u32); diff --git a/diskann-benchmark/src/index/build.rs b/diskann-benchmark/src/index/build.rs index 8625f4975..b4bf186e6 100644 --- a/diskann-benchmark/src/index/build.rs +++ b/diskann-benchmark/src/index/build.rs @@ -6,7 +6,6 @@ use std::{num::NonZeroUsize, sync::Arc}; use diskann::{ - error::DiskANNError::StartPointComputeError, graph::{DiskANNIndex, StartPointStrategy}, provider::{self, DataProvider, DefaultContext}, ANNError, ANNResult, @@ -43,9 +42,7 @@ where DP: SetStartPoints<[T]>, T: diskann::graph::SampleableForStart + AsyncFriendly, { - let start_points = start_strategy - .compute(data) - .map_err(|e| ANNError::new(diskann::ANNErrorKind::DiskANN(StartPointComputeError), e))?; + let start_points = start_strategy.compute(data).map_err(ANNError::new)?; provider.set_start_points(start_points.row_iter()) } diff --git a/diskann-benchmark/src/index/streaming/full_precision.rs b/diskann-benchmark/src/index/streaming/full_precision.rs index 4e3d62a25..ced09ed3e 100644 --- a/diskann-benchmark/src/index/streaming/full_precision.rs +++ b/diskann-benchmark/src/index/streaming/full_precision.rs @@ -9,7 +9,7 @@ use diskann::{ graph::{DiskANNIndex, InplaceDeleteMethod}, provider::{self, Delete}, utils::{VectorRepr, ONE}, - ANNError, ANNErrorKind, ANNResult, + ANNError, ANNResult, }; use diskann_benchmark_core::recall::{GroundTruthMode, Rows}; use diskann_providers::model::graph::provider::async_::{ @@ -210,7 +210,7 @@ where for internal_id in range { let internal_id: u32 = internal_id .try_into() - .map_err(|_| ANNError::message(ANNErrorKind::Opaque, "invalid id provided"))?; + .map_err(|_| ANNError::message("invalid id provided"))?; if provider .status_by_external_id(ctx, &internal_id) .await? diff --git a/diskann-benchmark/src/inputs/save_and_load.rs b/diskann-benchmark/src/inputs/save_and_load.rs index 3866ac894..176a1a18d 100644 --- a/diskann-benchmark/src/inputs/save_and_load.rs +++ b/diskann-benchmark/src/inputs/save_and_load.rs @@ -21,12 +21,8 @@ pub fn get_graph_num_frozen_points( file.read_exact(&mut usize_buffer)?; let file_frozen_pts = usize::from_le_bytes(usize_buffer); - NonZeroUsize::new(file_frozen_pts).ok_or_else(|| { - ANNError::log_index_config_error( - "num_frozen_pts".to_string(), - "num_frozen_pts is zero in saved file".to_string(), - ) - }) + NonZeroUsize::new(file_frozen_pts) + .ok_or_else(|| ANNError::message("num_frozen_pts is zero in saved file")) } pub fn get_graph_max_observed_degree( diff --git a/diskann-bftree/src/lib.rs b/diskann-bftree/src/lib.rs index 334e0dd82..e0a7f3de0 100644 --- a/diskann-bftree/src/lib.rs +++ b/diskann-bftree/src/lib.rs @@ -48,7 +48,7 @@ impl From for ANNError { #[track_caller] #[inline(never)] fn from(error: ConfigError) -> ANNError { - ANNError::new(diskann::ANNErrorKind::IndexError, error) + ANNError::new(error) } } @@ -90,7 +90,7 @@ impl TransientError for VectorUnavailable { where D: std::fmt::Display, { - ANNError::log_index_error(format!("{self}, escalated: {why}")) + ANNError::message(format!("{self}, escalated: {why}")) } } @@ -107,7 +107,7 @@ pub(crate) fn validate_record_size( let required = key_size + value_size; let configured_max = config.get_cb_max_record_size(); if required > configured_max { - return Err(ANNError::log_index_error(format!( + return Err(ANNError::message(format!( "{provider_name}: cb_max_record_size ({configured_max}) is too small; \ a record requires {required} bytes ({key_size}-byte key + {value_size}-byte value); \ increase cb_max_record_size to at least {required}" @@ -214,6 +214,6 @@ impl std::error::Error for InsertError {} impl From for ANNError { #[track_caller] fn from(error: InsertError) -> Self { - ANNError::new(diskann::ANNErrorKind::IndexError, error) + ANNError::new(error) } } diff --git a/diskann-bftree/src/neighbors.rs b/diskann-bftree/src/neighbors.rs index 148312720..b3e3c6991 100644 --- a/diskann-bftree/src/neighbors.rs +++ b/diskann-bftree/src/neighbors.rs @@ -122,7 +122,7 @@ impl NeighborProvider { if read_size > 0 { // A retrieved neighbor list should be exactly dim length if read_size as usize != self.dim * std::mem::size_of::() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Retrieved neighbor list is not expected length = max degree + 1", )); } @@ -135,7 +135,7 @@ impl NeighborProvider { // The specified list length must be smaller than the retrieved data length if count > self.max_degree() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Size of retrieved neighbor list is shorter than the stored neighbor count", )); } @@ -144,12 +144,12 @@ impl NeighborProvider { } } bf_tree::LeafReadResult::Deleted => { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "The bf-tree entry for the vector is marked as deleted", )); } bf_tree::LeafReadResult::InvalidKey => { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "The bf-tree entry for the vector key is marked as invalid", )); } @@ -196,9 +196,7 @@ impl NeighborProvider { self.num_get_calls.increment(); if buf.len() < self.dim { - return Err(ANNError::log_index_error( - "The provided buffer is not long enough", - )); + return Err(ANNError::message("The provided buffer is not long enough")); } // Serialize the value into the reusable buffer. @@ -227,7 +225,7 @@ impl NeighborProvider { /// - Buffer length (in `I` cells) is smaller than `max_degree() + 1` pub fn set_neighbors(&self, vector_id: I, neighbors: &[I], buf: &mut [I]) -> ANNResult<()> { if neighbors.len() > self.dim - 1 { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "The provided neighbor list is longer than the max degree", )); }; diff --git a/diskann-bftree/src/provider.rs b/diskann-bftree/src/provider.rs index 4fd90a79f..31c1b8b89 100644 --- a/diskann-bftree/src/provider.rs +++ b/diskann-bftree/src/provider.rs @@ -37,6 +37,7 @@ use diskann::{ }; use diskann_utils::{ future::{AsyncFriendly, SendFuture}, + lazy_format, views::MatrixView, }; use diskann_vector::{distance::Metric, DistanceFunction, PreprocessedDistanceFunction}; @@ -343,7 +344,7 @@ where { // Early validation before allocating resources if start_points.nrows() != params.num_start_points.get() { - return Err(ANNError::log_async_index_error(format!( + return Err(ANNError::message(format!( "start_points matrix has {} rows, but params.num_start_points is {}", start_points.nrows(), params.num_start_points.get(), @@ -732,7 +733,7 @@ where fn set_start_points(&self, _hidden: Hidden, start_points: MatrixView<'_, T>) -> ANNResult<()> { let start_point_ids = self.full_vectors.starting_points()?; if start_points.nrows() != start_point_ids.len() { - return Err(ANNError::log_async_index_error(format!( + return Err(ANNError::message(format!( "expected start_points to contain `{}` rows, instead it has {}", start_point_ids.len(), start_points.nrows(), @@ -763,7 +764,7 @@ where fn set_start_points(&self, _hidden: Hidden, start_points: MatrixView<'_, T>) -> ANNResult<()> { let start_point_ids = self.full_vectors.starting_points()?; if start_points.nrows() != start_point_ids.len() { - return Err(ANNError::log_async_index_error(format!( + return Err(ANNError::message(format!( "expected start_points to contain `{}` rows, instead it has {}", start_point_ids.len(), start_points.nrows(), @@ -1691,7 +1692,7 @@ fn save_bftree( use_snapshot: bool, ) -> ANNResult<()> { if !use_snapshot { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "cannot snapshot a BfTree that was not configured with use_snapshot(true)", )); } @@ -1726,7 +1727,7 @@ where let saved_params = SavedParams { max_points: self.max_points(), frozen_points: NonZeroUsize::new(self.num_start_points()) - .ok_or_else(|| ANNError::log_index_error("num_start_points is zero"))?, + .ok_or_else(|| ANNError::message("num_start_points is zero"))?, dim: self.dim(), metric: self.metric().as_str().to_string(), max_degree: self.max_degree(), @@ -1756,7 +1757,7 @@ where { let params_filename = BfTreePaths::params_json(&saved_params.prefix); let params_json = serde_json::to_string(&saved_params).map_err(|e| { - ANNError::log_index_error(format!("Failed to serialize params: {}", e)) + ANNError::message(lazy_format!(move, "Failed to serialize params: {}", e)) })?; let mut params_writer = storage.create_for_write(¶ms_filename)?; params_writer.write_all(params_json.as_bytes())?; @@ -1793,12 +1794,12 @@ where let mut params_json = String::new(); params_reader.read_to_string(&mut params_json)?; serde_json::from_str(¶ms_json).map_err(|e| { - ANNError::log_index_error(format!("Failed to deserialize params: {}", e)) + ANNError::message(lazy_format!(move, "Failed to deserialize params: {}", e)) })? }; let metric = Metric::from_str(&saved_params.metric) - .map_err(|e| ANNError::log_index_error(format!("Failed to parse metric: {}", e)))?; + .map_err(|e| ANNError::message(lazy_format!(move, "Failed to parse metric: {}", e)))?; let vector_index = load_bftree( BfTreePaths::vectors_bftree(&saved_params.prefix), @@ -1846,7 +1847,7 @@ where let saved_params = SavedParams { max_points: self.max_points(), frozen_points: NonZeroUsize::new(self.num_start_points()) - .ok_or_else(|| ANNError::log_index_error("num_start_points is zero"))?, + .ok_or_else(|| ANNError::message("num_start_points is zero"))?, dim: self.dim(), metric: self.metric().as_str().to_string(), max_degree: self.max_degree(), @@ -1887,7 +1888,7 @@ where { let params_filename = BfTreePaths::params_json(&saved_params.prefix); let params_json = serde_json::to_string(&saved_params).map_err(|e| { - ANNError::log_index_error(format!("Failed to serialize params: {}", e)) + ANNError::message(lazy_format!(move, "Failed to serialize params: {}", e)) })?; let mut params_writer = storage.create_for_write(¶ms_filename)?; params_writer.write_all(params_json.as_bytes())?; @@ -1914,7 +1915,7 @@ where .quant_vectors .quantizer .serialize(GlobalAllocator) - .map_err(|e| ANNError::log_index_error(format!("{e}")))?; + .map_err(|e| ANNError::message(lazy_format!(move, "{e}")))?; let mut writer = storage.create_for_write(&filename)?; writer.write_all(&serialized)?; @@ -1938,16 +1939,16 @@ where let mut params_json = String::new(); params_reader.read_to_string(&mut params_json)?; serde_json::from_str(¶ms_json).map_err(|e| { - ANNError::log_index_error(format!("Failed to deserialize params: {}", e)) + ANNError::message(lazy_format!(move, "Failed to deserialize params: {}", e)) })? }; let _quant_params = saved_params.quant_params.ok_or_else(|| { - ANNError::log_index_error("Missing quant_params in saved params for quantized provider") + ANNError::message("Missing quant_params in saved params for quantized provider") })?; let metric = Metric::from_str(&saved_params.metric) - .map_err(|e| ANNError::log_index_error(format!("Failed to parse metric: {}", e)))?; + .map_err(|e| ANNError::message(lazy_format!(move, "Failed to parse metric: {}", e)))?; let vector_index = load_bftree( BfTreePaths::vectors_bftree(&saved_params.prefix), @@ -1974,7 +1975,7 @@ where let mut bytes = Vec::new(); reader.read_to_end(&mut bytes)?; let quantizer: Poly = try_deserialize(&bytes, GlobalAllocator) - .map_err(|e| ANNError::log_index_error(format!("{e}")))?; + .map_err(|e| ANNError::message(lazy_format!(move, "{e}")))?; let quant_vector_index = load_bftree( BfTreePaths::quant_bftree(&saved_params.prefix), diff --git a/diskann-bftree/src/quant.rs b/diskann-bftree/src/quant.rs index 8a37ffd19..c64835b5f 100644 --- a/diskann-bftree/src/quant.rs +++ b/diskann-bftree/src/quant.rs @@ -14,6 +14,7 @@ use diskann_quantization::{ DistanceComputer, Opaque, OpaqueMut, Quantizer, QueryComputer, QueryLayout, }, }; +use diskann_utils::lazy_format; use diskann_vector::PreprocessedDistanceFunction; use super::ConfigError; @@ -25,7 +26,7 @@ impl QuantQueryComputer { pub(crate) fn evaluate(&self, x: &[u8]) -> ANNResult { match self.0.evaluate_similarity(Opaque::new(x)) { Ok(distance) => Ok(distance), - Err(err) => Err(ANNError::new(diskann::ANNErrorKind::IndexError, err)), + Err(err) => Err(ANNError::new(err)), } } } @@ -102,7 +103,7 @@ impl QuantVectorProvider { GlobalAllocator, ScopedAllocator::global(), ) - .map_err(|e| ANNError::log_sq_error(e))?; + .map_err(ANNError::new)?; Ok(QuantQueryComputer(inner)) } @@ -110,11 +111,10 @@ impl QuantVectorProvider { pub fn distance_computer(&self) -> ANNResult { self.quantizer .distance_computer(GlobalAllocator) - .map_err(|e| ANNError::log_sq_error(e)) + .map_err(ANNError::new) } pub(crate) fn get_vector_into(&self, i: usize, buffer: &mut [u8]) -> Result<(), AccessError> { - use diskann::ANNErrorKind; use thiserror::Error; let expected = self.quantizer.bytes(); @@ -123,17 +123,18 @@ impl QuantVectorProvider { #[error("expected a buffer with dim {0}, instead got {1}")] struct WrongDim(usize, usize); - return Err(AccessError::Error(ANNError::new( - ANNErrorKind::IndexError, - WrongDim(expected, buffer.len()), - ))); + return Err(AccessError::Error(ANNError::new(WrongDim( + expected, + buffer.len(), + )))); } self.num_get_calls.increment(); match self.quant_vector_index.read(bytemuck::bytes_of(&i), buffer) { bf_tree::LeafReadResult::Found(read_size) => { if read_size as usize != expected { - return Err(AccessError::Error(ANNError::log_index_error(format!( + return Err(AccessError::Error(ANNError::message(lazy_format!( + move, "The bf-tree entry for vector id {} is marked as found but has size {} instead of the expected size {}", i, read_size, expected, )))); @@ -146,7 +147,8 @@ impl QuantVectorProvider { })); } bf_tree::LeafReadResult::InvalidKey => { - return Err(AccessError::Error(ANNError::log_index_error(format!( + return Err(AccessError::Error(ANNError::message(lazy_format!( + move, "The bf-tree entry for vector id {} is marked as invalid", i, )))); @@ -183,8 +185,8 @@ impl QuantVectorProvider { let vf32: &[f32] = &T::as_f32(v).into_ann_result()?; if vf32.len() != self.full_dim() { - return Err(ANNError::log_dimension_mismatch_error( - "Vector f32 dimension is not equal to the expected dimension.".to_string(), + return Err(ANNError::message( + "Vector f32 dimension is not equal to the expected dimension.", )); } @@ -199,7 +201,7 @@ impl QuantVectorProvider { OpaqueMut::new(quant_vector), ScopedAllocator::global(), ) - .map_err(|e| ANNError::log_sq_error(e))?; + .map_err(ANNError::new)?; bftree_insert(&self.quant_vector_index, key, quant_vector)?; @@ -214,7 +216,7 @@ impl QuantVectorProvider { #[cfg(test)] pub(crate) fn set_quant_vector(&self, i: usize, v: &[u8]) -> ANNResult<()> { if v.len() != self.quantizer.bytes() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } @@ -275,7 +277,6 @@ pub(crate) fn create_test_quantizer(dim: usize) -> Poly { mod tests { use std::sync::Arc; - use diskann::ANNErrorKind; use diskann_quantization::spherical::iface::Opaque; use diskann_vector::DistanceFunction; use tokio::task::JoinSet; @@ -293,16 +294,13 @@ mod tests { let provider = QuantVectorProvider::new_with_config(quantizer, bf_tree_config).unwrap(); // try to set an out of bounds vector - let result = provider.set_quant_vector(20, &[]).unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::IndexError); + let _ = provider.set_quant_vector(20, &[]).unwrap_err(); // try to set an out of bounds vector via set_vector_sync - let result = provider.set_vector_sync::(20, &[]).unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::DimensionMismatchError); + let _ = provider.set_vector_sync::(20, &[]).unwrap_err(); // try to set a quant vector with the wrong dimension - let result = provider.set_quant_vector(0, &[]).unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::IndexError); + let _ = provider.set_quant_vector(0, &[]).unwrap_err(); // verify expected quant vector byte count assert_eq!(quant_bytes, provider.quantizer.bytes()); diff --git a/diskann-bftree/src/vectors.rs b/diskann-bftree/src/vectors.rs index 661ffac08..ab535401f 100644 --- a/diskann-bftree/src/vectors.rs +++ b/diskann-bftree/src/vectors.rs @@ -10,7 +10,8 @@ use std::marker::PhantomData; use crate::{AccessError, VectorError, VectorUnavailable}; use bf_tree::{BfTree, Config}; use bytemuck::cast_slice; -use diskann::{error::RankedError, utils::VectorRepr, ANNError, ANNErrorKind, ANNResult}; +use diskann::{error::RankedError, utils::VectorRepr, ANNError, ANNResult}; +use diskann_utils::lazy_format; use thiserror::Error; use super::ConfigError; @@ -99,7 +100,7 @@ impl VectorProvider { (self.max_vectors..self.total()) .map(|i| { u32::try_from(i).map_err(|_| { - ANNError::log_index_error(format_args!("start point id {i} exceeds u32::MAX")) + ANNError::message(lazy_format!(move, "start point id {i} exceeds u32::MAX")) }) }) .collect() @@ -124,12 +125,12 @@ impl VectorProvider { #[inline(always)] pub(crate) fn set_vector_sync(&self, i: usize, v: &[T]) -> ANNResult<()> { if v.len() != self.dim { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } if i >= self.total() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } @@ -149,10 +150,10 @@ impl VectorProvider { #[error("expected a buffer with dim {0}, instead got {1}")] struct WrongDim(usize, usize); - return Err(RankedError::Error(ANNError::new( - ANNErrorKind::IndexError, - WrongDim(self.dim(), buffer.len()), - ))); + return Err(RankedError::Error(ANNError::new(WrongDim( + self.dim(), + buffer.len(), + )))); } self.num_get_calls.increment(); @@ -163,7 +164,8 @@ impl VectorProvider { bf_tree::LeafReadResult::Found(read_size) => { let vector_size = std::mem::size_of::() * self.dim; if read_size as usize != vector_size { - return Err(RankedError::Error(ANNError::log_index_error(format!( + return Err(RankedError::Error(ANNError::message(lazy_format!( + move, "The bf-tree entry for vector id {} is marked as found but has size {} instead of the expected size {}", i, read_size, vector_size, )))); @@ -176,7 +178,8 @@ impl VectorProvider { })); } bf_tree::LeafReadResult::InvalidKey => { - return Err(RankedError::Error(ANNError::log_index_error(format!( + return Err(RankedError::Error(ANNError::message(lazy_format!( + move, "The bf-tree entry for vector id {} is marked as invalid", i )))); diff --git a/diskann-disk/src/build/builder/build.rs b/diskann-disk/src/build/builder/build.rs index 432c577fc..c0df5d6b7 100644 --- a/diskann-disk/src/build/builder/build.rs +++ b/diskann-disk/src/build/builder/build.rs @@ -13,7 +13,7 @@ use std::{ use crate::data_model::GraphDataType; use diskann::{ utils::{async_tools, VectorRepr, ONE}, - ANNError, ANNResult, + ANNResult, }; use diskann_providers::storage::{StorageReadProvider, StorageWriteProvider}; use diskann_providers::{ @@ -37,6 +37,7 @@ use crate::{ quantizer::BuildQuantizer, tokio::create_runtime, }, + error::{diskann_error, ErrorKind}, storage::{ quant::{PQGeneration, PQGenerationContext, QuantDataGenerator}, DiskIndexWriter, @@ -226,7 +227,12 @@ where // use either user-specified number of threads or default to available parallelism let num_tasks = NonZeroUsize::new(config.num_threads) .or_else(|| std::thread::available_parallelism().ok()) - .ok_or_else(|| ANNError::log_index_error("Failed to determine number of threads"))?; + .ok_or_else(|| { + diskann_error!( + ErrorKind::IndexError, + "Failed to determine number of threads" + ) + })?; // Associated data will only be used in the write_disk_layout function which only requires the none-partitioned associated data stream. let dataset_iter = Arc::new(Mutex::new({ @@ -293,7 +299,7 @@ async fn log_build_stats(index: &Arc>) - /// `u32::MAX`. fn u32_try_from(value: usize) -> ANNResult { u32::try_from(value) - .map_err(|_| ANNError::log_index_error(format_args!("id {value} exceeds u32::MAX"))) + .map_err(|_| diskann_error!(ErrorKind::IndexError, "id {value} exceeds u32::MAX")) } fn set_start_point_to_medoid( @@ -338,7 +344,7 @@ where for _ in partition { let vector_data = { let mut guard = iterator_clone.lock().map_err(|_| { - ANNError::log_index_error("Poisoned mutex during construction") + diskann_error!(ErrorKind::IndexError, "Poisoned mutex during construction") })?; guard.next() }; @@ -357,7 +363,7 @@ where // Wait for all tasks to complete. while let Some(res) = tasks.join_next().await { - res.map_err(|_| ANNError::log_index_error("A spawned insert task failed"))??; + res.map_err(|_| diskann_error!(ErrorKind::IndexError, "A spawned insert task failed"))??; } info!("Linked all points. Num points: #{}", total_points); @@ -382,7 +388,9 @@ async fn run_final_prune( // Wait for all final prune tasks to complete while let Some(res) = tasks.join_next().await { - res.map_err(|_| ANNError::log_index_error("A spawned final prune task failed"))??; + res.map_err(|_| { + diskann_error!(ErrorKind::IndexError, "A spawned final prune task failed") + })??; } Ok(()) diff --git a/diskann-disk/src/build/builder/quantizer.rs b/diskann-disk/src/build/builder/quantizer.rs index 7bbfacf3c..b0c0d807a 100644 --- a/diskann-disk/src/build/builder/quantizer.rs +++ b/diskann-disk/src/build/builder/quantizer.rs @@ -4,7 +4,7 @@ */ //! Disk index quantizer implementation. use crate::data_model::GraphDataType; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_providers::storage::{StorageReadProvider, StorageWriteProvider}; use diskann_providers::{ index::diskann_async::train_pq, @@ -19,7 +19,10 @@ use diskann_quantization::scalar::train::ScalarQuantizationParameters; use diskann_utils::views::MatrixView; use tracing::info; -use crate::QuantizationType; +use crate::{ + error::{diskann_error, ErrorKind}, + QuantizationType, +}; /// Quantizer types used specifically for async disk index building. #[derive(Clone)] @@ -86,9 +89,9 @@ impl BuildQuantizer { standard_deviation, } => { if nbits != 1 { - return Err(ANNError::log_index_config_error( - "build_quantization_type".to_string(), - "SQ quantization is only supported for 1 bit".to_string(), + return Err(diskann_error!( + ErrorKind::IndexConfigError("build_quantization_type"), + "SQ quantization is only supported for 1 bit", )); } let rng = diskann_providers::utils::create_rnd_provider_from_optional_seed( diff --git a/diskann-disk/src/build/builder/tokio.rs b/diskann-disk/src/build/builder/tokio.rs index 4b66c37ce..f87d59598 100644 --- a/diskann-disk/src/build/builder/tokio.rs +++ b/diskann-disk/src/build/builder/tokio.rs @@ -3,7 +3,9 @@ * Licensed under the MIT license. */ -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; + +use crate::error::{diskann_error, ErrorKind}; /// Creates a new multi-threaded tokio runtime with the specified number of worker threads. /// If `num_threads` is 0, it defaults to the number of logical CPUs. @@ -15,7 +17,11 @@ pub fn create_runtime(num_threads: usize) -> ANNResult } builder.build().map_err(|err| { - ANNError::log_index_error(format!("Failed to initialize tokio runtime: {}", err)) + diskann_error!( + ErrorKind::IndexError, + "Failed to initialize tokio runtime: {}", + err + ) }) } diff --git a/diskann-disk/src/build/configuration/disk_index_build_parameter.rs b/diskann-disk/src/build/configuration/disk_index_build_parameter.rs index 077c5090d..b4ba343b1 100644 --- a/diskann-disk/src/build/configuration/disk_index_build_parameter.rs +++ b/diskann-disk/src/build/configuration/disk_index_build_parameter.rs @@ -12,6 +12,8 @@ use thiserror::Error; use super::QuantizationType; +use crate::error::{diskann_error, ErrorKind}; + /// GB to bytes ratio. pub const BYTES_IN_GB: f64 = 1024_f64 * 1024_f64 * 1024_f64; @@ -28,7 +30,7 @@ pub struct InvalidMemBudget; impl From for ANNError { fn from(value: InvalidMemBudget) -> Self { - ANNError::log_index_config_error("MemoryBudget".to_string(), format!("{value:?}")) + diskann_error!(ErrorKind::IndexConfigError("MemoryBudget"), value) } } @@ -71,7 +73,7 @@ pub enum PQChunksError { impl From for ANNError { fn from(value: PQChunksError) -> Self { - ANNError::log_index_config_error("NumPQChunks".to_string(), format!("{value:?}")) + diskann_error!(ErrorKind::IndexConfigError("NumPQChunks"), value) } } @@ -167,7 +169,9 @@ impl DiskIndexBuildParameters { #[cfg(test)] mod dataset_test { - use diskann::{ANNError, ANNErrorKind}; + use diskann::ANNError; + + use crate::error::{error_kind, ErrorKind}; use super::*; @@ -230,7 +234,10 @@ mod dataset_test { let err = MemoryBudget::try_from_gb(-1.0) .map_err(ANNError::from) .unwrap_err(); - assert_eq!(err.kind(), ANNErrorKind::IndexConfigError); + assert_eq!( + error_kind(&err), + ErrorKind::IndexConfigError("MemoryBudget") + ); } #[test] diff --git a/diskann-disk/src/data_model/cache.rs b/diskann-disk/src/data_model/cache.rs index e0b18d00d..4287f1675 100644 --- a/diskann-disk/src/data_model/cache.rs +++ b/diskann-disk/src/data_model/cache.rs @@ -3,8 +3,11 @@ * Licensed under the MIT license. */ -use crate::data_model::GraphDataType; -use diskann::{graph::AdjacencyList, ANNError, ANNResult}; +use crate::{ + data_model::GraphDataType, + error::{diskann_error, ErrorKind}, +}; +use diskann::{graph::AdjacencyList, ANNResult}; use hashbrown::{hash_map::Entry::Occupied, HashMap}; pub struct Cache> { @@ -91,7 +94,8 @@ where associated_data: Data::AssociatedDataType, ) -> ANNResult<()> { if self.dimension != vector.len() { - return ANNResult::Err(ANNError::log_index_error( + return Err(diskann_error!( + ErrorKind::IndexError, "Vector dimension does not match the dimension set in cache.", )); } @@ -103,7 +107,8 @@ where } if self.mapping.len() >= self.capacity { - return ANNResult::Err(ANNError::log_index_error( + return Err(diskann_error!( + ErrorKind::IndexError, "Cache is full, cannot insert more nodes", )); } diff --git a/diskann-disk/src/data_model/graph_header.rs b/diskann-disk/src/data_model/graph_header.rs index f04803e4a..7ca5fbb0b 100644 --- a/diskann-disk/src/data_model/graph_header.rs +++ b/diskann-disk/src/data_model/graph_header.rs @@ -11,6 +11,8 @@ use thiserror::Error; use super::{GraphLayoutVersion, GraphMetadata}; +use crate::error::{diskann_error, ErrorKind}; + /// GraphHeader. The header is stored in the first sector of the disk index file, or the first segment of the JET stream. pub struct GraphHeader { // Graph metadata. @@ -34,7 +36,7 @@ pub enum GraphHeaderError { impl From for ANNError { #[track_caller] fn from(value: GraphHeaderError) -> Self { - ANNError::log_index_error(value) + diskann_error!(ErrorKind::IndexError, value) } } @@ -124,10 +126,9 @@ impl<'a> TryFrom<&'a [u8]> for GraphHeader { /// | GraphMetadata (80 bytes) | BlockSize (8 bytes) | GraphLayoutVersion (8 bytes) | fn try_from(value: &'a [u8]) -> ANNResult { if value.len() < Self::get_size() { - Err(ANNError::log_parse_slice_error( - "&[u8]".to_string(), - "GraphHeader".to_string(), - "The given bytes are not long enough to create a valid graph header.".to_string(), + Err(diskann_error!( + ErrorKind::SerdeError, + "The given bytes are not long enough to create a valid graph header.", )) } else { // Parse metadata. @@ -147,11 +148,13 @@ impl<'a> TryFrom<&'a [u8]> for GraphHeader { #[cfg(test)] mod tests { - use diskann::ANNErrorKind; use rstest::rstest; use super::*; - use crate::data_model::{GraphHeader, GraphLayoutVersion, GraphMetadata}; + use crate::{ + data_model::{GraphHeader, GraphLayoutVersion, GraphMetadata}, + error::{error_kind, ErrorKind}, + }; #[test] fn test_graph_header_to_bytes_and_try_from() { @@ -341,11 +344,11 @@ mod tests { fn test_graph_header_error_conversion() { let error = GraphHeaderError::MaxDegreeOverflow; let ann_error: ANNError = error.into(); - assert_eq!(ann_error.kind(), ANNErrorKind::IndexError); + assert_eq!(error_kind(&ann_error), ErrorKind::IndexError); let layout_version = GraphLayoutVersion::new(1, 0); let error = GraphHeaderError::MaxDegreeUnsupportedLayoutVersion(layout_version); let ann_error: ANNError = error.into(); - assert_eq!(ann_error.kind(), ANNErrorKind::IndexError); + assert_eq!(error_kind(&ann_error), ErrorKind::IndexError); } } diff --git a/diskann-disk/src/data_model/graph_layout_version.rs b/diskann-disk/src/data_model/graph_layout_version.rs index 62e665cc8..5118e5e1c 100644 --- a/diskann-disk/src/data_model/graph_layout_version.rs +++ b/diskann-disk/src/data_model/graph_layout_version.rs @@ -8,6 +8,8 @@ use std::{cmp::Ordering, fmt, io::Cursor}; use byteorder::{LittleEndian, ReadBytesExt}; use diskann::{ANNError, ANNResult}; +use crate::error::{diskann_error, ErrorKind}; + /// Graph layout version. In the format of `major.minor`. #[derive(Debug, PartialEq, Eq, Clone)] pub struct GraphLayoutVersion { @@ -69,11 +71,9 @@ impl<'a> TryFrom<&'a [u8]> for GraphLayoutVersion { /// | MajorVersion (4 bytes) | MinorVersion (4 bytes) | fn try_from(value: &'a [u8]) -> ANNResult { if value.len() < std::mem::size_of::() { - Err(ANNError::log_parse_slice_error( - "&[u8]".to_string(), - "GraphLayoutVersion".to_string(), + Err(diskann_error!( + ErrorKind::SerdeError, "The given bytes are not long enough to create a valid graph layout version." - .to_string(), )) } else { let mut cursor = Cursor::new(&value); diff --git a/diskann-disk/src/data_model/graph_metadata.rs b/diskann-disk/src/data_model/graph_metadata.rs index 4ff35916b..7cbd0ba29 100644 --- a/diskann-disk/src/data_model/graph_metadata.rs +++ b/diskann-disk/src/data_model/graph_metadata.rs @@ -8,6 +8,8 @@ use std::io::Cursor; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use diskann::{ANNError, ANNResult}; +use crate::error::{diskann_error, ErrorKind}; + /// Index graph metadata. The metadata is stored in the first sector of the disk index file, or the first segment of the BigStorageStream. /// The metadata is like a "header" of the index graph. #[derive(Debug, Clone)] @@ -109,10 +111,9 @@ impl<'a> TryFrom<&'a [u8]> for GraphMetadata { /// ...| vamana_frozen_loc (8 bytes) | append_reorder_data (8 bytes) | disk_index_file_size (8 bytes) | associated_data_length (8 bytes) | fn try_from(value: &'a [u8]) -> ANNResult { if value.len() < Self::get_size() { - return Err(ANNError::log_parse_slice_error( - "&[u8]".to_string(), - "GraphMetadata".to_string(), - "The given bytes are not long enough to create a valid graph metadata.".to_string(), + return Err(diskann_error!( + ErrorKind::SerdeError, + "The given bytes are not long enough to create a valid graph metadata.", )); } diff --git a/diskann-disk/src/error.rs b/diskann-disk/src/error.rs new file mode 100644 index 000000000..2dd96f5aa --- /dev/null +++ b/diskann-disk/src/error.rs @@ -0,0 +1,278 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +//! Local tagged error type for `diskann-disk`. +//! +//! # Historical Context +//! +//! Error representation has changed over the course of the `diskann`'s history. Original +//! versions of error handling used single top-level enum to encode error types and payloads +//! and much of the test code checking errors within `diskann-disk` was written using this +//! paradigm. +//! +//! The decisions around using a tagged [`ErrorKind`] are to keep test code within +//! `diskann-disk` relatively static during refactors of central [`diskann::ANNError`]. +//! +//! The [`crate::diskann_error!`] marco should be used to get most of the benefits from +//! [`diskann::ANNError`] by: +//! +//! * Constructing a tagged [`Error`] in an efficient way. +//! * Creating a new [`diskann::ANNError`] in-place, ensuring that the source line tracking +//! of that type is accurate. +//! +//! A limitation of this approach is that it forces string formatting upon error construction +//! (though in a few cases like `&'static str` literals, we can avoid this allocation). +//! Depending on the context, this formatting can negatively impact generated code even when +//! not used or add overhead on the error path. Direct use of [`diskann::ANNResult`] has +//! less overhead as error/display types are moved directly into that's types allocated +//! storage, costing just a relatively small allocation at construction time rather than +//! running string formatting eagerly. + +use std::borrow::Cow; + +/// Disk index related errors tagged with a provenance [`ErrorKind`]. +/// +/// These errors can be retrieved from [`diskann::ANNError`] by using the +/// [`downcast`](diskann::ANNError::downcast_ref) APIs. +#[derive(Debug)] +pub struct Error { + kind: ErrorKind, + message: Cow<'static, str>, +} + +impl Error { + /// Construct a new tagged [`Error`]. + pub(crate) fn new(kind: ErrorKind, message: impl Into>) -> Self { + Self { + kind, + message: message.into(), + } + } + + /// Construct a new [`Error`] using `message`'s implementation of [`ToString`]. + pub(crate) fn from_display(kind: ErrorKind, message: D) -> Self + where + D: ToString, + { + Self::new(kind, message.to_string()) + } + + /// Return the tagged [`ErrorKind`] of this error. + #[inline] + pub fn kind(&self) -> ErrorKind { + self.kind + } + + /// Construct a new [`Error`] from [`std::fmt::Arguments`]. + /// + /// This method avoids allocating if [`std::fmt::Arguments::as_str`] returns `Some`. + #[inline] + pub(crate) fn from_args(kind: ErrorKind, args: std::fmt::Arguments<'_>) -> Self { + let message = match args.as_str() { + Some(s) => Cow::Borrowed(s), + None => Cow::Owned(args.to_string()), + }; + + Self { kind, message } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}: {}", self.kind, self.message) + } +} + +impl std::error::Error for Error {} + +diskann::convert_error!(Error); + +/// Classification of error types in [`Error`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ErrorKind { + IndexError, + PQError, + KMeansError, + IndexConfigError(&'static str), + DimensionMismatchError, + SerdeError, + DiskIOAlignmentError, +} + +impl std::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ErrorKind::IndexError => f.write_str("IndexError"), + ErrorKind::PQError => f.write_str("PQError"), + ErrorKind::KMeansError => f.write_str("KMeansError"), + ErrorKind::IndexConfigError(key) => write!(f, "IndexConfigError for \"{}\"", key), + ErrorKind::DimensionMismatchError => f.write_str("DimensionMismatchError"), + ErrorKind::SerdeError => f.write_str("SerdeError"), + ErrorKind::DiskIOAlignmentError => f.write_str("DiskIOAlignmentError"), + } + } +} + +/// Construct a [`diskann::ANNError`] containing a tagged [`Error`]. +/// +/// This macro attempts to use the most efficient construction mechanism. +/// +/// Since this is an internal macro, see the tests for usage. +macro_rules! diskann_error { + ($kind:expr, $var:ident) => { + ::diskann::ANNError::new( + $crate::error::Error::from_display( + $kind, + &$var, + ) + ) + }; + ($kind:expr, $($args:tt)*) => { + ::diskann::ANNError::new( + $crate::error::Error::from_args( + $kind, + format_args!($($args)*), + ) + ) + }; +} + +pub(crate) use diskann_error; + +//----------------// +// Test Utilities // +//----------------// + +#[cfg(test)] +pub(crate) fn error_kind(err: &diskann::ANNError) -> ErrorKind { + match err.downcast_ref::() { + Some(e) => e.kind(), + None => panic!("error payload is not a `$crate::Error`"), + } +} + +/////////// +// Tests // +/////////// + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_error() { + // New - borrowed + let err = Error::new(ErrorKind::IndexError, "a &'static str"); + assert_eq!(err.kind(), ErrorKind::IndexError); + assert!(matches!(err.message, Cow::Borrowed("a &'static str"))); + assert_eq!(err.to_string(), "IndexError: a &'static str"); + + // New - owned + let err = Error::new(ErrorKind::PQError, String::from("a string")); + assert_eq!(err.kind(), ErrorKind::PQError); + assert!(matches!(err.message, Cow::Owned(_))); + assert_eq!(err.message, "a string"); + assert_eq!(err.to_string(), "PQError: a string"); + + // `from_display` + let err = Error::from_display(ErrorKind::PQError, ErrorKind::PQError); + assert_eq!(err.kind(), ErrorKind::PQError); + assert!(matches!(err.message, Cow::Owned(_))); + assert_eq!(err.message, "PQError"); + + // `from_args` - non-allocating. + let err = Error::from_args(ErrorKind::IndexError, format_args!("a &'static str")); + assert_eq!(err.kind(), ErrorKind::IndexError); + assert!(matches!(err.message, Cow::Borrowed("a &'static str"))); + + // `from_args` - allocating. + let err = Error::from_args( + ErrorKind::IndexError, + format_args!("a {}", ErrorKind::PQError), + ); + assert_eq!(err.kind(), ErrorKind::IndexError); + assert!(matches!(err.message, Cow::Owned(_))); + assert_eq!(err.message, "a PQError"); + } + + #[test] + fn test_error_kind() { + assert_eq!(ErrorKind::IndexError.to_string(), "IndexError"); + assert_eq!(ErrorKind::PQError.to_string(), "PQError"); + assert_eq!(ErrorKind::KMeansError.to_string(), "KMeansError"); + assert_eq!( + ErrorKind::IndexConfigError("foo").to_string(), + "IndexConfigError for \"foo\"" + ); + assert_eq!( + ErrorKind::IndexConfigError("bar").to_string(), + "IndexConfigError for \"bar\"" + ); + assert_eq!( + ErrorKind::DimensionMismatchError.to_string(), + "DimensionMismatchError" + ); + assert_eq!(ErrorKind::SerdeError.to_string(), "SerdeError"); + assert_eq!( + ErrorKind::DiskIOAlignmentError.to_string(), + "DiskIOAlignmentError" + ); + } + + #[test] + fn test_macro() { + // Variable identifiers - `IndexConfigError` + let var = String::from("oops"); + let err = diskann_error!(ErrorKind::IndexConfigError("some variable"), var); + assert_eq!( + error_kind(&err), + ErrorKind::IndexConfigError("some variable") + ); + let err = err.downcast::().unwrap(); + assert_eq!( + err.to_string(), + "IndexConfigError for \"some variable\": oops" + ); + + // Variable identifiers - unit error. + let err = diskann_error!(ErrorKind::IndexError, var); + assert_eq!(error_kind(&err), ErrorKind::IndexError); + let err = err.downcast::().unwrap(); + assert_eq!(err.to_string(), "IndexError: oops"); + + // formatting with string literal - non-allocating. + let err = diskann_error!(ErrorKind::IndexError, "something went wrong"); + assert_eq!(error_kind(&err), ErrorKind::IndexError); + let err = err.downcast::().unwrap(); + assert_eq!(err.to_string(), "IndexError: something went wrong"); + assert!(matches!(err.message, Cow::Borrowed(_))); + + let err = diskann_error!(ErrorKind::IndexConfigError("foo"), "something went wrong"); + assert_eq!(error_kind(&err), ErrorKind::IndexConfigError("foo")); + let err = err.downcast::().unwrap(); + assert_eq!( + err.to_string(), + "IndexConfigError for \"foo\": something went wrong" + ); + assert!(matches!(err.message, Cow::Borrowed(_))); + + // formatting - allocating. + let x = 1; + let y = 2; + let z = 3; + let err = diskann_error!( + ErrorKind::IndexError, + "something went wrong - {x}, {}, and {}", + y, + z, + ); + assert_eq!(error_kind(&err), ErrorKind::IndexError); + let err = err.downcast::().unwrap(); + assert_eq!( + err.to_string(), + "IndexError: something went wrong - 1, 2, and 3" + ); + } +} diff --git a/diskann-disk/src/lib.rs b/diskann-disk/src/lib.rs index 845b774df..facaebf09 100644 --- a/diskann-disk/src/lib.rs +++ b/diskann-disk/src/lib.rs @@ -11,6 +11,8 @@ #[cfg(test)] pub(crate) mod test_utils; +pub mod error; + pub mod build; pub use build::{ disk_index_build_parameter, filter_parameter, DiskIndexBuildParameters, QuantizationType, diff --git a/diskann-disk/src/search/pq/pq_scratch.rs b/diskann-disk/src/search/pq/pq_scratch.rs index ff4b49b1a..54408c0e8 100644 --- a/diskann-disk/src/search/pq/pq_scratch.rs +++ b/diskann-disk/src/search/pq/pq_scratch.rs @@ -4,10 +4,12 @@ */ //! Aligned allocator -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_quantization::alloc::{AlignedAllocator, Poly}; +use crate::error::{diskann_error, ErrorKind}; + #[derive(Debug)] /// PQ scratch pub struct PQScratch { @@ -42,12 +44,12 @@ impl PQScratch { ) -> ANNResult { let aligned_pq_coord_scratch = Poly::broadcast(0u8, graph_degree * num_pq_chunks, AlignedAllocator::A128) - .map_err(ANNError::log_index_error)?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; let aligned_pqtable_dist_scratch = Poly::broadcast(0f32, num_centers * num_pq_chunks, AlignedAllocator::A128) - .map_err(ANNError::log_index_error)?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; let aligned_dist_scratch = Poly::broadcast(0f32, graph_degree, AlignedAllocator::A128) - .map_err(ANNError::log_index_error)?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; let query_scratch = vec![0.0f32; dim]; Ok(Self { @@ -68,10 +70,11 @@ impl PQScratch { pub fn set(&mut self, query: &[f32]) -> ANNResult<()> { let dim = self.query_scratch.len(); if query.len() != dim { - return Err(ANNError::log_dimension_mismatch_error(format!( + return Err(diskann_error!( + ErrorKind::DimensionMismatchError, "PQScratch::set: expected query of length {dim}, got {}", query.len() - ))); + )); } self.query_scratch.copy_from_slice(query); Ok(()) @@ -91,6 +94,8 @@ mod tests { use super::PQScratch; + use crate::error::{error_kind, ErrorKind}; + #[rstest] #[case(512, 8, 128, 256)] // default test case #[case(59, 16, 37, 41)] // not multiple of 256 @@ -135,7 +140,7 @@ mod tests { // Query shorter than dim should fail let short_query: Vec = (1..dim).map(|i| i as f32).collect(); // dim-1 elements let err = pq_scratch.set(&short_query).unwrap_err(); - assert_eq!(err.kind(), diskann::ANNErrorKind::DimensionMismatchError); + assert_eq!(error_kind(&err), ErrorKind::DimensionMismatchError); assert!(err.to_string().contains("expected query of length")); } @@ -147,7 +152,7 @@ mod tests { // Query longer than dim should fail let long_query: Vec = (1..=dim + 10).map(|i| i as f32).collect(); let err = pq_scratch.set(&long_query).unwrap_err(); - assert_eq!(err.kind(), diskann::ANNErrorKind::DimensionMismatchError); + assert_eq!(error_kind(&err), ErrorKind::DimensionMismatchError); assert!(err.to_string().contains("expected query of length")); } } diff --git a/diskann-disk/src/search/provider/aligned_file_reader/aligned_read.rs b/diskann-disk/src/search/provider/aligned_file_reader/aligned_read.rs index 8261d2c26..93fb4c701 100644 --- a/diskann-disk/src/search/provider/aligned_file_reader/aligned_read.rs +++ b/diskann-disk/src/search/provider/aligned_file_reader/aligned_read.rs @@ -5,9 +5,11 @@ use std::marker::PhantomData; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_quantization::num::PowerOfTwo; +use crate::error::{diskann_error, ErrorKind}; + /// Type-level memory-alignment witness for [`AlignedRead`]. Each implementor is /// a unit type carrying a single `PowerOfTwo` value. /// @@ -69,9 +71,10 @@ impl<'a, T, A: Alignment> AlignedRead<'a, T, A> { if val.is_multiple_of(align) { Ok(()) } else { - Err(ANNError::log_disk_io_request_alignment_error(format!( + Err(diskann_error!( + ErrorKind::DiskIOAlignmentError, "{kind} {val} not aligned to {align}", - ))) + )) } } @@ -91,9 +94,10 @@ impl<'a, T, A: Alignment> AlignedRead<'a, T, A> { #[cfg(test)] mod tests { use super::*; - use diskann::ANNErrorKind; use diskann_quantization::alloc::{AlignedAllocator, Poly}; + use crate::error::{error_kind, ErrorKind}; + fn aligned_512(len: usize) -> Poly<[u8], AlignedAllocator> { Poly::broadcast(0u8, len, AlignedAllocator::A512).unwrap() } @@ -133,7 +137,7 @@ mod tests { let slice = &mut buf[1..513]; // ptr offset by 1; length 512 ✓; offset 0 ✓ let err = AlignedRead::::new(0, slice) .expect_err("misaligned buffer pointer should be rejected"); - assert_eq!(err.kind(), ANNErrorKind::DiskIOAlignmentError); + assert_eq!(error_kind(&err), ErrorKind::DiskIOAlignmentError); } #[test] @@ -142,7 +146,7 @@ mod tests { let slice = &mut buf[..100]; // ptr ✓; length 100 ✗; offset 0 ✓ let err = AlignedRead::::new(0, slice) .expect_err("buffer length 100 (not a multiple of 512) should be rejected"); - assert_eq!(err.kind(), ANNErrorKind::DiskIOAlignmentError); + assert_eq!(error_kind(&err), ErrorKind::DiskIOAlignmentError); } #[test] @@ -151,6 +155,6 @@ mod tests { let slice = &mut buf[..512]; // ptr ✓; length 512 ✓; offset 1 ✗ let err = AlignedRead::::new(1, slice) .expect_err("offset 1 (not a multiple of 512) should be rejected"); - assert_eq!(err.kind(), ANNErrorKind::DiskIOAlignmentError); + assert_eq!(error_kind(&err), ErrorKind::DiskIOAlignmentError); } } diff --git a/diskann-disk/src/search/provider/aligned_file_reader/reader/linux.rs b/diskann-disk/src/search/provider/aligned_file_reader/reader/linux.rs index a8cf8184c..d897fb736 100644 --- a/diskann-disk/src/search/provider/aligned_file_reader/reader/linux.rs +++ b/diskann-disk/src/search/provider/aligned_file_reader/reader/linux.rs @@ -11,8 +11,11 @@ use diskann::{ANNError, ANNResult}; use io_uring::IoUring; use libc; -use crate::search::provider::aligned_file_reader::{ - platform::IOContext, traits::AlignedFileReader, AlignedRead, A512, +use crate::{ + error::{diskann_error, ErrorKind}, + search::provider::aligned_file_reader::{ + platform::IOContext, traits::AlignedFileReader, AlignedRead, A512, + }, }; pub const MAX_IO_CONCURRENCY: usize = 128; @@ -34,17 +37,10 @@ impl LinuxAlignedFileReader { // Open file as read-only // Apply the `O_DIRECT` flag to bypass the kernel page cache. // See: https://man7.org/linux/man-pages/man2/open.2.html - let open_result = OpenOptions::new() + let file = OpenOptions::new() .read(true) .custom_flags(libc::O_DIRECT) - .open(fname); - - let file = match open_result { - Ok(file_handle) => file_handle, - Err(err) => { - return Err(ANNError::log_io_error(err)); - } - }; + .open(fname)?; let ring = IoUring::new(MAX_IO_CONCURRENCY as u32)?; let fd = file.as_raw_fd(); @@ -80,7 +76,7 @@ impl LinuxAlignedFileReader { unsafe { ring.submission() .push(&read) - .map_err(ANNError::log_push_error)? + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))? }; Ok(()) } @@ -117,9 +113,7 @@ impl AlignedFileReader for LinuxAlignedFileReader { // Flush the completion queue. for cqe in ring.completion() { if cqe.result() < 0 { - return Err(ANNError::log_io_error(std::io::Error::from_raw_os_error( - cqe.result(), - ))); + return Err(std::io::Error::from_raw_os_error(cqe.result()).into()); } } } diff --git a/diskann-disk/src/search/provider/aligned_file_reader/reader/windows.rs b/diskann-disk/src/search/provider/aligned_file_reader/reader/windows.rs index c54fb036a..7a8b3ba5d 100644 --- a/diskann-disk/src/search/provider/aligned_file_reader/reader/windows.rs +++ b/diskann-disk/src/search/provider/aligned_file_reader/reader/windows.rs @@ -35,20 +35,10 @@ impl WindowsAlignedFileReader { pub fn new(fname: &str) -> ANNResult { let mut io_context = IOContext::new(); tracing::debug!("Creating file handle for {}", fname); - match unsafe { FileHandle::new(fname) } { - Ok(file_handle) => io_context.file_handle = file_handle, - Err(err) => { - return Err(ANNError::log_io_error(err)); - } - } + io_context.file_handle = unsafe { FileHandle::new(fname) }?; // Create a io completion port for the file handle, later it will be used to get the completion status. - match IOCompletionPort::new(&io_context.file_handle, None, 0, 0) { - Ok(io_completion_port) => io_context.io_completion_port = io_completion_port, - Err(err) => { - return Err(ANNError::log_io_error(err)); - } - } + io_context.io_completion_port = IOCompletionPort::new(&io_context.file_handle, None, 0, 0)?; Ok(WindowsAlignedFileReader { io_context }) } @@ -77,14 +67,7 @@ impl AlignedFileReader for WindowsAlignedFileReader { let offset = req.offset(); let os = &mut overlapped_in_out[j]; - match unsafe { - read_file_to_slice(&ctx.file_handle, req.aligned_buf_mut(), os, offset) - } { - Ok(_) => {} - Err(error) => { - return Err(ANNError::log_io_error(error)); - } - } + unsafe { read_file_to_slice(&ctx.file_handle, req.aligned_buf_mut(), os, offset) }?; } let mut n_read: DWORD = 0; @@ -108,7 +91,7 @@ impl AlignedFileReader for WindowsAlignedFileReader { thread::sleep(ASYNC_IO_COMPLETION_CHECK_INTERVAL); } // An error ocurred. - Err(error) => return Err(ANNError::log_io_error(error)), + Err(error) => return Err(ANNError::from(error)), } } } diff --git a/diskann-disk/src/search/provider/disk_provider.rs b/diskann-disk/src/search/provider/disk_provider.rs index a5ebf2549..2fa906932 100644 --- a/diskann-disk/src/search/provider/disk_provider.rs +++ b/diskann-disk/src/search/provider/disk_provider.rs @@ -48,6 +48,7 @@ use tracing::debug; use crate::{ data_model::{CachingStrategy, GraphHeader}, + error::{diskann_error, ErrorKind}, search::{ provider::{ aligned_file_reader::AlignedFileReaderFactory, @@ -966,8 +967,8 @@ where // `diskann::graph::Config` and is forced to be non-zero. But this is defensive // against misconfiguration. if batch_size == 0 { - return Err(ANNError::message( - diskann::ANNErrorKind::IndexError, + return Err(diskann_error!( + ErrorKind::IndexError, "pq scratch must support at least one vector", )); } @@ -1144,7 +1145,8 @@ where .as_deref() .map_or(PostprocessStrategy::AcceptAll, PostprocessStrategy::Apply), ); - let knn_search = Knn::new(k, l, beam_width)?; + let knn_search = Knn::new(k, l, beam_width) + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; self.runtime.block_on(self.index.search( knn_search, &strategy, @@ -1230,12 +1232,8 @@ fn ensure_vertex_loaded>( mod disk_provider_tests { use crate::test_utils::{GraphDataF32VectorU32Data, GraphDataF32VectorUnitData}; use diskann::{ - graph::{ - search::{record::VisitedSearchRecord, Knn}, - KnnSearchError, - }, + graph::search::{record::VisitedSearchRecord, Knn}, utils::IntoUsize, - ANNErrorKind, }; use diskann_providers::storage::{ DynWriteProvider, StorageReadProvider, VirtualStorageProvider, @@ -1250,6 +1248,7 @@ mod disk_provider_tests { use super::*; use crate::{ build::builder::core::disk_index_builder_tests::{IndexBuildFixture, TestParams}, + error::{error_kind, ErrorKind}, search::provider::aligned_file_reader::VirtualAlignedReaderFactory, utils::QueryStatistics, }; @@ -1698,10 +1697,7 @@ mod disk_provider_tests { // Test error case: l < k let res = Knn::new_default(20, 10); assert!(res.is_err()); - assert_eq!( - >::into(res.unwrap_err()).kind(), - ANNErrorKind::IndexError - ); + // Test error case: beam_width = 0 let res = Knn::new(10, 10, Some(0)); assert!(res.is_err()); @@ -1756,7 +1752,7 @@ mod disk_provider_tests { ); assert!(result.is_err()); - assert_eq!(result.unwrap_err().kind(), ANNErrorKind::IndexError); + assert_eq!(error_kind(&result.unwrap_err()), ErrorKind::IndexError); } #[test] diff --git a/diskann-disk/src/search/provider/disk_sector_graph.rs b/diskann-disk/src/search/provider/disk_sector_graph.rs index 68a47eb92..a0b587960 100644 --- a/diskann-disk/src/search/provider/disk_sector_graph.rs +++ b/diskann-disk/src/search/provider/disk_sector_graph.rs @@ -7,11 +7,12 @@ //! Sector graph use std::ops::Deref; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_quantization::alloc::{AlignedAllocator, Poly}; use crate::{ data_model::GraphHeader, + error::{diskann_error, ErrorKind}, search::provider::aligned_file_reader::{traits::AlignedFileReader, AlignedRead, Alignment}, }; @@ -78,7 +79,7 @@ impl DiskSectorGraph { max_n_batch_sector_read * num_sectors_per_node * block_size, AlignedAllocator::new(AlignedReaderType::Alignment::VALUE), ) - .map_err(ANNError::log_index_error)?, + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?, cur_sector_idx: 0, num_nodes_per_sector, node_len, @@ -97,7 +98,7 @@ impl DiskSectorGraph { max_n_batch_sector_read * self.num_sectors_per_node * self.block_size, AlignedAllocator::new(AlignedReaderType::Alignment::VALUE), ) - .map_err(ANNError::log_index_error)?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; } Ok(()) } @@ -112,19 +113,22 @@ impl DiskSectorGraph { pub fn read_graph(&mut self, sectors_to_fetch: &[u64]) -> ANNResult<()> { let cur_sector_idx_usize: usize = self.cur_sector_idx.try_into()?; if sectors_to_fetch.len() > self.max_n_batch_sector_read - cur_sector_idx_usize { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "Trying to read too many sectors. number of sectors to read: {}, max number of sectors can read: {}", sectors_to_fetch.len(), self.max_n_batch_sector_read - cur_sector_idx_usize, - ))); + )); } let len_per_node = self.num_sectors_per_node * self.block_size; if len_per_node == 0 { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "len_per_node is 0 (num_sectors_per_node={}, block_size={})", - self.num_sectors_per_node, self.block_size, - ))); + self.num_sectors_per_node, + self.block_size, + )); } let range = cur_sector_idx_usize * len_per_node ..(cur_sector_idx_usize + sectors_to_fetch.len()) * len_per_node; diff --git a/diskann-disk/src/search/provider/disk_vertex_provider.rs b/diskann-disk/src/search/provider/disk_vertex_provider.rs index 80b701752..66c723d89 100644 --- a/diskann-disk/src/search/provider/disk_vertex_provider.rs +++ b/diskann-disk/src/search/provider/disk_vertex_provider.rs @@ -12,6 +12,7 @@ use hashbrown::HashMap; use crate::{ data_model::GraphHeader, + error::{diskann_error, ErrorKind}, search::{ provider::{ aligned_file_reader::traits::AlignedFileReader, disk_sector_graph::DiskSectorGraph, @@ -80,9 +81,9 @@ where match self.loaded_nodes.get(vertex_id) { Some(local_offset) => Ok(&self.vector_buf [local_offset.idx * self.dim..(local_offset.idx * self.dim) + self.dim]), - None => Err(ANNError::log_get_vertex_data_error( - vertex_id.to_string(), - "Vector".to_string(), + None => Err(diskann_error!( + ErrorKind::IndexError, + "vertex id {vertex_id} is out-of-bounds", )), } } @@ -93,9 +94,9 @@ where ) -> ANNResult<&[Data::VectorIdType]> { match self.loaded_nodes.get(vertex_id) { Some(local_offset) => Ok(&self.cached_adjacency_list[local_offset.vec_idx]), - None => Err(ANNError::log_get_vertex_data_error( - vertex_id.to_string(), - "AdjacencyList".to_string(), + None => Err(diskann_error!( + ErrorKind::IndexError, + "adjacency list id {vertex_id} is out-of-bounds", )), } } @@ -106,9 +107,9 @@ where ) -> ANNResult<&Data::AssociatedDataType> { match self.loaded_nodes.get(vertex_id) { Some(local_offset) => Ok(&self.cached_associated_data[local_offset.vec_idx]), - None => Err(ANNError::log_get_vertex_data_error( - vertex_id.to_string(), - "AssociatedData".to_string(), + None => Err(diskann_error!( + ErrorKind::IndexError, + "associated data id {vertex_id} is out-of-bounds", )), } } @@ -152,16 +153,14 @@ where self.cached_adjacency_list.push(adjacency_list); } None => { - return Err(ANNError::message( - diskann::ANNErrorKind::SerdeError, - format!( - "malformed length for vertex {} \ - - reported neighbors is {} ({} bytes) which exceeds the buffer length {}", - vertex_id, - num_neighbors, - 4 * num_neighbors + 4, - neighbor_and_data_buf.len() - ), + return Err(diskann_error!( + ErrorKind::SerdeError, + "malformed length for vertex {} \ + - reported neighbors is {} ({} bytes) which exceeds the buffer length {}", + vertex_id, + num_neighbors, + 4 * num_neighbors + 4, + neighbor_and_data_buf.len() )); } } @@ -171,9 +170,10 @@ where &neighbor_and_data_buf[data_end - self.associated_data_size..data_end], ) .map_err(|err| { - ANNError::log_serde_error( - "Error deserializing associated data from bytes".to_string(), - *err, + diskann_error!( + ErrorKind::SerdeError, + "Error deserializing associated data from bytes: {}", + err, ) })?; self.cached_associated_data.push(associated_data); diff --git a/diskann-disk/src/search/provider/disk_vertex_provider_factory.rs b/diskann-disk/src/search/provider/disk_vertex_provider_factory.rs index f57dc12c8..f92a2ffe4 100644 --- a/diskann-disk/src/search/provider/disk_vertex_provider_factory.rs +++ b/diskann-disk/src/search/provider/disk_vertex_provider_factory.rs @@ -5,7 +5,7 @@ use std::{cmp::min, collections::VecDeque, sync::Arc, time::Instant}; use crate::data_model::GraphDataType; -use diskann::{graph::AdjacencyList, ANNError, ANNResult}; +use diskann::{graph::AdjacencyList, ANNResult}; use diskann_quantization::{ alloc::{AlignedAllocator, Poly}, num::PowerOfTwo, @@ -15,6 +15,7 @@ use tracing::info; use crate::{ data_model::{Cache, CachingStrategy, GraphHeader}, + error::{diskann_error, ErrorKind}, search::provider::aligned_file_reader::{ traits::{AlignedFileReader, AlignedReaderFactory}, AlignedFileReaderFactory, AlignedRead, @@ -58,9 +59,12 @@ where let mut read_buf = Poly::broadcast( 0u8, buffer_len, - AlignedAllocator::new(PowerOfTwo::new(buffer_len).map_err(ANNError::log_index_error)?), + AlignedAllocator::new( + PowerOfTwo::new(buffer_len) + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?, + ), ) - .map_err(ANNError::log_index_error)?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; let aligned_read = AlignedRead::new(0_u64, &mut read_buf)?; self.aligned_reader_factory .build()? @@ -84,7 +88,8 @@ where sector_reader, cache.clone(), ), - None => Err(ANNError::log_index_error( + None => Err(diskann_error!( + ErrorKind::IndexError, "Cache must be initialised for StaticCacheWithBfsNodes caching strategy", )), }, @@ -149,9 +154,10 @@ impl, ReaderFactory: AlignedReaderFactor match self.caching_strategy { CachingStrategy::StaticCacheWithBfsNodes(mut num_nodes_to_cache) => { if num_nodes_to_cache == 0 { - ANNError::log_index_error( + return Err(diskann_error!( + ErrorKind::IndexError, "num_nodes_to_cache should be greater than 0 for StaticCacheWithBfsNodes caching strategy", - ); + )); } let graph_metadata = self.get_header()?; @@ -202,7 +208,10 @@ impl, ReaderFactory: AlignedReaderFactor let batch_size = min(queue.len(), BEAM_WIDTH_FOR_BFS); for _ in 0..batch_size { let node = queue.pop_front().ok_or_else(|| { - ANNError::log_index_error("Error while caching Nodes via BFS: Queue is empty") + diskann_error!( + ErrorKind::IndexError, + "Error while caching Nodes via BFS: Queue is empty" + ) })?; nodes_in_a_batch.push(node); } @@ -212,7 +221,11 @@ impl, ReaderFactory: AlignedReaderFactor for (idx, node) in nodes_in_a_batch.iter().enumerate() { Self::insert_in_cache(node, idx, &mut vertex_provider, &mut cache)?; let adjacency_list = cache.get_adjacency_list(node).ok_or_else(|| { - ANNError::log_index_error(format!("Error while caching Nodes via BFS: Adjacency List not found for inserted node {} in cache.", node)) + diskann_error!( + ErrorKind::IndexError, + "Error while caching Nodes via BFS: Adjacency List not found for inserted node {} in cache.", + node + ) })?; for neighbor_id in adjacency_list.iter() { if !visited.contains(neighbor_id) { diff --git a/diskann-disk/src/storage/cached_reader.rs b/diskann-disk/src/storage/cached_reader.rs index 3326d7607..79165d965 100644 --- a/diskann-disk/src/storage/cached_reader.rs +++ b/diskann-disk/src/storage/cached_reader.rs @@ -4,10 +4,12 @@ */ use std::io::{Read, Seek}; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_providers::storage::StorageReadProvider; use tracing::info; +use crate::error::{diskann_error, ErrorKind}; + /// Sequential cached reads with a generic storage provider with read access. pub struct CachedReader where @@ -76,13 +78,14 @@ where // case 2: cache contains some data let cached_bytes = self.cache_size - self.cur_off; if n_bytes - cached_bytes > self.size - self.reader.stream_position()? { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "Reading beyond end of file, n_bytes: {} cached_bytes: {} fsize: {} current pos: {}", n_bytes, cached_bytes, self.size, self.reader.stream_position()? - ))); + )); } read_buf[..cached_bytes as usize] diff --git a/diskann-disk/src/storage/disk_index_reader.rs b/diskann-disk/src/storage/disk_index_reader.rs index 2db2824d6..593fe7280 100644 --- a/diskann-disk/src/storage/disk_index_reader.rs +++ b/diskann-disk/src/storage/disk_index_reader.rs @@ -67,7 +67,6 @@ impl DiskIndexReader { #[cfg(test)] mod disk_index_storage_test { - use diskann::ANNErrorKind; use diskann_providers::storage::VirtualStorageProvider; use diskann_utils::test_data_root; use vfs::OverlayFS; @@ -103,7 +102,6 @@ mod disk_index_storage_test { Ok(_) => panic!("this function should not have succeeded"), Err(err) => err, }; - assert_eq!(err.kind(), ANNErrorKind::PQError); assert!(err.to_string().contains("PQ k-means pivot file not found")); } diff --git a/diskann-disk/src/storage/disk_index_writer.rs b/diskann-disk/src/storage/disk_index_writer.rs index 7c26180f2..2467c3cdf 100644 --- a/diskann-disk/src/storage/disk_index_writer.rs +++ b/diskann-disk/src/storage/disk_index_writer.rs @@ -8,7 +8,7 @@ use std::{ use crate::data_model::GraphDataType; use byteorder::{ByteOrder, LittleEndian, ReadBytesExt}; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_providers::storage::{StorageReadProvider, StorageWriteProvider}; use diskann_providers::{ storage::{get_mem_index_file, path_utility::*}, @@ -18,6 +18,7 @@ use tracing::info; use crate::{ data_model::{GraphHeader, GraphMetadata}, + error::{diskann_error, ErrorKind}, storage::{CachedReader, CachedWriter}, }; @@ -99,12 +100,10 @@ impl DiskIndexWriter { block_size: usize, ) -> ANNResult { if block_size < GraphMetadata::get_size() { - return Err(ANNError::log_index_config_error( - "index_block_size".to_string(), - format!( - "block_size should be greater than the size of GraphMetadata: {}", - GraphMetadata::get_size() - ), + return Err(diskann_error!( + ErrorKind::IndexConfigError("index_block_size"), + "block_size should be greater than the size of GraphMetadata: {}", + GraphMetadata::get_size() )); } @@ -138,7 +137,10 @@ impl DiskIndexWriter { if let Some(vamana_reader) = state.muti_shard_index_reader.as_mut() { num_nbrs = vamana_reader.read_u32::()?; } else { - return Err(ANNError::log_index_error("invalid index reader")); + return Err(diskann_error!( + ErrorKind::IndexError, + "invalid index reader" + )); } Ok(num_nbrs) @@ -154,7 +156,10 @@ impl DiskIndexWriter { if let Some(vamana_reader) = state.muti_shard_index_reader.as_mut() { vamana_reader.read_exact(nbrs_buf)?; } else { - return Err(ANNError::log_index_error("invalid index reader")); + return Err(diskann_error!( + ErrorKind::IndexError, + "invalid index reader" + )); } Ok(()) @@ -193,7 +198,10 @@ impl DiskIndexWriter { return Ok(()); } - Err(ANNError::log_index_error("invalid index reader")) + Err(diskann_error!( + ErrorKind::IndexError, + "invalid index reader" + )) } fn open_associated_data_reader( @@ -218,10 +226,11 @@ impl DiskIndexWriter { let length = associated_data_reader.read_u32()? as usize; if state.num_pts != associated_data_num_pts { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "Number of points in dataset file ({}) does not match number of points in associated data file ({}).", state.num_pts, associated_data_num_pts - ))); + )); } (Option::Some(associated_data_reader), length) diff --git a/diskann-disk/src/storage/quant/generator.rs b/diskann-disk/src/storage/quant/generator.rs index 7c09c1182..2593f9bf3 100644 --- a/diskann-disk/src/storage/quant/generator.rs +++ b/diskann-disk/src/storage/quant/generator.rs @@ -9,7 +9,7 @@ use std::{ time::Instant, }; -use diskann::{error::IntoANNResult, utils::VectorRepr, ANNError, ANNResult}; +use diskann::{error::IntoANNResult, utils::VectorRepr, ANNResult}; use diskann_providers::{ storage::{StorageReadProvider, StorageWriteProvider}, utils::{load_metadata_from_file, BridgeErr, ParallelIteratorInPool, RayonThreadPoolRef}, @@ -18,7 +18,10 @@ use diskann_utils::{io::Metadata, views}; use rayon::iter::IndexedParallelIterator; use tracing::info; -use crate::storage::quant::compressor::QuantCompressor; +use crate::{ + error::{diskann_error, ErrorKind}, + storage::quant::compressor::QuantCompressor, +}; /// [`QuantDataGenerator`] orchestrates the process of reading vector data, applying quantization, /// and writing compressed results to storage in batches. @@ -78,12 +81,14 @@ where let metadata = load_metadata_from_file(storage_provider, &self.data_path)?; let (num_points, dim) = metadata.into_dims(); if max_block_size == 0 { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Data compression chunk vector count must be greater than zero", )); } if num_points == 0 { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Cannot generate compressed data for an empty dataset", )); } diff --git a/diskann-disk/src/storage/quant/pq/pq_dataset.rs b/diskann-disk/src/storage/quant/pq/pq_dataset.rs index 5dc31e3b3..adf51c433 100644 --- a/diskann-disk/src/storage/quant/pq/pq_dataset.rs +++ b/diskann-disk/src/storage/quant/pq/pq_dataset.rs @@ -5,11 +5,13 @@ use core::fmt::Debug; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_providers::model::FixedChunkPQTable; -use diskann_quantization::product::TransposedTable; +use diskann_quantization::{error::Format, product::TransposedTable}; use diskann_utils::views::Matrix; +use crate::error::{diskann_error, ErrorKind}; + #[derive(Debug)] pub struct PQData { // pq pivot table. @@ -28,7 +30,7 @@ impl PQData { pq_pivot_table.view_pivots(), pq_pivot_table.view_offsets().to_owned(), ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))?; + .map_err(|err| diskann_error!(ErrorKind::PQError, "{}", Format(err)))?; Ok(Self { pq_pivot_table, @@ -64,7 +66,10 @@ impl PQData { // Get compressed vector with the given vector id from the pq_compressed_data. pub fn get_compressed_vector(&self, vector_id: usize) -> ANNResult<&[u8]> { self.pq_compressed_data.get_row(vector_id).ok_or_else(|| { - ANNError::log_index_error("Vector id is out of boundary in the compressed dataset.") + diskann_error!( + ErrorKind::IndexError, + "Vector id is out of boundary in the compressed dataset." + ) }) } } diff --git a/diskann-disk/src/storage/quant/pq/pq_generation.rs b/diskann-disk/src/storage/quant/pq/pq_generation.rs index b1eae30c3..c5297ae9c 100644 --- a/diskann-disk/src/storage/quant/pq/pq_generation.rs +++ b/diskann-disk/src/storage/quant/pq/pq_generation.rs @@ -5,7 +5,7 @@ use std::{marker::PhantomData, time::Instant}; -use diskann::{utils::VectorRepr, ANNError}; +use diskann::utils::VectorRepr; use diskann_providers::storage::{StorageReadProvider, StorageWriteProvider}; use diskann_providers::{ model::{ @@ -15,12 +15,15 @@ use diskann_providers::{ storage::PQStorage, utils::{BridgeErr, RayonThreadPoolRef}, }; -use diskann_quantization::{product::TransposedTable, CompressInto}; +use diskann_quantization::{error::Format, product::TransposedTable, CompressInto}; use diskann_utils::views::MatrixBase; use diskann_vector::distance::Metric; use tracing::info; -use crate::storage::quant::compressor::QuantCompressor; +use crate::{ + error::{diskann_error, ErrorKind}, + storage::quant::compressor::QuantCompressor, +}; pub struct PQGenerationContext<'a, Storage> where @@ -59,7 +62,8 @@ where fn new(context: &Self::CompressorContext) -> diskann::ANNResult { // validate that the number of chunks is correct. if context.num_chunks > context.dim { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Error: number of chunks more than dimension.", )); } @@ -134,7 +138,7 @@ where .bridge_err()? .to_owned(), ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))?; + .map_err(|err| diskann_error!(ErrorKind::PQError, "{}", Format(err)))?; Ok(Self { table, @@ -151,7 +155,7 @@ where ) -> Result<(), diskann::ANNError> { self.table .compress_into(vector, output) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err))) + .map_err(|err| diskann_error!(ErrorKind::PQError, "{}", Format(err))) } fn compressed_bytes(&self) -> usize { diff --git a/diskann-disk/src/utils/kmeans.rs b/diskann-disk/src/utils/kmeans.rs index 7ee6ba384..b66ee65d1 100644 --- a/diskann-disk/src/utils/kmeans.rs +++ b/diskann-disk/src/utils/kmeans.rs @@ -11,7 +11,7 @@ use std::cmp::min; -use diskann::{ANNError, ANNResult}; +use diskann::ANNResult; use diskann_providers::utils::{ParallelIteratorInPool, RayonThreadPoolRef}; use diskann_vector::{distance::SquaredL2, PureDistanceFunction}; use hashbrown::HashSet; @@ -24,6 +24,8 @@ use rayon::prelude::*; use super::math_util::{compute_closest_centers, compute_vecs_l2sq}; +use crate::error::{diskann_error, ErrorKind}; + /// Run Lloyds one iteration /// Given data in row-major num_points * dim, and centers in row-major /// num_centers * dim and squared lengths of ata points, output the closest @@ -138,7 +140,8 @@ pub fn run_lloyds( for i in 0..max_reps { if *cancellation_token { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Error: Cancellation requested by caller.", )); } @@ -178,18 +181,21 @@ fn select_random_pivots( rng: &mut impl Rng, ) -> ANNResult<()> { if num_points < num_centers { - return Err(ANNError::log_kmeans_error(format!( + return Err(diskann_error!( + ErrorKind::KMeansError, "Number of points {} is less than number of centers {}", - num_points, num_centers - ))); + num_points, + num_centers + )); } if pivot_data.len() != num_centers * dim { - return Err(ANNError::log_kmeans_error(format!( + return Err(diskann_error!( + ErrorKind::KMeansError, "Pivot data buffer should be of size num_centers * dim = {} * {} = {}", num_centers, dim, num_centers * dim - ))); + )); } let mut picked = HashSet::new(); @@ -238,23 +244,26 @@ pub fn k_meanspp_selecting_pivots( pool: RayonThreadPoolRef<'_>, ) -> ANNResult<()> { if num_points > (1 << 23) { - return Err(ANNError::log_kmeans_error(format!( + return Err(diskann_error!( + ErrorKind::KMeansError, "Number of points {} is greater than 8388608, and k-means++ can not process this. Try selecting_random_pivots instead.", num_points - ))); + )); } if pivot_data.len() != num_centers * dim { - return Err(ANNError::log_kmeans_error(format!( + return Err(diskann_error!( + ErrorKind::KMeansError, "Pivot data buffer should be of size num_centers * dim = {} * {} = {}", num_centers, dim, num_centers * dim - ))); + )); } if *cancellation_token { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Error: Cancellation requested by caller.", )); } @@ -264,7 +273,7 @@ pub fn k_meanspp_selecting_pivots( let real_distribution = StandardUniform; let int_distribution = Uniform::new(0, num_points) - .map_err(|_| ANNError::log_kmeans_error("cannot cluster an empty dataset".into()))?; + .map_err(|_| diskann_error!(ErrorKind::KMeansError, "cannot cluster an empty dataset"))?; // Randomly select a node as the first pivot. let init_id = int_distribution.sample(rng); @@ -291,7 +300,8 @@ pub fn k_meanspp_selecting_pivots( // At the end of the loop we should have num_centers pivots. for _ in 1..num_centers { if *cancellation_token { - return Err(ANNError::log_pq_error( + return Err(diskann_error!( + ErrorKind::PQError, "Error: Cancellation requested by caller.", )); } @@ -329,8 +339,8 @@ pub fn k_meanspp_selecting_pivots( || (dart_val <= prefix_sum && *pivot_dist != 0.0f32)) { if picked.contains(&i) { - return Err(ANNError::log_kmeans_error( - "A pivot was sampled again, the condition on dart_val range should not have happened".to_string(), + return Err(diskann_error!(ErrorKind::KMeansError, + "A pivot was sampled again, the condition on dart_val range should not have happened", )); } picked.insert(i); @@ -341,18 +351,19 @@ pub fn k_meanspp_selecting_pivots( prefix_sum += *pivot_dist as f64; } if prefix_sum > sum { - return Err(ANNError::log_kmeans_error( + return Err(diskann_error!( + ErrorKind::KMeansError, "Prefix sum should not be greater than sum. If the for loop above ran to conclusion without break, prefix_sum should be equal to sum" - .to_string(), )); } // We should have picked a pivot in this loop. // If not, there is a corner condition we might have missed and we should fix this function. if picked_pivot_id == num_points { - return Err(ANNError::log_kmeans_error( - "Did not pick a pivot in this loop".to_string(), + return Err(diskann_error!( + ErrorKind::KMeansError, + "Did not pick a pivot in this loop", )); } @@ -427,7 +438,6 @@ pub fn k_means_clustering( #[cfg(test)] mod kmeans_test { use approx::assert_relative_eq; - use diskann::ANNErrorKind; use diskann_providers::{ storage::{StorageReadProvider, VirtualStorageProvider}, utils::{ @@ -438,6 +448,8 @@ mod kmeans_test { use diskann_utils::test_data_root; use rstest::rstest; + use crate::error::{error_kind, ErrorKind}; + use super::*; #[test] @@ -555,7 +567,7 @@ mod kmeans_test { ) .unwrap_err(); - assert_eq!(err.kind(), ANNErrorKind::PQError); + assert_eq!(error_kind(&err), ErrorKind::PQError); assert!(err .to_string() .contains("Error: Cancellation requested by caller.")); @@ -666,7 +678,7 @@ mod kmeans_test { ) .unwrap_err(); - assert_eq!(err.kind(), ANNErrorKind::KMeansError); + assert_eq!(error_kind(&err), ErrorKind::KMeansError); assert!(err.to_string().contains(expected_error_message)); } @@ -850,7 +862,7 @@ mod kmeans_test { let use_correct_buffer_size = true; let cancellation_token = &mut false; - let expected_error_type = ANNErrorKind::KMeansError; + let expected_error_type = ErrorKind::KMeansError; let expected_error_message = "Number of points 8388609 is greater than 8388608, and k-means++ can not process this."; @@ -873,7 +885,7 @@ mod kmeans_test { let use_correct_buffer_size = false; // Buffer size is 1 less than required let cancellation_token = &mut false; - let expected_error_type = ANNErrorKind::KMeansError; + let expected_error_type = ErrorKind::KMeansError; let expected_error_message = "Pivot data buffer should be of size num_centers * dim = 3 * 2 = 6"; @@ -896,7 +908,7 @@ mod kmeans_test { let use_correct_buffer_size = true; let cancellation_token = &mut true; - let expected_error_type = ANNErrorKind::PQError; + let expected_error_type = ErrorKind::PQError; let expected_error_message = "Error: Cancellation requested by caller."; k_meanspp_selecting_pivots_test_error_internal( @@ -916,7 +928,7 @@ mod kmeans_test { num_centers: usize, use_correct_buffer_size: bool, cancellation_token: &mut bool, - expected_error_type: ANNErrorKind, + expected_error_type: ErrorKind, expected_error_message: &str, ) { // Generate some random data points @@ -942,7 +954,7 @@ mod kmeans_test { ) .unwrap_err(); - assert_eq!(err.kind(), expected_error_type); + assert_eq!(error_kind(&err), expected_error_type); assert!(err.to_string().contains(expected_error_message)); } @@ -1056,7 +1068,7 @@ mod kmeans_test { ) .unwrap_err(); - assert_eq!(err.kind(), ANNErrorKind::PQError); + assert_eq!(error_kind(&err), ErrorKind::PQError); assert!(err.to_string().contains("Cancellation requested by caller")); } } diff --git a/diskann-disk/src/utils/math_util.rs b/diskann-disk/src/utils/math_util.rs index 16cfe612a..30b773108 100644 --- a/diskann-disk/src/utils/math_util.rs +++ b/diskann-disk/src/utils/math_util.rs @@ -12,12 +12,14 @@ use std::{cmp::Ordering, collections::BinaryHeap}; -use diskann::{ANNError, ANNErrorKind, ANNResult}; +use diskann::ANNResult; use diskann_linalg::{self, Transpose}; use diskann_providers::utils::{ParallelIteratorInPool, RayonThreadPoolRef}; use diskann_vector::{norm::FastL2NormSquared, Norm}; use rayon::prelude::*; +use crate::error::{diskann_error, ErrorKind}; + // This is the chunk size applied when computing the closest centers in a block. // The chunk size is the number of points to process in a single iteration to reduce memory usage of // distance_matrix. @@ -83,26 +85,29 @@ pub fn compute_vecs_l2sq( pool: RayonThreadPoolRef<'_>, ) -> ANNResult<()> { if dim == 0 { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "dim must be non-zero" - ))); + )); } let expected_data_len = vecs_l2sq.len().checked_mul(dim).ok_or_else(|| { - ANNError::log_index_error(format_args!( + diskann_error!( + ErrorKind::IndexError, "vecs_l2sq.len() * dim overflowed: vecs_l2sq.len() ({}) * dim ({})", vecs_l2sq.len(), dim - )) + ) })?; if data.len() != expected_data_len { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "data.len() ({}) should be vecs_l2sq.len() ({}) * dim ({})", data.len(), vecs_l2sq.len(), dim - ))); + )); } if dim < 5 { @@ -144,10 +149,12 @@ pub fn compute_closest_centers_in_block( pool: RayonThreadPoolRef<'_>, ) -> ANNResult<()> { if k > num_centers { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "k ({}) should be equal or less than num_centers ({})", - k, num_centers - ))); + k, + num_centers + )); } let ones_a: Vec = vec![1.0; num_centers]; @@ -165,7 +172,7 @@ pub fn compute_closest_centers_in_block( None, // Initialize the destination matrix dist_matrix, ) - .map_err(|e| ANNError::new(ANNErrorKind::IndexError, e))?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; diskann_linalg::sgemm( Transpose::None, @@ -179,7 +186,7 @@ pub fn compute_closest_centers_in_block( Some(1.0), // Add to the destination matrix dist_matrix, ) - .map_err(|e| ANNError::new(ANNErrorKind::IndexError, e))?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; diskann_linalg::sgemm( Transpose::None, @@ -193,7 +200,7 @@ pub fn compute_closest_centers_in_block( Some(1.0), // Add to the destination matrix. dist_matrix, ) - .map_err(|e| ANNError::new(ANNErrorKind::IndexError, e))?; + .map_err(|e| diskann_error!(ErrorKind::IndexError, e))?; if k == 1 { center_index @@ -260,70 +267,82 @@ pub fn compute_closest_centers( pool: RayonThreadPoolRef<'_>, ) -> ANNResult<()> { if k > num_centers { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "k ({}) should be equal or less than num_centers ({})", - k, num_centers - ))); + k, + num_centers + )); } // Validate data slice length let expected_data_len = num_points.checked_mul(dim).ok_or_else(|| { - ANNError::log_index_error(format_args!( + diskann_error!( + ErrorKind::IndexError, "num_points * dim overflowed: num_points ({}) * dim ({})", - num_points, dim - )) + num_points, + dim + ) })?; if data.len() != expected_data_len { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "data.len() ({}) should equal num_points ({}) * dim ({})", data.len(), num_points, dim - ))); + )); } // Validate pivot_data slice length let expected_pivot_len = num_centers.checked_mul(dim).ok_or_else(|| { - ANNError::log_index_error(format_args!( + diskann_error!( + ErrorKind::IndexError, "num_centers * dim overflowed: num_centers ({}) * dim ({})", - num_centers, dim - )) + num_centers, + dim + ) })?; if pivot_data.len() != expected_pivot_len { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "pivot_data.len() ({}) should equal num_centers ({}) * dim ({})", pivot_data.len(), num_centers, dim - ))); + )); } let expected_closest_centers_len = num_points.checked_mul(k).ok_or_else(|| { - ANNError::log_index_error(format_args!( + diskann_error!( + ErrorKind::IndexError, "num_points * k overflowed: num_points ({}) * k ({})", - num_points, k - )) + num_points, + k + ) })?; if closest_centers_ivf.len() != expected_closest_centers_len { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "closest_centers_ivf.len() ({}) should equal num_points ({}) * k ({})", closest_centers_ivf.len(), num_points, k - ))); + )); } let mut owned_pts_norms_squared; let pts_norms_squared: &[f32] = if let Some(pts_norms) = pts_norms_squared { if pts_norms.len() != num_points { - return Err(ANNError::log_index_error(format_args!( + return Err(diskann_error!( + ErrorKind::IndexError, "pts_norms_squared.len() ({}) should equal num_points ({})", pts_norms.len(), num_points - ))); + )); } pts_norms } else { diff --git a/diskann-disk/src/utils/partition.rs b/diskann-disk/src/utils/partition.rs index a8556f5d5..943acd561 100644 --- a/diskann-disk/src/utils/partition.rs +++ b/diskann-disk/src/utils/partition.rs @@ -2,7 +2,7 @@ * Copyright (c) Microsoft Corporation. * Licensed under the MIT license. */ -use diskann::{error::IntoANNResult, utils::VectorRepr, ANNError, ANNResult}; +use diskann::{error::IntoANNResult, utils::VectorRepr, ANNResult}; use diskann_providers::storage::{StorageReadProvider, StorageWriteProvider}; use diskann_providers::utils::{gen_random_slice, RayonThreadPoolRef, READ_WRITE_BLOCK_SIZE}; @@ -12,6 +12,7 @@ use tracing::info; use crate::{ disk_index_build_parameter::BYTES_IN_GB, + error::{diskann_error, ErrorKind}, storage::{CachedReader, CachedWriter, DiskIndexWriter}, }; @@ -254,7 +255,8 @@ where let num_points = dataset_reader.read_u32()?; let base_dim = dataset_reader.read_u32()?; if base_dim != dim as u32 { - return Err(ANNError::log_index_error( + return Err(diskann_error!( + ErrorKind::IndexError, "dimensions dont match for train set and base set", )); } diff --git a/diskann-garnet/src/provider.rs b/diskann-garnet/src/provider.rs index 3468c2c90..6b2db9e15 100644 --- a/diskann-garnet/src/provider.rs +++ b/diskann-garnet/src/provider.rs @@ -5,7 +5,7 @@ use dashmap::DashMap; use diskann::{ - ANNError, ANNErrorKind, ANNResult, default_post_processor, + ANNError, ANNResult, default_post_processor, graph::{ AdjacencyList, SearchOutputBuffer, config::defaults::MAX_OCCLUSION_SIZE, @@ -107,13 +107,7 @@ pub(crate) enum GarnetProviderError { PostProcessing(Box), } -impl From for ANNError { - #[track_caller] - fn from(value: GarnetProviderError) -> Self { - ANNError::new(ANNErrorKind::GetVertexDataError, value) - } -} - +diskann::convert_error!(GarnetProviderError); diskann::always_escalate!(GarnetProviderError); /// The Garnet DataProvider implementation. diff --git a/diskann-inmem/src/epoch.rs b/diskann-inmem/src/epoch.rs index 9ecd46cba..65bfc5a21 100644 --- a/diskann-inmem/src/epoch.rs +++ b/diskann-inmem/src/epoch.rs @@ -439,7 +439,7 @@ impl std::fmt::Display for Unavailable { impl std::error::Error for Unavailable {} -crate::opaque!(Unavailable); +diskann::convert_error!(Unavailable); // Delays // diff --git a/diskann-inmem/src/ids.rs b/diskann-inmem/src/ids.rs index d4e549fd2..dcf41424a 100644 --- a/diskann-inmem/src/ids.rs +++ b/diskann-inmem/src/ids.rs @@ -174,7 +174,7 @@ pub(crate) enum InsertError { InternalExists, } -crate::opaque!(InsertError); +diskann::convert_error!(InsertError); /// A handle to a valid entry in a [`IdMap`]. /// diff --git a/diskann-inmem/src/layers/full.rs b/diskann-inmem/src/layers/full.rs index a2d36c48e..9cdd94c52 100644 --- a/diskann-inmem/src/layers/full.rs +++ b/diskann-inmem/src/layers/full.rs @@ -145,7 +145,7 @@ enum SetError { Bytes { got: usize, expected: usize }, } -crate::opaque!(SetError); +diskann::convert_error!(SetError); impl layers::AsDistance for Full where @@ -209,7 +209,7 @@ where ylen: y.len(), }; - Err(ANNError::opaque(error)) + Err(ANNError::new(error)) } fn dim(&self) -> usize { @@ -310,7 +310,7 @@ impl<'a, T, U, D> QueryDistance<'a, T, U, D> { xlen: len, }; - Err(ANNError::opaque(error)) + Err(ANNError::new(error)) } } @@ -346,7 +346,7 @@ struct QueryDistanceError { xlen: usize, } -crate::opaque!(QueryDistanceError); +diskann::convert_error!(QueryDistanceError); macro_rules! mint { ($query:ident, $visitor:ident, $T:ty => { $N:literal, $f:ident }) => {{ diff --git a/diskann-inmem/src/lib.rs b/diskann-inmem/src/lib.rs index f04769de8..8f0570dc3 100644 --- a/diskann-inmem/src/lib.rs +++ b/diskann-inmem/src/lib.rs @@ -47,17 +47,3 @@ impl Hidden { Self(()) } } - -macro_rules! opaque { - ($T:ty) => { - impl From<$T> for diskann::ANNError { - #[track_caller] - #[cold] - fn from(err: $T) -> diskann::ANNError { - diskann::ANNError::opaque(err) - } - } - }; -} - -pub(crate) use opaque; diff --git a/diskann-inmem/src/neighbors.rs b/diskann-inmem/src/neighbors.rs index e61256e26..88f9b786a 100644 --- a/diskann-inmem/src/neighbors.rs +++ b/diskann-inmem/src/neighbors.rs @@ -255,7 +255,7 @@ pub(crate) enum NeighborsError { #[error("index {} is out-of-bounds", self.0)] pub(crate) struct OutOfBounds(u32); -crate::opaque!(OutOfBounds); +diskann::convert_error!(OutOfBounds); /// A neighbor list was longer than the configured per-list capacity. /// @@ -268,7 +268,7 @@ pub(crate) struct TooLong { max: u32, } -crate::opaque!(TooLong); +diskann::convert_error!(TooLong); /// Errors during [`Neighbors::set`]. #[derive(Debug, Clone, Copy, Error)] @@ -282,7 +282,7 @@ pub(crate) enum SetError { TooLong(TooLong), } -crate::opaque!(SetError); +diskann::convert_error!(SetError); /// A locked adjacency list to implement atomic read-modify-write operations. /// diff --git a/diskann-inmem/src/provider.rs b/diskann-inmem/src/provider.rs index 616727618..84048610b 100644 --- a/diskann-inmem/src/provider.rs +++ b/diskann-inmem/src/provider.rs @@ -36,7 +36,7 @@ use std::{ }; use diskann::{ - ANNError, ANNErrorKind, ANNResult, + ANNError, ANNResult, graph::{ AdjacencyList, SearchOutputBuffer, glue::{self, HybridPredicate}, @@ -244,7 +244,7 @@ where ) -> Result { match self.mapping.to_internal(gid) { Some(id) => Ok(id), - None => Err(ANNError::message(ANNErrorKind::Opaque, "no mapping")), + None => Err(ANNError::message("no mapping")), } } @@ -256,7 +256,7 @@ where ) -> Result { match self.mapping.to_external(id) { Some(gid) => Ok(gid), - None => Err(ANNError::message(ANNErrorKind::Opaque, "no mapping")), + None => Err(ANNError::message("no mapping")), } } } @@ -278,10 +278,7 @@ where // This ensures both either succeed or are aborted. let entry = match self.mapping.occupied_entry(gid.clone()) { None => { - return Err(ANNError::message( - ANNErrorKind::Opaque, - "id already deleted", - )); + return Err(ANNError::message("id already deleted")); } Some(e) => e, }; @@ -292,7 +289,7 @@ where entry.delete(); Ok(()) } - Err(err) => Err(ANNError::opaque(err)), + Err(err) => Err(ANNError::new(err)), } } @@ -310,10 +307,7 @@ where match self.store.can_read_approximate(id.into_usize()) { Some(true) => Ok(diskann::provider::ElementStatus::Valid), Some(false) => Ok(diskann::provider::ElementStatus::Deleted), - None => Err(ANNError::message( - ANNErrorKind::Opaque, - "accessed invalid internal ID", - )), + None => Err(ANNError::message("accessed invalid internal ID")), } } @@ -351,9 +345,10 @@ where element: T, ) -> impl std::future::Future> + Send { let work = move || { - let mut slot = self.store.acquire().ok_or_else(|| { - ANNError::message(ANNErrorKind::Opaque, "could not allocate a new slot") - })?; + let mut slot = self + .store + .acquire() + .ok_or_else(|| ANNError::message("could not allocate a new slot"))?; // TODO: Proper cleanup via `Guard` or some other mechanism on the event of // insert failure after `set_element` returns. @@ -428,10 +423,7 @@ impl glue::SearchAccessor for SearchAccessor<'_> { f(p, self.expand_beam.evaluate(point)?); } None => { - return Err(ANNError::message( - ANNErrorKind::Opaque, - "could not retrieve start point", - )); + return Err(ANNError::message("could not retrieve start point")); } } } @@ -934,7 +926,7 @@ where // By construction - the downcast should succeed. Otherwise, this is a program bug. let provider = match accessor.provider.downcast_ref::>() { Some(provider) => provider, - None => return Err(ANNError::message(ANNErrorKind::Opaque, "bad any cast")), + None => return Err(ANNError::message("bad any cast")), }; let mut count = 0; @@ -1033,10 +1025,7 @@ where let data = match reader.read(id.into_usize()) { Some(data) => data, None => { - return Err(ANNError::message( - ANNErrorKind::Opaque, - "item could not be read", - )); + return Err(ANNError::message("item could not be read")); } }; diff --git a/diskann-label-filter/src/encoded_attribute_provider/ast_label_id_mapper.rs b/diskann-label-filter/src/encoded_attribute_provider/ast_label_id_mapper.rs index 0fa21cc02..13935f109 100644 --- a/diskann-label-filter/src/encoded_attribute_provider/ast_label_id_mapper.rs +++ b/diskann-label-filter/src/encoded_attribute_provider/ast_label_id_mapper.rs @@ -4,7 +4,7 @@ */ use crate::{parser::ast::ASTVisitor, ASTExpr, CompareOp}; -use diskann::{ANNError, ANNErrorKind, ANNResult}; +use diskann::{ANNError, ANNResult}; use std::sync::{Arc, RwLock}; use crate::{ @@ -39,13 +39,10 @@ impl ASTLabelIdMapper { ) -> ANNResult> { match encoder.get(attribute) { Some(attribute_id) => Ok(ASTIdExpr::Terminal(attribute_id)), - None => Err(ANNError::message( - ANNErrorKind::Opaque, - format!( - "{}+{} present in the query does not exist in the dataset.", - field, op - ), - )), + None => Err(ANNError::message(format!( + "{}+{} present in the query does not exist in the dataset.", + field, op + ))), } } } @@ -93,7 +90,7 @@ impl ASTVisitor for ASTLabelIdMapper { CompareOp::Eq(value) => match Attribute::from_json_value(field, value) { Ok(v) => Some(v), Err(json_e) => { - return Err(ANNError::new(ANNErrorKind::Opaque, json_e)); + return Err(ANNError::new(json_e)); } }, CompareOp::Ne(_value) => { @@ -127,13 +124,10 @@ impl ASTVisitor for ASTLabelIdMapper { } } } else { - Err(ANNError::message( - ANNErrorKind::Opaque, - format!( - "CompareOp {} is not supported in the mapped filter search scenario.", - op - ), - )) + Err(ANNError::message(format!( + "CompareOp {} is not supported in the mapped filter search scenario.", + op + ))) } } } @@ -147,7 +141,6 @@ mod tests { parser::ast::ASTVisitor, ASTExpr, CompareOp, }; - use diskann::ANNErrorKind; use serde_json::Value; use std::sync::{Arc, RwLock}; @@ -228,7 +221,6 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), ANNErrorKind::Opaque); assert!(error.to_string().contains("does not exist in the dataset")); } @@ -246,7 +238,6 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), ANNErrorKind::Opaque); assert!(error.to_string().contains("does not exist in the dataset")); } @@ -484,7 +475,6 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), ANNErrorKind::Opaque); assert!(error.to_string().contains("does not exist in the dataset")); } @@ -508,7 +498,6 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - assert_eq!(error.kind(), ANNErrorKind::Opaque); assert!(error.to_string().contains("does not exist in the dataset")); } diff --git a/diskann-label-filter/src/encoded_attribute_provider/document_provider.rs b/diskann-label-filter/src/encoded_attribute_provider/document_provider.rs index 02634a0ce..e60b5bc15 100644 --- a/diskann-label-filter/src/encoded_attribute_provider/document_provider.rs +++ b/diskann-label-filter/src/encoded_attribute_provider/document_provider.rs @@ -6,10 +6,10 @@ use diskann::{ error::{ErrorExt, IntoANNResult}, provider::{DataProvider, Delete, Guard, SetElement}, - ANNError, ANNErrorKind, + ANNError, }; -use diskann_utils::future::AsyncFriendly; +use diskann_utils::{future::AsyncFriendly, lazy_format}; use crate::{document::Document, traits::attribute_store::AttributeStore}; @@ -122,10 +122,7 @@ where id: Self::InternalId, ) -> impl std::future::Future> + Send { let _ = self.attribute_store.delete(&(id)).map_err(|_| { - ANNError::message( - ANNErrorKind::IndexError, - format!("Could not delete attributes of {}.", id), - ) + ANNError::message(lazy_format!(move, "Could not delete attributes of {}.", id)) }); self.inner_provider.release(context, id) @@ -137,30 +134,23 @@ where id: Self::InternalId, ) -> Result { let is_id_in_attr_store_w = self.attribute_store.id_exists(&id).map_err(|e| { - ANNError::new(ANNErrorKind::IndexError, e) - .context("Failed to get attribute status by internal id.") + ANNError::new(e).context("Failed to get attribute status by internal id.") }); let id_in_data_store_w = self .inner_provider .status_by_internal_id(context, id) .await .into_ann_result(); - // .map_err(|e| { - // ANNError::new(ANNErrorKind::IndexError, e) - // .context("Failed to get status from data provider.") - // }); let is_id_in_attr_store = is_id_in_attr_store_w?; let id_in_data_store = id_in_data_store_w?; if is_id_in_attr_store && id_in_data_store.is_deleted() { Err(ANNError::message( - ANNErrorKind::IndexError, "Id was found in the attribute store, but not in the data store.", )) } else if !is_id_in_attr_store && id_in_data_store.is_valid() { Err(ANNError::message( - ANNErrorKind::IndexError, "Id was found in the data store but not in the attribute store.", )) } else { diff --git a/diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs b/diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs index 3b62c35ec..b315aaf30 100644 --- a/diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs +++ b/diskann-label-filter/src/encoded_attribute_provider/roaring_attribute_store.rs @@ -13,7 +13,7 @@ use crate::{ }; use diskann::{ utils::{IntoUsize, VectorId}, - ANNError, ANNErrorKind, ANNResult, + ANNError, ANNResult, }; use diskann_utils::future::AsyncFriendly; use std::sync::{Arc, RwLock}; @@ -77,18 +77,14 @@ where let mut deleted = true; // Acquire locks in consistent order: index first, then inv_index - let mut index_guard = self.index.write().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire write lock on index", - ) - })?; - let mut inv_index_guard = self.inv_index.write().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire write lock on inv_index", - ) - })?; + let mut index_guard = self + .index + .write() + .map_err(|_| ANNError::message("Failed to acquire write lock on index"))?; + let mut inv_index_guard = self + .inv_index + .write() + .map_err(|_| ANNError::message("Failed to acquire write lock on inv_index"))?; let existing_set = match index_guard.get(vec_id)? { Some(set) => set, @@ -107,7 +103,6 @@ where } if !deleted { return Err(ANNError::message( - ANNErrorKind::IndexError, "Failed to delete id from the inverted index.", )); } @@ -117,20 +112,15 @@ where if deleted { Ok(true) } else { - Err(ANNError::message( - ANNErrorKind::IndexError, - "Failed to delete id from the index.", - )) + Err(ANNError::message("Failed to delete id from the index.")) } } fn id_exists(&self, vec_id: &IT) -> ANNResult { - let index_guard = self.index.read().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire read lock on the label index.", - ) - })?; + let index_guard = self + .index + .read() + .map_err(|_| ANNError::message("Failed to acquire read lock on the label index."))?; index_guard.exists(vec_id) } @@ -143,30 +133,23 @@ where //For now, we assume that it is an error if a point has zero attributes. if attributes.is_empty() { return Err(ANNError::message( - ANNErrorKind::Opaque, "A vector must have atleast one attribute.", )); } // Acquire locks in consistent order: attribute_map, index, inv_index - let mut attr_map_guard = self.attribute_map.write().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire write lock on attribute_map", - ) - })?; - let mut index_guard = self.index.write().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire write lock on index", - ) - })?; - let mut inv_index_guard = self.inv_index.write().map_err(|_| { - ANNError::message( - ANNErrorKind::LockPoisonError, - "Failed to acquire write lock on inv_index", - ) - })?; + let mut attr_map_guard = self + .attribute_map + .write() + .map_err(|_| ANNError::message("Failed to acquire write lock on attribute_map"))?; + let mut index_guard = self + .index + .write() + .map_err(|_| ANNError::message("Failed to acquire write lock on index"))?; + let mut inv_index_guard = self + .inv_index + .write() + .map_err(|_| ANNError::message("Failed to acquire write lock on inv_index"))?; // Update the inverted index. // Delete all instances of id from the inv_index for the old labels. diff --git a/diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs b/diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs index 23532ecd2..feafa6ad2 100644 --- a/diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs +++ b/diskann-label-filter/src/inline_beta_search/encoded_document_accessor.rs @@ -8,7 +8,7 @@ use std::{ sync::{Arc, RwLock}, }; -use diskann::{graph::glue, provider::HasId, ANNError, ANNErrorKind, ANNResult}; +use diskann::{graph::glue, provider::HasId, ANNError, ANNResult}; use roaring::RoaringTreemap; use crate::traits::attribute_accessor::AttributeAccessor; @@ -65,10 +65,7 @@ where .attribute_accessor .visit_labels_of_point(id, |_, opt_set| match opt_set { Some(set) => Ok(f(&mut self.computer, set)), - None => Err(ANNError::message( - ANNErrorKind::IndexError, - "No labels were found for vector", - )), + None => Err(ANNError::message("No labels were found for vector")), }) { Ok(Ok(v)) => Ok(v), Ok(Err(err)) => Err(err), diff --git a/diskann-providers/src/common/minmax_repr.rs b/diskann-providers/src/common/minmax_repr.rs index 19dc8ec96..902e54fe8 100644 --- a/diskann-providers/src/common/minmax_repr.rs +++ b/diskann-providers/src/common/minmax_repr.rs @@ -30,6 +30,7 @@ use diskann_quantization::{ MinMaxCosineNormalized, MinMaxIP, MinMaxL2Squared, }, }; +use diskann_utils::lazy_format; use diskann_vector::{PureDistanceFunction, distance::Metric}; use thiserror::Error; @@ -54,7 +55,8 @@ pub enum MMConvertError { impl From for ANNError { fn from(value: MMConvertError) -> Self { - ANNError::log_index_error(format_args!( + ANNError::message(lazy_format!( + move, "Unable to convert MinMaxElement slice, error : {:?}", value )) diff --git a/diskann-providers/src/index/wrapped_async.rs b/diskann-providers/src/index/wrapped_async.rs index 78ba0e9a4..1a3b88b63 100644 --- a/diskann-providers/src/index/wrapped_async.rs +++ b/diskann-providers/src/index/wrapped_async.rs @@ -551,7 +551,7 @@ pub mod noawait { task::{Context, Poll, Waker}, }; - use diskann::{ANNErrorKind, utils::VectorId}; + use diskann::utils::VectorId; use thiserror::Error; type Input = Rc>>; @@ -622,7 +622,6 @@ pub mod noawait { Some(input) => input.replace(Some(k)), None => { return Err(ANNError::message( - ANNErrorKind::Opaque, "paged searcher errored and is no longer runnable", )); } @@ -654,13 +653,7 @@ pub mod noawait { MissingOutput, } - impl From for ANNError { - #[track_caller] - #[cold] - fn from(err: InternalInvariantViolated) -> Self { - Self::new(ANNErrorKind::Opaque, err) - } - } + diskann::convert_error!(InternalInvariantViolated); } #[cfg(test)] diff --git a/diskann-providers/src/model/graph/provider/async_/common.rs b/diskann-providers/src/model/graph/provider/async_/common.rs index 74d0949dc..9e3f3745b 100644 --- a/diskann-providers/src/model/graph/provider/async_/common.rs +++ b/diskann-providers/src/model/graph/provider/async_/common.rs @@ -33,7 +33,7 @@ impl StartPoints { end: match valid_points.checked_add(frozen_points.get() as u32) { Some(end) => end, None => { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Sum of valid points and frozen points exceeds u32::MAX", )); } @@ -277,7 +277,7 @@ impl std::error::Error for Panics {} impl From for ANNError { #[cold] fn from(_: Panics) -> ANNError { - ANNError::log_async_error("unreachable") + ANNError::message("unreachable") } } diff --git a/diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs b/diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs index db158a4b8..012253507 100644 --- a/diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs +++ b/diskann-providers/src/model/graph/provider/async_/experimental/multi_pq_async.rs @@ -7,6 +7,7 @@ use std::sync::{Arc, Mutex}; use arc_swap::{ArcSwap, Guard}; use diskann::{ANNError, ANNResult, error::IntoANNResult, utils::VectorRepr}; +use diskann_utils::lazy_format; use diskann_vector::{DistanceFunction, PreprocessedDistanceFunction, distance::Metric}; use rand::{Rng, SeedableRng, rngs::StdRng}; @@ -79,7 +80,11 @@ impl TestMultiPQProviderAsync { T: VectorRepr, { let table = self.multi_table().map_err(|err| { - ANNError::log_index_error(format_args!("Table construction failed with: {}", err)) + ANNError::message(lazy_format!( + move, + "Table construction failed with: {}", + err + )) })?; Ok(NoneToInfinity(QueryComputer::new( table, @@ -90,7 +95,11 @@ impl TestMultiPQProviderAsync { pub fn get_distance_computer(&self) -> ANNResult>> { let table = self.multi_table().map_err(|err| { - ANNError::log_index_error(format_args!("Table construction failed with: {}", err)) + ANNError::message(lazy_format!( + move, + "Table construction failed with: {}", + err + )) })?; Ok(NoneToInfinity(DistanceComputer::new(table, self.metric))) } @@ -98,7 +107,7 @@ impl TestMultiPQProviderAsync { pub fn get_vector(&self, id: usize) -> ANNResult>> { match self.quant_vectors.get(id) { Some(vector) => Ok(vector.load()), - None => Err(ANNError::log_index_error( + None => Err(ANNError::message( "Vector id is out of boundary in the dataset.", )), } @@ -109,12 +118,12 @@ impl TestMultiPQProviderAsync { T: Copy + Into, { if id >= self.max_vectors + self.num_start_points { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } if v.len() != self.table_new.get_dim() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } @@ -131,9 +140,10 @@ impl TestMultiPQProviderAsync { None => (&self.table_new, 1), Some(table_old) => { let v: f64 = { - let mut guard = self.rng.lock().map_err(|_| { - ANNError::log_lock_poison_error("in multi provider".to_string()) - })?; + let mut guard = self + .rng + .lock() + .map_err(|_| ANNError::message("in multi provider"))?; guard.random() }; if v <= self.split { @@ -154,7 +164,7 @@ impl TestMultiPQProviderAsync { ) .is_err() { - return Err(ANNError::log_index_error("Error in generating PQ data.")); + return Err(ANNError::message("Error in generating PQ data.")); } let new = Arc::new(VersionedPQVector::new(quant_vector, version)); diff --git a/diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs b/diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs index 420d6af37..d01261dcd 100644 --- a/diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs @@ -161,7 +161,7 @@ impl FastMemoryQuantVectorProviderAsync { T: VectorRepr, { if i >= self.total() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } @@ -169,7 +169,7 @@ impl FastMemoryQuantVectorProviderAsync { let vf32: &[f32] = &T::as_f32(v).into_ann_result()?; if vf32.len() != self.full_dim() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector f32 dimension is not equal to the expected dimension.", )); } @@ -208,12 +208,12 @@ impl FastMemoryQuantVectorProviderAsync { /// 2. Be okay with racey data. pub(crate) unsafe fn set_quant_vector(&self, i: usize, v: &[u8]) -> ANNResult<()> { if i >= self.total() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } if v.len() != self.pq_chunks() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } @@ -379,7 +379,7 @@ impl storage::bin::GetData for FastMemoryQuantVectorProviderAsync { #[cfg(test)] mod tests { use crate::storage::VirtualStorageProvider; - use diskann::{ANNErrorKind, utils::ONE}; + use diskann::utils::ONE; use diskann_vector::{DistanceFunction, PreprocessedDistanceFunction, distance::Metric}; use super::*; @@ -397,16 +397,28 @@ mod tests { // try to set an out of bounds vector // SAFETY: We have exclusive ownership of `provider`. let result = unsafe { provider.set_quant_vector(20, &[]) }.unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::IndexError); + let msg = result.to_string(); + assert!( + msg.contains("Vector id is out of boundary in the dataset."), + "{msg}", + ); // SAFETY: We have exclusive ownership of `provider`. let result = unsafe { provider.set_vector_sync::(20, &[]) }.unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::IndexError); + let msg = result.to_string(); + assert!( + msg.contains("Vector id is out of boundary in the dataset."), + "{msg}", + ); // try to set a vector with the wrong dimension // SAFETY: We have exclusive ownership of `provider`. let result = unsafe { provider.set_quant_vector(0, &[]) }.unwrap_err(); - assert_eq!(result.kind(), ANNErrorKind::IndexError); + let msg = result.to_string(); + assert!( + msg.contains("Vector dimension is not equal to the expected dimension.",), + "{msg}", + ); } fn create_test_provider() -> FastMemoryQuantVectorProviderAsync { diff --git a/diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs b/diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs index ff5d24008..aafe2b233 100644 --- a/diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs @@ -144,12 +144,12 @@ impl FastMemoryVectorProviderAsync { #[inline(always)] pub(crate) unsafe fn set_vector_sync(&self, i: usize, v: &[T]) -> ANNResult<()> { if v.len() != self.dim { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } if i >= self.total() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } diff --git a/diskann-providers/src/model/graph/provider/async_/inmem/provider.rs b/diskann-providers/src/model/graph/provider/async_/inmem/provider.rs index 79c7ccdce..df9ecca4d 100644 --- a/diskann-providers/src/model/graph/provider/async_/inmem/provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/inmem/provider.rs @@ -15,7 +15,7 @@ use diskann::{ }, utils::{IntoUsize, ONE, VectorRepr}, }; -use diskann_utils::future::AsyncFriendly; +use diskann_utils::{future::AsyncFriendly, lazy_format}; use diskann_vector::distance::Metric; use crate::{ @@ -401,11 +401,14 @@ where Itr: ExactSizeIterator + 'a, { let start_points = self.start_points.range(); - if itr.len() != start_points.len() { - return Err(ANNError::log_async_index_error(format!( + let num_start_points = start_points.len(); + let itr_len = itr.len(); + if itr_len != num_start_points { + return Err(ANNError::message(lazy_format!( + move, "expected `itr` to contain `{}` items, instead it has {}", - start_points.len(), - itr.len(), + num_start_points, + itr_len, ))); } @@ -504,10 +507,13 @@ where let valid_points = npts .checked_sub(ctx.num_frozen_points.get()) .ok_or_else(|| { - ANNError::log_index_error(format_args!( + let num_frozen_points = ctx.num_frozen_points.get(); + let num_base_vectors = base_vectors.total(); + ANNError::message(lazy_format!( + move, "Expected {} start points but the stored index only has {} total points", - ctx.num_frozen_points.get(), - base_vectors.total(), + num_frozen_points, + num_base_vectors, )) })?; let start_points = StartPoints::new(valid_points as u32, ctx.num_frozen_points)?; diff --git a/diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs b/diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs index 561eaaec5..54376590e 100644 --- a/diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs +++ b/diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs @@ -880,12 +880,7 @@ pub enum SQError { QuantizerDecodeError(#[from] crate::storage::protos::ProtoConversionError), } -impl From for ANNError { - #[cold] - fn from(err: SQError) -> Self { - ANNError::log_sq_error(err) - } -} +diskann::convert_error!(SQError); #[cfg(test)] mod tests { diff --git a/diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs b/diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs index 10f174a87..c94ab5eec 100644 --- a/diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs +++ b/diskann-providers/src/model/graph/provider/async_/inmem/spherical.rs @@ -8,7 +8,7 @@ use std::{future::Future, sync::Mutex}; use diskann::{ - ANNError, ANNErrorKind, ANNResult, default_post_processor, + ANNError, ANNResult, convert_error, default_post_processor, error::IntoANNResult, graph::{ AdjacencyList, @@ -50,45 +50,17 @@ use crate::{ // Error Promotion // ///////////////////// -impl From> for ANNError { - #[track_caller] - fn from(err: Bridge) -> Self { - ANNError::new(ANNErrorKind::SQError, err) - } -} - -impl From> for ANNError { - #[track_caller] - fn from(err: Bridge) -> Self { - ANNError::new(ANNErrorKind::SQError, err) - } -} - -impl From> for ANNError { - #[track_caller] - fn from(err: Bridge) -> Self { - ANNError::new(ANNErrorKind::SQError, err) - } -} - -impl From> for ANNError { - #[track_caller] - fn from(err: Bridge) -> Self { - ANNError::new(ANNErrorKind::SQError, err) - } -} +convert_error!(Bridge); +convert_error!(Bridge); +convert_error!(Bridge); +convert_error!(Bridge); /// An allocator error scoped to the spherical store. #[derive(Debug, Clone, Copy, Error)] #[error(transparent)] pub struct AllocatorError(#[from] diskann_quantization::alloc::AllocatorError); -impl From for ANNError { - #[track_caller] - fn from(err: AllocatorError) -> Self { - ANNError::new(ANNErrorKind::SQError, err) - } -} +convert_error!(AllocatorError); /////////// // Error // @@ -707,13 +679,7 @@ pub enum RQError { FullPrecisionConversionErr(Box), } -impl From for ANNError { - #[cold] - #[track_caller] - fn from(err: RQError) -> Self { - ANNError::log_sq_error(err) - } -} +diskann::convert_error!(RQError); /////////// // Tests // diff --git a/diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs b/diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs index 760f431cd..391873612 100644 --- a/diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/memory_quant_vector_provider.rs @@ -109,7 +109,7 @@ impl MemoryQuantVectorProviderAsync { self.num_get_calls.increment(); match self.quant_vectors.get(i) { Some(vector) => Ok(vector.load()), - None => Err(ANNError::log_index_error( + None => Err(ANNError::message( "Vector id is out of boundary in the dataset.", )), } @@ -129,7 +129,7 @@ impl MemoryQuantVectorProviderAsync { { let v_f32 = T::as_f32(v).map_err(|x| x.into())?; if v_f32.len() != self.full_dim() { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector f32 dimension is not equal to the expected dimension.", )); } @@ -151,7 +151,7 @@ impl MemoryQuantVectorProviderAsync { let slot = match self.quant_vectors.get(i) { Some(slot) => slot, None => { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } diff --git a/diskann-providers/src/model/graph/provider/async_/memory_vector_provider.rs b/diskann-providers/src/model/graph/provider/async_/memory_vector_provider.rs index b0523c1cf..f1e6a2c11 100644 --- a/diskann-providers/src/model/graph/provider/async_/memory_vector_provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/memory_vector_provider.rs @@ -58,7 +58,7 @@ impl MemoryVectorProviderAsync { self.num_get_calls.increment(); match self.vectors.get(i) { Some(vector) => Ok(VectorGuard::from_guard(vector.load())), - None => Err(ANNError::log_index_error( + None => Err(ANNError::message( "Vector id is out of boundary in the dataset.", )), } @@ -72,14 +72,14 @@ impl MemoryVectorProviderAsync { /// * `v.dim() != self.dim()`: The slice must have the proper length. pub(crate) fn set_vector_sync(&self, i: usize, v: &[T]) -> ANNResult<()> { if v.len() != self.dim { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector dimension is not equal to the expected dimension.", )); } let slot = match self.vectors.get(i) { Some(slot) => slot, None => { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "Vector id is out of boundary in the dataset.", )); } diff --git a/diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs b/diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs index 2125fb06a..3e8812186 100644 --- a/diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs +++ b/diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs @@ -7,6 +7,7 @@ use std::sync::RwLock; use crate::storage::{StorageReadProvider, StorageWriteProvider}; use diskann::{ANNError, ANNResult, graph::AdjacencyList, provider::HasId}; +use diskann_utils::lazy_format; use diskann_vector::contains::ContainsSimd; use tracing::trace; @@ -156,9 +157,11 @@ impl SimpleNeighborProviderAsync { // // Work backwards from this value to determine the internal `max_points`. let max_points = num_points.checked_sub(num_start_points).ok_or_else(|| { - ANNError::log_index_error(format_args!( + ANNError::message(lazy_format!( + move, "expected {} start points but the on-disk dataset only has {} total points", - num_start_points, num_points, + num_start_points, + num_points, )) })?; diff --git a/diskann-providers/src/model/graph/provider/determinant_diversity.rs b/diskann-providers/src/model/graph/provider/determinant_diversity.rs index 5e35c8708..b45a24b68 100644 --- a/diskann-providers/src/model/graph/provider/determinant_diversity.rs +++ b/diskann-providers/src/model/graph/provider/determinant_diversity.rs @@ -151,21 +151,7 @@ pub enum DeterminantDiversityError { }, } -impl From for diskann::ANNError { - #[track_caller] - fn from(err: DeterminantDiversityError) -> Self { - use diskann::ANNErrorKind; - let kind = match err { - DeterminantDiversityError::InvalidPower(_) - | DeterminantDiversityError::InvalidEta(_) => ANNErrorKind::IndexConfigError, - DeterminantDiversityError::QueryDimensionMismatch { .. } - | DeterminantDiversityError::DistanceCountMismatch { .. } => { - ANNErrorKind::DimensionMismatchError - } - }; - diskann::ANNError::new(kind, err) - } -} +diskann::convert_error!(DeterminantDiversityError); #[derive(Clone, Copy)] struct DistanceRange { diff --git a/diskann-providers/src/model/pq/distance/dynamic.rs b/diskann-providers/src/model/pq/distance/dynamic.rs index c3ee138ed..2c80a5d9b 100644 --- a/diskann-providers/src/model/pq/distance/dynamic.rs +++ b/diskann-providers/src/model/pq/distance/dynamic.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use diskann::{ANNError, ANNResult}; -use diskann_utils::object_pool::ObjectPool; +use diskann_utils::{lazy_format, object_pool::ObjectPool}; use diskann_vector::{DistanceFunction, PreprocessedDistanceFunction, distance::Metric}; // Concrete implementations @@ -68,10 +68,12 @@ impl<'a> QueryComputer<'a> { pool: Option>>>, ) -> ANNResult { let dim = table.get_dim(); - if query.len() != dim { - return Err(ANNError::log_dimension_mismatch_error(format!( + let query_len = query.len(); + if query_len != dim { + return Err(ANNError::message(lazy_format!( + move, "QueryComputer::new: expected query of length {dim}, got {}", - query.len() + query_len ))); } let result = match metric { @@ -483,8 +485,7 @@ mod tests { }; let table = test_utils::seed_pivot_table(config); let short_query = vec![0.0f32; config.dim - 1]; - let err = QueryComputer::new(&table, Metric::L2, &short_query, None).unwrap_err(); - assert_eq!(err.kind(), diskann::ANNErrorKind::DimensionMismatchError); + let _ = QueryComputer::new(&table, Metric::L2, &short_query, None).unwrap_err(); } #[test] diff --git a/diskann-providers/src/model/pq/fixed_chunk_pq_table.rs b/diskann-providers/src/model/pq/fixed_chunk_pq_table.rs index 2973db8fd..6dbeeb89c 100644 --- a/diskann-providers/src/model/pq/fixed_chunk_pq_table.rs +++ b/diskann-providers/src/model/pq/fixed_chunk_pq_table.rs @@ -9,7 +9,10 @@ use diskann_quantization::{ product::{self, BasicTable}, views::ChunkOffsetsBase, }; -use diskann_utils::views::{self, MatrixBase, MatrixView}; +use diskann_utils::{ + lazy_format, + views::{self, MatrixBase, MatrixView}, +}; use diskann_vector::{PureDistanceFunction, distance}; use diskann_wide::ARCH; @@ -136,7 +139,7 @@ impl FixedChunkPQTable { MatrixBase::try_from(pq_table, len / dim, dim).bridge_err()?, ChunkOffsetsBase::new(chunk_offsets).bridge_err()?, ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))?; + .map_err(ANNError::new)?; Ok(Self { table }) } @@ -159,7 +162,7 @@ impl FixedChunkPQTable { let dim = self.get_dim(); debug_assert_eq!(query.len(), dim); if aligned_pq_table_dist_scratch.len() < num_chunks * num_centers { - return Err(ANNError::log_pq_error( + return Err(ANNError::message( "aligned_pq_table_dist_scratch.len() should at least be num_pq_chunks * num_centers", )); } @@ -431,11 +434,7 @@ impl FixedChunkPQTable { // However, we can use a wrapper type to implement the conversion. // This is a workaround to allow the conversion from `product::TableCompressionError` to // `ANNError` without violating the orphan rule. -impl From> for ANNError { - fn from(value: Bridge) -> ANNError { - ANNError::log_pq_error(diskann_quantization::error::format(&value.into_inner())) - } -} +diskann::convert_error!(Bridge); impl CompressInto<&[T], &mut [u8]> for FixedChunkPQTable where @@ -492,9 +491,11 @@ fn pq_dist_lookup( let dists_out = match dists_out.get_mut(..n_pts) { None => { - return Err(ANNError::log_pq_error(format_args!( + let dists_out_len = dists_out.len(); + return Err(ANNError::message(lazy_format!( + move, "ERROR: dists_out length: {} is less than n_pts: {}", - dists_out.len(), + dists_out_len, n_pts ))); } @@ -604,10 +605,13 @@ fn aggregate_coords( pq_coordinate_scratch: &mut [u8], ) -> ANNResult<()> { if pq_coordinate_scratch.len() < ids.len() * num_pq_chunks { - return Err(ANNError::log_pq_error(format_args!( + let found_len = pq_coordinate_scratch.len(); + let expected_len = ids.len() * num_pq_chunks; + return Err(ANNError::message(lazy_format!( + move, "pq_coordinate_scratch doesn't have enough length. It has length {} but requires length {}", - pq_coordinate_scratch.len(), - ids.len() * num_pq_chunks + found_len, + expected_len, ))); } @@ -1002,7 +1006,7 @@ mod fixed_chunk_pq_table_test { let offsets = read_bin_from::(&mut reader, 0)?; if offsets.nrows() != 4 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. \ Offsets don't contain correct metadata, \ # offsets = {}, but expecting 4.", @@ -1015,7 +1019,7 @@ mod fixed_chunk_pq_table_test { let mut pivots = read_bin_from::(&mut reader, file_offset_data[(0, 0)])?; if pivots.nrows() != NUM_PQ_CENTROIDS { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_num_centers = {}, but expecting {} centers.", pq_pivots_path, pivots.nrows(), @@ -1026,7 +1030,7 @@ mod fixed_chunk_pq_table_test { let centroids = read_bin_from::(&mut reader, file_offset_data[(1, 0)])?; if centroids.nrows() != dim || centroids.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_dim = {}, \ file_cols = {} but expecting {} entries in 1 dimension.", pq_pivots_path, @@ -1042,7 +1046,7 @@ mod fixed_chunk_pq_table_test { let chunk_offsets_m = read_bin_from::(&mut reader, file_offset_data[(2, 0)])?; if chunk_offsets_m.nrows() != num_pq_chunks + 1 || chunk_offsets_m.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file at chunk offsets; \ file has nr={}, nc={} but expecting nr={} and nc=1.", chunk_offsets_m.nrows(), diff --git a/diskann-providers/src/model/pq/generate_pivot_arguments.rs b/diskann-providers/src/model/pq/generate_pivot_arguments.rs index a70252f8a..f42bc58a4 100644 --- a/diskann-providers/src/model/pq/generate_pivot_arguments.rs +++ b/diskann-providers/src/model/pq/generate_pivot_arguments.rs @@ -6,7 +6,6 @@ //! Arguments for generate pq pivot -use diskann::ANNError; use thiserror::Error; /// Represents the configuration parameters required to generate pivots for Product Quantization (PQ). @@ -58,13 +57,7 @@ pub enum GeneratePivotArgumentsError { NumTrainGreaterThanI32MaxValue(usize), } -// Compatibility with ANNError. -impl From for ANNError { - #[track_caller] - fn from(value: GeneratePivotArgumentsError) -> Self { - ANNError::log_pq_error(value) - } -} +diskann::convert_error!(GeneratePivotArgumentsError); impl GeneratePivotArguments { /// Constructor @@ -132,7 +125,7 @@ impl GeneratePivotArguments { #[cfg(test)] mod arguments_test { - use diskann::{ANNErrorKind, ANNResult}; + use diskann::ANNResult; use super::*; @@ -233,7 +226,6 @@ mod arguments_test { let result = compatibility_helper(); assert!(result.is_err()); - assert_eq!(result.unwrap_err().kind(), ANNErrorKind::PQError,); } fn compatibility_helper() -> ANNResult<()> { diff --git a/diskann-providers/src/model/pq/pq_construction.rs b/diskann-providers/src/model/pq/pq_construction.rs index 8ca3f11a4..68fc1fed4 100644 --- a/diskann-providers/src/model/pq/pq_construction.rs +++ b/diskann-providers/src/model/pq/pq_construction.rs @@ -109,9 +109,9 @@ where }; let dim = NonZeroUsize::new(parameters.dim()) - .ok_or_else(|| ANNError::log_pq_error("dim must be non-zero"))?; + .ok_or_else(|| ANNError::message("dim must be non-zero"))?; let num_chunks = NonZeroUsize::new(parameters.num_pq_chunks()) - .ok_or_else(|| ANNError::log_pq_error("num_pq_chunks must be non-zero"))?; + .ok_or_else(|| ANNError::message("num_pq_chunks must be non-zero"))?; let chunk_offsets = ChunkOffsets::partition(dim, num_chunks).bridge_err()?; let trainer = diskann_quantization::product::train::LightPQTrainingParameters::new( @@ -129,7 +129,7 @@ where &random_provider, &diskann_quantization::cancel::DontCancel, ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))? + .map_err(ANNError::new)? .flatten(); Ok(result) })?; @@ -170,19 +170,19 @@ pub fn generate_pq_pivots_from_membuf>( pool: RayonThreadPoolRef<'_>, ) -> ANNResult<()> { if full_pivot_data.len() != parameters.num_centers() * parameters.dim() { - return Err(ANNError::log_pq_error( + return Err(ANNError::message( "Error: full_pivot_data size is not num_centers * dim.", )); } if offsets.len() != parameters.num_pq_chunks() + 1 { - return Err(ANNError::log_pq_error( + return Err(ANNError::message( "Error: invalid offsets buffer input size.", )); } if *cancellation_token { - return Err(ANNError::log_pq_error( + return Err(ANNError::message( "Error: Cancellation requested by caller.", )); } @@ -195,7 +195,7 @@ pub fn generate_pq_pivots_from_membuf>( // Calculate the chunk offsets, filling the caller-owned buffer. let dim = NonZeroUsize::new(parameters.dim()) - .ok_or_else(|| ANNError::log_pq_error("dim must be non-zero"))?; + .ok_or_else(|| ANNError::message("dim must be non-zero"))?; let chunk_offsets_view = ChunkOffsetsView::partition_into(dim, offsets).bridge_err()?; let trainer = diskann_quantization::product::train::LightPQTrainingParameters::new( @@ -233,7 +233,7 @@ pub fn generate_pq_pivots_from_membuf>( &rng_builder, &cancelation, ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))? + .map_err(ANNError::new)? .flatten(); Ok(result) })?; @@ -346,9 +346,7 @@ where let full_dim: usize; if !pq_storage.pivot_data_exist(storage_provider) { - return Err(ANNError::log_pq_error( - "ERROR: PQ k-means pivot file not found.", - )); + return Err(ANNError::message("ERROR: PQ k-means pivot file not found.")); } else { (_, full_dim) = pq_storage.read_existing_pivot_metadata(storage_provider)?; (full_pivot_data, centroid, chunk_offsets) = pq_storage.load_existing_pivot_data( @@ -395,7 +393,7 @@ where .bridge_err()? .to_owned(), ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))?; + .map_err(ANNError::new)?; let mut buffer = vec![0.0; full_dim * block_size]; @@ -438,9 +436,7 @@ where .par_window_iter(BATCH_SIZE) .zip_eq(compressed_block.par_window_iter_mut(BATCH_SIZE)) .try_for_each_in_pool(pool, |(src, dst)| { - table.compress_into(src, dst).map_err(|err| { - ANNError::log_pq_error(diskann_quantization::error::format(&err)) - }) + table.compress_into(src, dst).map_err(ANNError::new) })?; let offset = start_index * num_pq_chunks + std::mem::size_of::() * 2; @@ -505,7 +501,7 @@ pub fn generate_pq_data_from_pivots_from_membuf>( MatrixView::try_from(pivot_data, num_pivots, dim).bridge_err()?, diskann_quantization::views::ChunkOffsetsView::new(offsets).bridge_err()?, ) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err)))?; + .map_err(ANNError::new)?; let data = vector_data .iter() @@ -514,7 +510,7 @@ pub fn generate_pq_data_from_pivots_from_membuf>( table .compress_into(data.as_slice(), pq_out) - .map_err(|err| ANNError::log_pq_error(diskann_quantization::error::format(&err))) + .map_err(ANNError::new) } /// Legacy compatibility function for providing an batch data generation. @@ -542,14 +538,12 @@ pub fn generate_pq_data_from_pivots_from_membuf_batch let dim = parameters.dim(); if vector_data.len() != num_train * dim { - return Err(ANNError::log_pq_error( + return Err(ANNError::message( "Error: Vector data length has the incorrect size!", )); } if pq_out.len() != num_train * num_pq_chunks { - return Err(ANNError::log_pq_error( - "Error: Invalid PQ buffer input size.", - )); + return Err(ANNError::message("Error: Invalid PQ buffer input size.")); } pq_out diff --git a/diskann-providers/src/model/pq/strided.rs b/diskann-providers/src/model/pq/strided.rs index 4930e6b79..4ff3c4d2d 100644 --- a/diskann-providers/src/model/pq/strided.rs +++ b/diskann-providers/src/model/pq/strided.rs @@ -8,11 +8,10 @@ use diskann_utils::{strided, views}; use crate::utils::Bridge; -// Compatibility with ANNError. impl From>> for ANNError { #[track_caller] fn from(value: Bridge>) -> Self { - ANNError::log_pq_error(value.into_inner()) + ANNError::new(value.into_inner().as_static()) } } @@ -22,8 +21,6 @@ impl From>> for ANNError { #[cfg(test)] mod tests { - use diskann::ANNErrorKind; - use super::*; use crate::utils::BridgeErr; @@ -41,7 +38,6 @@ mod tests { let message = format!("{}", err); let ann = ANNError::from(err); - assert_eq!(ann.kind(), ANNErrorKind::PQError); let formatted = ann.to_string(); assert!(formatted.contains(&message)); } diff --git a/diskann-providers/src/model/pq/views.rs b/diskann-providers/src/model/pq/views.rs index 17028de3c..d9c757d74 100644 --- a/diskann-providers/src/model/pq/views.rs +++ b/diskann-providers/src/model/pq/views.rs @@ -3,48 +3,20 @@ * Licensed under the MIT license. */ -use diskann::ANNError; +use diskann::{ANNError, convert_error}; use diskann_utils::views; use crate::utils::Bridge; -// Compatibility with ANNError. -impl From> for ANNError { - #[track_caller] - fn from(value: Bridge) -> Self { - ANNError::log_pq_error(value.into_inner()) - } -} - -// Compatibility with ANNError. -impl From> for ANNError { - #[track_caller] - fn from(value: Bridge) -> Self { - ANNError::log_pq_error(value.into_inner()) - } -} - -// Compatibility with ANNError. -impl From> for ANNError { - #[track_caller] - fn from(value: Bridge) -> Self { - ANNError::log_pq_error(value.into_inner()) - } -} - -// Compatibility with ANNError. -impl From> for ANNError { - #[track_caller] - fn from(value: Bridge) -> Self { - ANNError::log_pq_error(value.into_inner()) - } -} +convert_error!(Bridge); +convert_error!(Bridge); +convert_error!(Bridge); +convert_error!(Bridge); -// Compatibility with ANNError. impl From>> for ANNError { #[track_caller] fn from(value: Bridge>) -> Self { - ANNError::log_pq_error(value.into_inner()) + ANNError::new(value.into_inner().as_static()) } } @@ -54,8 +26,6 @@ impl From>> for ANNError { #[cfg(test)] mod tests { - use diskann::ANNErrorKind; - use super::*; use crate::utils::BridgeErr; @@ -69,7 +39,6 @@ mod tests { let message = format!("{}", err); let ann = ANNError::from(err); - assert_eq!(ann.kind(), ANNErrorKind::PQError); let formatted = ann.to_string(); assert!(formatted.contains(&message)); } diff --git a/diskann-providers/src/storage/bin.rs b/diskann-providers/src/storage/bin.rs index 9b607e715..358356265 100644 --- a/diskann-providers/src/storage/bin.rs +++ b/diskann-providers/src/storage/bin.rs @@ -136,7 +136,7 @@ where T: VectorRepr, { let metadata = load_metadata_from_file(provider, path).map_err(|err| { - ANNError::log_index_error(format_args!( + ANNError::message(format!( "failed to load data file \"{}\" due to the following error: {}", path, err )) @@ -195,7 +195,7 @@ where let len = slice.len(); if len != dim { - return Err(ANNError::log_index_error( + return Err(ANNError::message( "data provider returned a vector with a dimension other than advertised", )); } diff --git a/diskann-providers/src/storage/index_storage.rs b/diskann-providers/src/storage/index_storage.rs index fc526aa1f..adb89cf62 100644 --- a/diskann-providers/src/storage/index_storage.rs +++ b/diskann-providers/src/storage/index_storage.rs @@ -9,7 +9,7 @@ use super::{StorageReadProvider, StorageWriteProvider}; use diskann::{ ANNError, ANNResult, graph::DiskANNIndex, provider::DataProvider, utils::VectorRepr, }; -use diskann_utils::future::AsyncFriendly; +use diskann_utils::{future::AsyncFriendly, lazy_format}; use super::{AsyncIndexMetadata, AsyncQuantLoadContext, DiskGraphOnly, LoadWith, SaveWith}; use crate::model::{ @@ -197,7 +197,8 @@ fn get_and_validate_single_starting_point( let num_starting_points = start_ids.len(); if num_starting_points > 1 { - return Err(ANNError::log_index_error(format_args!( + return Err(ANNError::message(lazy_format!( + move, "ERROR: Save index does not support multiple starting points. Found {} starting points.", num_starting_points ))); @@ -206,7 +207,7 @@ fn get_and_validate_single_starting_point( start_ids .first() .cloned() - .ok_or_else(|| ANNError::log_index_error("ERROR: No starting points found")) + .ok_or_else(|| ANNError::message("ERROR: No starting points found")) } /////////// // Tests // diff --git a/diskann-providers/src/storage/pq_storage.rs b/diskann-providers/src/storage/pq_storage.rs index a2d49391b..5b6449ef8 100644 --- a/diskann-providers/src/storage/pq_storage.rs +++ b/diskann-providers/src/storage/pq_storage.rs @@ -184,7 +184,7 @@ impl PQStorage { let offsets = read_bin_from::(reader, 0)?; if offsets.nrows() != 4 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. Offsets don't contain correct \ metadata, # offsets = {}, but expecting 4.", &self.pivot_data_path, @@ -197,7 +197,7 @@ impl PQStorage { let pivots = read_bin_from::(reader, file_offset_data[(0, 0)])?; if pivots.nrows() != *num_centers || pivots.ncols() != *dim { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_num_centers = {}, \ file_dim = {} but expecting {} centers in {} dimensions.", &self.pivot_data_path, @@ -210,7 +210,7 @@ impl PQStorage { let centroid_m = read_bin_from::(reader, file_offset_data[(1, 0)])?; if centroid_m.nrows() != *dim || centroid_m.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_dim = {}, \ file_cols = {} but expecting {} entries in 1 dimension.", &self.pivot_data_path, @@ -222,7 +222,7 @@ impl PQStorage { let chunk_offsets_m = read_bin_from::(reader, file_offset_data[(2, 0)])?; if chunk_offsets_m.nrows() != *num_pq_chunks + 1 || chunk_offsets_m.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file at chunk offsets; \ file has nr={}, nc={} but expecting nr={} and nc=1.", chunk_offsets_m.nrows(), @@ -261,7 +261,7 @@ impl PQStorage { let data = read_bin::(&mut storage_provider.open_reader(pq_compressed_data)?)?; if data.nrows() != num_points_to_load || data.ncols() != num_pq_chunks { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "PQ compressed data mismatch: file has {}x{} but expected {}x{}", data.nrows(), data.ncols(), @@ -282,9 +282,7 @@ impl PQStorage { storage_provider: &Storage, ) -> ANNResult { if !storage_provider.exists(pq_pivots) { - return Err(ANNError::log_pq_error( - "ERROR: PQ k-means pivot file not found.", - )); + return Err(ANNError::message("ERROR: PQ k-means pivot file not found.")); } info!("Loading PQ pivots from {}...", pq_pivots); @@ -292,7 +290,7 @@ impl PQStorage { let mut reader = storage_provider.open_reader(pq_pivots)?; let offsets = read_bin_from::(&mut reader, 0)?; if offsets.nrows() != 4 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. Offsets don't contain correct metadata, \ # offsets = {}, but expecting 4.", pq_pivots, @@ -303,7 +301,7 @@ impl PQStorage { let mut pivots = read_bin_from::(&mut reader, file_offset_data[(0, 0)])?; if pivots.nrows() > NUM_PQ_CENTROIDS { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_num_centers = {}, but expecting {} centers.", pq_pivots, pivots.nrows(), @@ -314,7 +312,7 @@ impl PQStorage { let centroids = read_bin_from::(&mut reader, file_offset_data[(1, 0)])?; if centroids.nrows() != dim || centroids.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file {}. file_dim = {}, file_cols = {} \ but expecting {} entries in 1 dimension.", pq_pivots, @@ -328,7 +326,7 @@ impl PQStorage { if (chunk_offsets_m.nrows() != num_pq_chunks + 1 && num_pq_chunks as u32 != 0) || chunk_offsets_m.ncols() != 1 { - return Err(ANNError::log_pq_error(format_args!( + return Err(ANNError::message(format!( "Error reading pq_pivots file at chunk offsets; file has nr={}, nc={} \ but expecting nr={} and nc=1. The expected num_pq_chunks should be \ passed as 0 if we want to infer.", @@ -372,12 +370,7 @@ impl PQStorage { pub fn get_data_path(&self) -> ANNResult<&str> { self.data_path .as_ref() - .ok_or_else(|| { - ANNError::log_index_config_error( - "data_path".to_string(), - "pq_storage.data_path is not defined".to_string(), - ) - }) + .ok_or_else(|| ANNError::message("expected \"data_path\" to be configured")) .map(|s| s.as_str()) } diff --git a/diskann-providers/src/utils/file_util.rs b/diskann-providers/src/utils/file_util.rs index f5ebdadc6..548f43472 100644 --- a/diskann-providers/src/utils/file_util.rs +++ b/diskann-providers/src/utils/file_util.rs @@ -10,7 +10,7 @@ use std::io::Read; use crate::storage::StorageReadProvider; use diskann::{ANNError, ANNResult, utils::IntoUsize}; -use diskann_utils::{io::Metadata, views::Matrix}; +use diskann_utils::{io::Metadata, lazy_format, views::Matrix}; /// Read metadata of data file. pub fn load_metadata_from_file( @@ -72,17 +72,17 @@ pub fn load_multivec_bin::new(path, None, reader)?; let (medoid, medoid_id) = find_nearest_vector_with_id(iter, ¢roid)? - .ok_or_else(|| ANNError::log_index_error("medoid not found"))?; + .ok_or_else(|| ANNError::message("medoid not found"))?; Ok((medoid.to_vec(), medoid_id)) } else { - Err(ANNError::log_index_error( + Err(ANNError::message( "Medoid not calculable on zero length iterator", )) } @@ -241,7 +241,7 @@ where // Find medoid (point closest to centroid) from the full dataset let iter = VectorDataIterator::::new(path, None, reader)?; let (medoid, medoid_id) = find_nearest_vector_with_id(iter, ¢roid)? - .ok_or_else(|| ANNError::log_index_error("medoid not found"))?; + .ok_or_else(|| ANNError::message("medoid not found"))?; Ok((medoid.to_vec(), medoid_id)) } diff --git a/diskann-providers/src/utils/rayon_util.rs b/diskann-providers/src/utils/rayon_util.rs index ac5c2101d..744cbd6cc 100644 --- a/diskann-providers/src/utils/rayon_util.rs +++ b/diskann-providers/src/utils/rayon_util.rs @@ -11,7 +11,7 @@ pub fn create_thread_pool(num_threads: usize) -> ANNResult { let pool = rayon::ThreadPoolBuilder::new() .num_threads(num_threads) .build() - .map_err(|err| ANNError::log_thread_pool_error(err.to_string()))?; + .map_err(ANNError::new)?; Ok(RayonThreadPool(pool)) } @@ -37,7 +37,7 @@ pub fn create_thread_pool_for_test() -> RayonThreadPool { pub fn create_thread_pool_for_bench() -> RayonThreadPool { let pool = rayon::ThreadPoolBuilder::new() .build() - .map_err(|err| ANNError::log_thread_pool_error(err.to_string())) + .map_err(ANNError::new) .unwrap(); RayonThreadPool(pool) } diff --git a/diskann-providers/src/utils/sampling.rs b/diskann-providers/src/utils/sampling.rs index f5af47a07..92314ae9b 100644 --- a/diskann-providers/src/utils/sampling.rs +++ b/diskann-providers/src/utils/sampling.rs @@ -10,6 +10,7 @@ use std::{ use crate::storage::StorageReadProvider; use byteorder::{ByteOrder, LittleEndian, ReadBytesExt}; use diskann::{ANNError, ANNResult, error::IntoANNResult, utils::VectorRepr}; +use diskann_utils::lazy_format; use rand::{ Rng, distr::{Distribution, StandardUniform}, @@ -146,7 +147,7 @@ where let expected_size = 8 + (npts as u64 * dim as u64 * std::mem::size_of::() as u64); let actual_size = storage_provider.get_length(data_file)?; if actual_size != expected_size { - return Err(ANNError::log_invalid_file_format(format!( + return Err(ANNError::message(format!( "Vector file '{}' has invalid format: size {} bytes doesn't match expected size of {} bytes based on header ({} vectors of dimension {})", data_file, actual_size, expected_size, npts, dim ))); @@ -179,10 +180,12 @@ where for idx in indices { // Check if the index is within bounds if idx >= self.npts { - return Err(ANNError::log_index_error(format!( - "Vector index {} is out of bounds (max: {})", + let npts = self.npts; + return Err(ANNError::message(lazy_format!( + move, + "Vector index {} is out of bounds (must be less than: {})", idx, - self.npts - 1 + npts, ))); } let offset = (idx as i64 - self.cur_pos as i64) * vector_len as i64; @@ -270,9 +273,7 @@ pub fn gen_random_slice( Ok((sampled_vectors, sampled_count, full_dim)) } else { - Err(ANNError::log_index_error( - "Could not read vectors to sample from.", - )) + Err(ANNError::message("Could not read vectors to sample from.")) } } @@ -282,7 +283,6 @@ mod sampling_test { use crate::storage::{StorageWriteProvider, VirtualStorageProvider}; use byteorder::{LittleEndian, WriteBytesExt}; - use diskann::ANNErrorKind; use rstest::rstest; use super::*; @@ -407,10 +407,9 @@ mod sampling_test { .unwrap(); // Try to read invalid indices - let result = reader + let _ = reader .read_vectors(vec![TEST_NUM_POINTS + 1].into_iter(), |_| Ok(())) .unwrap_err(); - assert!(matches!(result.kind(), ANNErrorKind::IndexError)); } #[rstest] @@ -428,7 +427,7 @@ mod sampling_test { writer.flush().unwrap(); } // Should fail validation check - let err = match SampleVectorReader::::new( + let _ = match SampleVectorReader::::new( TEST_BINARY_FILE, sampling_density, &storage_provider, @@ -436,10 +435,5 @@ mod sampling_test { Ok(_) => panic!("operations should not succeed"), Err(err) => err, }; - assert!( - matches!(err.kind(), ANNErrorKind::InvalidFileFormatError), - "Invalid file format error expected, got {:?}", - err - ); } } diff --git a/diskann-providers/src/utils/vector_data_iterator.rs b/diskann-providers/src/utils/vector_data_iterator.rs index bd195cacf..c3d43341f 100644 --- a/diskann-providers/src/utils/vector_data_iterator.rs +++ b/diskann-providers/src/utils/vector_data_iterator.rs @@ -10,7 +10,7 @@ use std::{ }; use crate::storage::StorageReadProvider; -use diskann::{ANNError, ANNErrorKind, utils::read_exact_into}; +use diskann::utils::read_exact_into; use diskann_utils::io::Metadata; use serde::Deserialize; use thiserror::Error; @@ -205,17 +205,16 @@ enum SkipElementsError { IoError(#[from] std::io::Error), } -impl From for ANNError { - fn from(err: SkipElementsError) -> Self { - ANNError::new(ANNErrorKind::IndexError, err) - } -} +diskann::convert_error!(SkipElementsError); #[cfg(test)] mod tests { + use super::*; + use std::io::Cursor; - use super::*; + use diskann::ANNError; + const TEST_VECTOR_STREAM: &str = "vector"; const TEST_ASSOCIATED_DATA_STREAM: &str = "associated_data"; const INCORRECT_TEST_ASSOCIATED_DATA_STREAM: &str = "incorrect_associated_data"; @@ -334,8 +333,6 @@ mod tests { }; let ann_err = ANNError::from(skip_err); - assert_eq!(ann_err.kind(), ANNErrorKind::IndexError); - let err_msg = ann_err.to_string(); assert!(err_msg.contains("Tried to skip 10 elements, but only 5 are left")); @@ -355,8 +352,6 @@ mod tests { let skip_err = SkipElementsError::IoError(io_err); let ann_err = ANNError::from(skip_err); - assert_eq!(ann_err.kind(), ANNErrorKind::IndexError); - let err_msg = ann_err.to_string(); assert!(err_msg.contains("IO error while skipping elements")); assert!(err_msg.contains("unexpected end of file")); diff --git a/diskann-quantization/src/error.rs b/diskann-quantization/src/error.rs index 1e71a1de2..ab7ae0846 100644 --- a/diskann-quantization/src/error.rs +++ b/diskann-quantization/src/error.rs @@ -18,6 +18,83 @@ impl std::fmt::Display for Infallible { impl std::error::Error for Infallible {} +/// A utility for printing the whole [source tree](https://doc.rust-lang.org/std/error/trait.Error.html#method.source). +/// of an error type. +/// +/// ```rust +/// use diskann_quantization::error::Format; +/// use std::{error::Error, fmt::{Display, Formatter, Result}}; +/// +/// #[derive(Debug)] +/// struct A; +/// +/// impl Display for A { +/// fn fmt(&self, f: &mut Formatter<'_>) -> Result { +/// f.write_str("A") +/// } +/// } +/// +/// impl Error for A { +/// fn source(&self) -> Option<&(dyn Error + 'static)> { +/// Some(&B) +/// } +/// } +/// +/// #[derive(Debug)] +/// struct B; +/// +/// impl Display for B { +/// fn fmt(&self, f: &mut Formatter<'_>) -> Result { +/// f.write_str("B") +/// } +/// } +/// +/// impl Error for B {} +/// +/// assert_eq!(Format(A).to_string(), "A\n caused by: B"); +/// assert_eq!(Format(&A).to_string(), "A\n caused by: B"); +/// ``` +#[derive(Debug)] +pub struct Format(pub T); + +impl std::fmt::Display for Format +where + T: std::error::Error, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // Protect against pathological `Source` implementations that just return + // themselves. + const LIMIT: usize = 256; + + // Cast wrap the walking of the source chain into something that behaves like an + // iterator. + struct Source<'a>(Option<&'a (dyn std::error::Error + 'static)>); + impl<'a> Iterator for Source<'a> { + type Item = &'a (dyn std::error::Error + 'static); + fn next(&mut self) -> Option { + let current = self.0; + self.0 = match current { + Some(current) => current.source(), + None => None, + }; + current + } + } + + write!(f, "{}", self.0)?; + let mut itr = Source(self.0.source()); + for source in itr.by_ref().take(LIMIT) { + write!(f, "\n caused by: {}", source)?; + } + + if itr.next().is_some() { + write!(f, "\n ... (limit reached)")?; + } + + Ok(()) + } +} + /// Format the entire error chain for `err` by first calling `err.to_string()` and then /// by walking the error's /// [source tree](https://doc.rust-lang.org/std/error/trait.Error.html#method.source). @@ -25,29 +102,7 @@ pub fn format(err: &E) -> String where E: std::error::Error + ?Sized, { - // Cast wrap the walking of the source chain into something that behaves like an - // iterator. - struct SourceIterator<'a>(Option<&'a (dyn std::error::Error + 'static)>); - impl<'a> Iterator for SourceIterator<'a> { - type Item = &'a (dyn std::error::Error + 'static); - fn next(&mut self) -> Option { - let current = self.0; - self.0 = match current { - Some(current) => current.source(), - None => None, - }; - current - } - } - - // Get the base message from the error. - let mut message = err.to_string(); - // Walk the source chain, formatting each - for source in SourceIterator(err.source()) { - message.push_str("\n caused by: "); - message.push_str(&source.to_string()); - } - message + Format(err).to_string() } /// An implementation of `Box` that stores the error payload inline, @@ -251,11 +306,11 @@ mod tests { use super::*; - #[derive(Error, Debug)] + #[derive(Error, Debug, Clone)] #[error("error A")] struct ErrorA; - #[derive(Error, Debug)] + #[derive(Error, Debug, Clone)] #[error("error B with val {val}")] struct ErrorB { val: usize, @@ -277,6 +332,9 @@ mod tests { let message = format(&ErrorA); assert_eq!(message, "error A"); + assert_eq!(Format(ErrorA).to_string(), "error A"); + assert_eq!(Format(&ErrorA).to_string(), "error A"); + // One Level of Nesting let error = ErrorB { val: 10, @@ -285,6 +343,8 @@ mod tests { let expected = "error B with val 10\n caused by: error A"; assert_eq!(format(&error), expected); + assert_eq!(Format(&error).to_string(), expected); + assert_eq!(Format(error.clone()).to_string(), expected); // Multiple Levels of Nesting let error = ErrorC { @@ -294,7 +354,35 @@ mod tests { let expected = "error C with message Hello World\n \ caused by: error B with val 10\n \ caused by: error A"; + assert_eq!(format(&error), expected); + assert_eq!(Format(&error).to_string(), expected); + assert_eq!(Format(error).to_string(), expected); + } + + // A pathological error type that returns itself for `source` and thus ends up + // with an unlimited chain. + #[derive(Debug)] + struct Infinite; + + impl std::fmt::Display for Infinite { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("an unending source") + } + } + + impl std::error::Error for Infinite { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(self) + } + } + + #[test] + fn test_infinite_detected() { + // Without a limit - this line will never finish (we'll either hit a stack overflow + // or run out of memory for the string). + let s = Format(Infinite).to_string(); + assert!(s.contains("(limit reached)")); } /////////// diff --git a/diskann-tools/src/utils/cmd_tool_error.rs b/diskann-tools/src/utils/cmd_tool_error.rs index f84527a76..b8c7293c7 100644 --- a/diskann-tools/src/utils/cmd_tool_error.rs +++ b/diskann-tools/src/utils/cmd_tool_error.rs @@ -143,11 +143,7 @@ mod tests { #[test] fn test_from_ann_error() { - use diskann::ANNErrorKind; - let ann_error = diskann::ANNError::new( - ANNErrorKind::IndexError, - std::io::Error::other("test error"), - ); + let ann_error = diskann::ANNError::new(std::io::Error::other("test error")); let cmd_error: CMDToolError = ann_error.into(); assert!(cmd_error.details.contains("test error")); } diff --git a/diskann-tools/src/utils/search_index_utils.rs b/diskann-tools/src/utils/search_index_utils.rs index 176f2e1ec..702c351e4 100644 --- a/diskann-tools/src/utils/search_index_utils.rs +++ b/diskann-tools/src/utils/search_index_utils.rs @@ -221,14 +221,14 @@ pub fn calculate_filtered_search_recall( k_recall: u32, ) -> ANNResult { if k_recall == 0 { - return Err(ANNError::log_index_error(format_args!( + return Err(ANNError::message(format!( "k_recall value must be greater than 0, but got {}", k_recall ))); } if groundtruth.len() != num_queries || our_results.len() != num_queries { - return Err(ANNError::log_index_error(format_args!( + return Err(ANNError::message(format!( "groundtruth length ({}) or our_results length ({}) does not match num_queries ({})", groundtruth.len(), our_results.len(), @@ -257,7 +257,7 @@ pub fn calculate_filtered_search_recall( let gt_dist_vec = gt_dist[i].as_slice(); if gt_dist_vec.len() != groundtruth[i].len() { - return Err(ANNError::log_index_error(format_args!( + return Err(ANNError::message(format!( "Ground truth distance for query ({}) vector length ({}) is not equal to groundtruth len ({})", i, gt_dist_vec.len(), @@ -314,7 +314,7 @@ pub fn load_truthset( let truthset_type : i32 = match actual_file_size { x if x == expected_file_size_with_dists => 1, x if x == expected_file_size_just_ids => 2, - _ => return Err(ANNError::log_index_error(format_args!( + _ => return Err(ANNError::message(format!( "Error. File size mismatch. File should have bin format, with npts followed by ngt followed by npts*ngt ids and optionally followed by npts*ngt distance values; actual size: {}, expected: {} or {}", actual_file_size, expected_file_size_with_dists, diff --git a/diskann/src/error/ann_error.rs b/diskann/src/error/ann_error.rs index c27e622c1..bfd211281 100644 --- a/diskann/src/error/ann_error.rs +++ b/diskann/src/error/ann_error.rs @@ -19,12 +19,30 @@ pub type ANNResult = Result; /// Common error type shared through DiskANN. /// -/// This type disambiguates the runtime origin of errors using the `kind()` enum. Third -/// party implementations of DiskANN plugin types like provider can use `kind()` and the -/// downcasting API to throw custom errors from low in the callstack and retrieve those -/// errors higher in the stack. +/// [`ANNError`] is used to represent and propagate unrecoverable errors with minimal +/// overhead on the happy path. It attempts to gather as much information upon creation and +/// provides the following services. +/// +/// * Constructible from types implementing [`std::error::Error`] via [`ANNError::new`]. +/// When using this constructor, the [`source`](std::error::Error::source) chain of the +/// original error will be printed as well. +/// +/// * Constructible from non-error types implementing [`Display`] and [`Debug`] via +/// [`ANNError::message`] to support ad-hoc errors. +/// +/// * Context chaining via [`ANNError::context`] to add information as the error propagates +/// up the call stack. +/// +/// * [Down casting](ANNError::downcast) to attempt to retrieve the original error type. +/// +/// * Construction site source information via +/// [`#[track_caller]`](https://rustc-dev-guide.rust-lang.org/backend/implicit-caller-location.html) for all constructors. +/// +/// * Backtrace collection under `RUST_BACKTRACE=1`. +/// +/// * A footprint of `std::mem::size_of::<*const ()>()` for minimal impact on the happy path. /// ```rust -/// use diskann::{ANNError, ANNErrorKind, error::ErrorContext}; +/// use diskann::{ANNError, error::ErrorContext}; /// use thiserror::Error; /// /// // A custom error type used by a third-party. @@ -34,7 +52,7 @@ pub type ANNResult = Result; /// /// // A low-level function that returns an error. /// fn errors() -> Result<(), ANNError> { -/// Err(ANNError::new(ANNErrorKind::Tagged(100), CustomError(42))) +/// Err(ANNError::new(CustomError(42))) /// } /// /// // A function that propagates an error, adding context. @@ -50,13 +68,14 @@ pub type ANNResult = Result; /// assert!(message.contains("custom error: 42")); /// assert!(message.contains("propagated")); /// -/// // If we retrieve the `ANNErrorKind`, we can recognize that it belongs to a third-party -/// // plugin. -/// assert_eq!(err.kind(), ANNErrorKind::Tagged(100)); -/// /// // If we know the concrete error type, we can downcast the error. /// let downcasted = err.downcast_ref::().unwrap(); /// assert_eq!(downcasted.0, 42); +/// +/// // If we don't have an `Error` - we can still use `ANNError` for ad-hoc errors. +/// let err = ANNError::message("an ad-hoc error message"); +/// assert!(err.is::<&'static str>()); +/// assert!(err.to_string().contains("an ad-hoc error message")); /// ``` /// /// # Backtraces @@ -66,31 +85,15 @@ pub type ANNResult = Result; /// /// Backtrace collection adds a time overhead to error collection. /// -/// # Legacy API -/// -/// The `log_*` prefixed constructors exist to maintain compatibility with an earlier -/// iteration of this struct. These constructors set an internal `ANNErrorKind` and have -/// the side effect of logging the constructed object at an `Error` level. -/// -/// The log records associated with these messages contain the following keyed metadata: -/// -/// * "diskann.file" (&str) - The file of the constructor's caller. -/// * "diskann.line" (&str) - The line within the file of the constructor's caller. -/// -/// This can lead to double logging as errors are logged upon creation, and the logged again -/// upon reaching the top level. +/// # Source Information /// -/// # Properties -/// -/// `ANNError` has the following properties to support efficiency: -/// -/// * `std::mem::size_of::() == 16`: The struct is 16 bytes. This allows it to be -/// returned in registers rather than on the stack. -/// * `std::mem::size_of::>() == 16`: The struct can use Rust's niche -/// optimization. +/// All constructors are decorated with `#[track_caller]` to associate source location +/// information for better debugging. As such, implementations of `From for ANNError` +/// should also use `#[track_caller]` to record the source location where the error is +/// actually created. Simple error types may use the [`crate::convert_error!`] macro to +/// perform this automatically. #[derive(Debug)] pub struct ANNError { - kind: ANNErrorKind, error: anyhow::Error, } @@ -99,11 +102,10 @@ impl ANNError { /// /// Errors constructed this way can be retrieved using downcasting. /// ```rust - /// use diskann::{ANNError, ANNErrorKind}; + /// use diskann::{ANNError}; /// use std::env::VarError; /// /// let err = ANNError::new( - /// ANNErrorKind::IndexError, /// VarError::NotPresent, /// ); /// @@ -120,45 +122,11 @@ impl ANNError { /// is marked as `[inline(never)]` to outline error handling code. #[track_caller] #[inline(never)] - pub fn new(kind: ANNErrorKind, err: E) -> Self - where - E: std::error::Error + Send + Sync + 'static, - { - Self { - kind, - error: anyhow::Error::new(Located::new(err)), - } - } - - /// Construct a new `ANNError` encapsulating `err` tagged with `ANNErrorKind::Opaque`. - /// - /// Errors constructed this way can be retrieved using downcasting. - /// ```rust - /// use diskann::{ANNError, ANNErrorKind}; - /// use std::env::VarError; - /// - /// let err = ANNError::opaque(VarError::NotPresent); - /// - /// assert_eq!(err.kind(), ANNErrorKind::Opaque); - /// let retrieved: VarError = err.downcast::().unwrap(); - /// ``` - /// - /// # Attributes - /// - /// - `track_caller`: Internally, the type `err` is embedded inside a `Located` struct, - /// recording the file and line of creation. The `[track_caller]` attribute allows - /// for precise recording of the caller. - /// - /// - `inline(never)`: To keep the happy-path cost as minimal as possible, this function - /// is marked as `[inline(never)]` to outline error handling code. - #[track_caller] - #[inline(never)] - pub fn opaque(err: E) -> Self + pub fn new(err: E) -> Self where E: std::error::Error + Send + Sync + 'static, { Self { - kind: ANNErrorKind::Opaque, error: anyhow::Error::new(Located::new(err)), } } @@ -180,16 +148,25 @@ impl ANNError { /// is marked as `[inline(never)]` to outline error handling code. #[track_caller] #[inline(never)] - pub fn message(kind: ANNErrorKind, display: D) -> Self + pub fn message(display: D) -> Self where - D: std::fmt::Display + std::fmt::Debug + Send + Sync + 'static, + D: Display + Debug + Send + Sync + 'static, { Self { - kind, error: anyhow::Error::msg(Located::new(display)), } } + /// Return `true` is the the type `E` exists as either the root error type of in the + /// context chain of `self`. + #[must_use] + pub fn is(&self) -> bool + where + E: Display + Debug + Send + Sync + 'static, + { + self.error.is::>() || self.error.is::() + } + /// Attempt to downcast the error object to a concrete type. pub fn downcast(self) -> Result where @@ -199,10 +176,7 @@ impl ANNError { Ok(value) => Ok(value), Err(error) => match error.downcast::>() { Ok(value) => Ok(value.err), - Err(error) => Err(Self { - kind: self.kind, - error, - }), + Err(error) => Err(Self { error }), }, } } @@ -241,268 +215,16 @@ impl ANNError { C: Display + Debug + Send + Sync + 'static, { Self { - kind: self.kind, error: self.error.context(Located::new(context)), } } - - /// Return the kind of the originally constructed error. - pub fn kind(&self) -> ANNErrorKind { - self.kind - } - - ///////////////////////////// - // Compatibility interface // - ///////////////////////////// - - /// Create, log and return IndexError - #[track_caller] - #[inline(never)] - pub fn log_index_error(err: D) -> Self { - Self::message(ANNErrorKind::IndexError, err.to_string()) - } - - /// Create, log and return FileHandleError - #[track_caller] - #[inline(never)] - pub fn log_file_handle_error(err: D) -> Self { - Self::message(ANNErrorKind::FileHandleError, err.to_string()) - } - - /// Create, log and return FileNotFoundError - #[track_caller] - #[inline(never)] - pub fn log_file_not_found_error(err: String) -> Self { - Self::message(ANNErrorKind::FileNotFoundError, err) - } - - /// Create, log and return GroundTruthError - #[track_caller] - #[inline(never)] - pub fn log_ground_truth_error(err: String) -> Self { - Self::message(ANNErrorKind::GroundTruthError, err) - } - - /// Create, log and return IndexConfigError - #[track_caller] - #[inline(never)] - pub fn log_index_config_error(parameter: String, err: String) -> Self { - Self::message( - ANNErrorKind::IndexConfigError, - format!("{} is invalid, err = {}", parameter, err), - ) - } - - /// Create, log and return TryFromIntError - #[track_caller] - #[inline(never)] - pub fn log_try_from_int_error(err: TryFromIntError) -> Self { - Self::new(ANNErrorKind::TryFromIntError, err) - } - - /// Create, log and return IOError - #[track_caller] - #[inline(never)] - pub fn log_io_error(err: io::Error) -> Self { - Self::new(ANNErrorKind::IOError, err) - } - - /// Create, log and return IOSendError - #[track_caller] - #[inline(never)] - pub fn log_io_send_error(err: mpsc::SendError) -> Self { - Self::new(ANNErrorKind::IOSendError, err) - } - - /// Create, log and return DiskIOAlignmentError - #[track_caller] - #[inline(never)] - pub fn log_disk_io_request_alignment_error(err: String) -> Self { - Self::message(ANNErrorKind::DiskIOAlignmentError, err) - } - - /// Create, log and return IOError - #[track_caller] - #[inline(never)] - pub fn log_mem_alloc_layout_error(err: LayoutError) -> Self { - Self::new(ANNErrorKind::MemoryAllocLayoutError, err) - } - - /// Create, log and return LockPoisonError - #[track_caller] - #[inline(never)] - pub fn log_lock_poison_error(err: String) -> Self { - Self::message(ANNErrorKind::LockPoisonError, err) - } - - /// Create, log and return PQError - #[track_caller] - #[inline(never)] - pub fn log_pq_error(err: D) -> Self { - Self::message(ANNErrorKind::PQError, err.to_string()) - } - - /// Create, log and return SQError - #[track_caller] - #[inline(never)] - pub fn log_sq_error(err: E) -> Self - where - E: std::error::Error + Send + Sync + 'static, - { - Self::new(ANNErrorKind::SQError, err) - } - - /// Create, log and return KMeansError - #[track_caller] - #[inline(never)] - pub fn log_kmeans_error(err: String) -> Self { - Self::message(ANNErrorKind::KMeansError, err) - } - - /// Create, log and return KMeansError - #[track_caller] - #[inline(never)] - pub fn log_push_error(err: E) -> Self - where - E: std::error::Error + Send + Sync + 'static, - { - Self::new(ANNErrorKind::PushError, err) - } - - /// Create, log and return TryFromSliceError - #[track_caller] - #[inline(never)] - pub fn log_try_from_slice_error(err: TryFromSliceError) -> Self { - Self::new(ANNErrorKind::TryFromSliceError, err) - } - - /// Create, log and return Serde error. - #[track_caller] - #[inline(never)] - pub fn log_serde_error(operation: String, err: D) -> Self - where - D: Display, - { - Self::message( - ANNErrorKind::SerdeError, - format!("Operation: {} Error: {}", operation, err), - ) - } - - /// Create, log and return get vertex data error. - #[track_caller] - #[inline(never)] - pub fn log_get_vertex_data_error(vertex_id: String, data_type: String) -> Self { - Self::message( - ANNErrorKind::GetVertexDataError, - format!("vertex_id: {} data_type: {}", vertex_id, data_type), - ) - } - - /// Create, log and return parse slice error. - #[track_caller] - #[inline(never)] - pub fn log_parse_slice_error( - parsing_source: String, - parsing_target: String, - err: String, - ) -> Self { - Self::message( - ANNErrorKind::ParseSliceError, - format!( - "source: {} target: {} error: {}", - parsing_source, parsing_target, err - ), - ) - } - - #[track_caller] - #[inline(never)] - pub fn log_thread_pool_error(err: String) -> Self { - Self::message(ANNErrorKind::ThreadPoolError, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_invalid_operation_error(err: String) -> Self { - Self::message(ANNErrorKind::InvalidOperation, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_async_error(err: D) -> Self { - Self::message(ANNErrorKind::AsyncError, err.to_string()) - } - - #[track_caller] - #[inline(never)] - pub fn log_async_index_error(err: D) -> Self { - Self::message(ANNErrorKind::AsyncIndexError, err.to_string()) - } - - #[track_caller] - #[inline(never)] - pub fn log_async_shutdown_error(err: D) -> Self { - Self::message(ANNErrorKind::AsyncShutdownError, err.to_string()) - } - - #[track_caller] - #[inline(never)] - pub fn log_async_runtime_error(err: String) -> Self { - Self::message(ANNErrorKind::RustRuntimeError, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_dimension_mismatch_error(err: String) -> Self { - Self::message(ANNErrorKind::DimensionMismatchError, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_paged_search_error(err: String) -> Self { - Self::message(ANNErrorKind::PagedSearchError, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_profiler_error(err: String) -> Self { - Self::message(ANNErrorKind::ProfilerError, err) - } - - #[track_caller] - #[inline(never)] - pub fn log_pq_schema_registration_error(err: T) -> Self - where - T: Display, - { - Self::message(ANNErrorKind::PQSchemaRegistrationError, err.to_string()) - } - - #[track_caller] - #[inline(never)] - pub fn log_invalid_file_format(err: T) -> Self - where - T: Display, - { - Self::message(ANNErrorKind::InvalidFileFormatError, err.to_string()) - } - - #[track_caller] - #[inline(never)] - pub fn log_build_interrupted(err: T) -> Self - where - T: Display, - { - Self::message(ANNErrorKind::BuildInterrupted, err.to_string()) - } } impl Display for ANNError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { // Use the debug format `{:?}` for `anyhow::Error` to get the source chain as well // as a stack trace. - write!(formatter, "ANNError: {:?}\n\n{:?}", self.kind, self.error) + write!(formatter, "ANNError\n\n{:?}", self.error) } } @@ -513,6 +235,20 @@ impl std::error::Error for ANNError { always_escalate!(ANNError); +/// Define `From for ANNError` for a simple type `T` to a [`ANNError`] in a way that +/// preserves source location information of the conversion. +#[macro_export] +macro_rules! convert_error { + ($T:ty) => { + impl From<$T> for $crate::ANNError { + #[track_caller] + fn from(e: $T) -> $crate::ANNError { + $crate::ANNError::new(e) + } + } + }; +} + impl From for ANNError { #[track_caller] fn from(_: std::convert::Infallible) -> Self { @@ -520,13 +256,13 @@ impl From for ANNError { } } -// Convert from `io::Error` to `ANNError` -impl From for ANNError { - #[track_caller] - fn from(err: io::Error) -> Self { - ANNError::log_io_error(err) - } -} +convert_error!(io::Error); +convert_error!(LayoutError); +convert_error!(TryFromIntError); +convert_error!(TryFromSliceError); +convert_error!(diskann_utils::io::ReadBinError); +convert_error!(diskann_utils::io::SaveBinError); +convert_error!(diskann_utils::views::TryFromErrorLight); // Convert from `mpsc::SendError` to `ANNError` impl From> for ANNError @@ -535,45 +271,7 @@ where { #[track_caller] fn from(err: mpsc::SendError) -> Self { - ANNError::log_io_send_error(err) - } -} - -// Convert from `LayoutError` to `ANNError` -impl From for ANNError { - #[track_caller] - fn from(err: LayoutError) -> Self { - ANNError::log_mem_alloc_layout_error(err) - } -} - -// Convert from `TryFromIntError` to `ANNError` -impl From for ANNError { - #[track_caller] - fn from(err: TryFromIntError) -> Self { - ANNError::log_try_from_int_error(err) - } -} - -// Convert from `TryFromSliceError` to `ANNError` -impl From for ANNError { - #[track_caller] - fn from(err: TryFromSliceError) -> Self { - ANNError::log_try_from_slice_error(err) - } -} - -impl From for ANNError { - #[track_caller] - fn from(err: diskann_utils::io::ReadBinError) -> Self { - ANNError::new(ANNErrorKind::IOError, err) - } -} - -impl From for ANNError { - #[track_caller] - fn from(err: diskann_utils::io::SaveBinError) -> Self { - ANNError::new(ANNErrorKind::IOError, err) + ANNError::new(err) } } @@ -584,14 +282,7 @@ where { #[track_caller] fn from(err: diskann_utils::io::MetadataError) -> Self { - ANNError::new(ANNErrorKind::IOError, err) - } -} - -impl From for ANNError { - #[track_caller] - fn from(err: diskann_utils::views::TryFromErrorLight) -> Self { - ANNError::new(ANNErrorKind::DimensionMismatchError, err) + ANNError::new(err) } } @@ -659,10 +350,10 @@ where /// Add context to a returned error that will be included in the source chain. /// ```rust -/// use diskann::{ANNError, ANNErrorKind, error::ErrorContext}; +/// use diskann::{ANNError, error::ErrorContext}; /// /// fn fn_a() -> Result<(), ANNError> { -/// Err(ANNError::message(ANNErrorKind::IndexError, "thrown by function A")) +/// Err(ANNError::message("thrown by function A")) /// } /// /// fn fn_b() -> Result<(), ANNError> { @@ -747,206 +438,8 @@ where } } -// /// An internal macro for creating opaque, adhoc errors to help when debugging. -// macro_rules! ann_error { -// ($($arg:tt)+) => {{ -// ANNError::message(ANNErrorKind::Opaque, format!($($args)+)) -// }}; -// } -// -// pub(crate) use ann_error; - -////////////////// -// ANNErrorKind // -////////////////// - -/// DiskANN error kinds used to tag a returned error. -/// -/// Third-party implementations of DiskANN components (for example, custom implementations -/// of providers), can use the `Tagged` alternative to tag the error type for later -/// inspection. This mechanism can be used in coordination with the downcast API of -/// `ANNError` to retrieve the source error. -#[derive(Debug, PartialEq, Clone, Copy)] -pub enum ANNErrorKind { - /// The error originiated within DiskANN. - DiskANN(DiskANNError), - /// An error with a tagged type to help with recovery. Provider implementations may - /// choose to tag their errors if useful. - Tagged(u32), - /// An opaque error with no tag. - /// - /// This can be used by provider implementations that do not care to tag their errors. - Opaque, -} - -macro_rules! define_alias { - ($name:ident) => { - #[allow(non_upper_case_globals)] - pub const $name: ANNErrorKind = ANNErrorKind::DiskANN(DiskANNError::$name); - }; -} - -impl ANNErrorKind { - // Aliases - this is to maintain compatibility with an earlier version of the error type. - define_alias!(IndexError); - define_alias!(IndexConfigError); - define_alias!(TryFromIntError); - define_alias!(DimensionMismatchError); - define_alias!(FileNotFoundError); - define_alias!(FileHandleError); - define_alias!(AsyncIOThreadError); - define_alias!(GroundTruthError); - define_alias!(IOError); - define_alias!(IOSendError); - define_alias!(MemoryAllocLayoutError); - define_alias!(LockPoisonError); - define_alias!(DiskIOAlignmentError); - define_alias!(PQError); - define_alias!(OPQError); - define_alias!(KMeansError); - define_alias!(TryFromSliceError); - define_alias!(AdjacencyListConversionError); - define_alias!(SerdeError); - define_alias!(GetVertexDataError); - define_alias!(ParseSliceError); - define_alias!(ThreadPoolError); - define_alias!(NoExpectedNormError); - define_alias!(UnexpectedCheckpoint); - define_alias!(InvalidOperation); - define_alias!(PagedSearchError); - define_alias!(AsyncError); - define_alias!(AsyncShutdownError); - define_alias!(RustRuntimeError); - define_alias!(AsyncIndexError); - define_alias!(ProfilerError); - define_alias!(PQSchemaRegistrationError); - define_alias!(InvalidFileFormatError); - define_alias!(StartPointComputeError); - define_alias!(SQError); - define_alias!(BuildInterrupted); - - // Legacy Linux specific error. - define_alias!(PushError); -} - -/// A Internal errors yielded by DiskANN. -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum DiskANNError { - /// Index construction and search error - IndexError, - - /// Index configuration error - IndexConfigError, - - /// Integer conversion error - TryFromIntError, - - /// Dimension mismatch error - DimensionMismatchError, - - /// File does not exist - FileNotFoundError, - - /// Error with the file handle - FileHandleError, - - /// Error with async IO threading - AsyncIOThreadError, - - /// Error with ground-truth - GroundTruthError, - - /// IO error - IOError, - - /// IO SendError - IOSendError, - - /// Layout error in memory allocation - MemoryAllocLayoutError, - - /// PoisonError which can be returned whenever a lock is acquired - /// Both Mutexes and RwLocks are poisoned whenever a thread fails while the lock is held - LockPoisonError, - - /// DiskIOAlignmentError which can be returned when calling windows API CreateFileA for the disk index file fails. - DiskIOAlignmentError, - - // PQ construction error - // Error happened when we construct PQ pivot or PQ compressed table - PQError, - - // OPQ construction error (deprecated — kept to preserve variant numbering) - OPQError, - - // K-means error - // Error happened when we run k-means clustering - KMeansError, - - /// Array conversion error - TryFromSliceError, - - /// Array conversion error - AdjacencyListConversionError, - - /// Array conversion error - SerdeError, - - /// Error when we try to get the vertex data from vertex provider. - GetVertexDataError, - - /// Error when we try to parse a slice to an object. - ParseSliceError, - - ThreadPoolError, - - NoExpectedNormError, - - // Error when the checkpoint record is expected. - UnexpectedCheckpoint, - - // Generic invalid operation error. - InvalidOperation, - - PagedSearchError, - - AsyncError, - - AsyncShutdownError, - - RustRuntimeError, - - AsyncIndexError, - - ProfilerError, - - // Errors related to PQ schema registration. - PQSchemaRegistrationError, - - // Error when file format doesn't match expectations - InvalidFileFormatError, - - /// Error when index build process is intentionally interrupted - /// - /// This is not a true error, but a controlled interruption signal used to gracefully - /// exit from multi-level function calls in the build process. - BuildInterrupted, - - // Error when computing start point from data - StartPointComputeError, - - // SQ construction error - // Error happened when we build the SQ index - SQError, - - /// Linux io-uring error when pushing a task into the submission ring - PushError, -} - #[cfg(test)] mod ann_result_test { - use std::{alloc::Layout, array::TryFromSliceError, io}; - use super::*; #[test] @@ -962,8 +455,8 @@ mod ann_result_test { // registers. #[test] fn check_struct_size() { - assert_eq!(std::mem::size_of::(), 16); - assert_eq!(std::mem::size_of::>(), 16); + assert_eq!(std::mem::size_of::(), 8); + assert_eq!(std::mem::size_of::>(), 8); assert_eq!(std::mem::size_of::>(), 16); } @@ -985,13 +478,7 @@ mod ann_result_test { } impl std::error::Error for SampleError {} - - impl From for ANNError { - #[track_caller] - fn from(value: SampleError) -> ANNError { - ANNError::new(ANNErrorKind::Tagged(0), value) - } - } + convert_error!(SampleError); #[derive(Debug, Clone)] struct SampleChainedError { @@ -1023,11 +510,13 @@ mod ann_result_test { let base_error = err.to_string(); { let mut ann = ANNError::from(err.clone()); - assert_eq!(ann.kind(), ANNErrorKind::Tagged(0)); // Make sure the error message is properly contained inside the larger error. assert!(format!("{}", ann).contains(&base_error)); + // Make sure `is` is accurate. + assert!(ann.is::()); + // Can we downcast by reference? let r = ann.downcast_ref::().unwrap(); assert_eq!(r.value, 10); @@ -1050,6 +539,8 @@ mod ann_result_test { .context("some context here") .context("more context"); + assert!(ann.is::()); + let formatted = ann.to_string(); assert!(formatted.contains(&base_error)); assert!(formatted.contains("some context here")); @@ -1078,6 +569,8 @@ mod ann_result_test { .context("some context here") .context("more context"); + assert!(!ann.is::()); + println!("{}", ann); let formatted = ann.to_string(); @@ -1091,23 +584,13 @@ mod ann_result_test { } } - // Opaque - #[test] - fn test_opaque_constructor() { - let err = SampleError::new(50); - let ann = ANNError::opaque(err.clone()); - - assert_eq!(ann.kind(), ANNErrorKind::Opaque); - assert!(ann.to_string().contains(&err.to_string())); - } - // Context Chaining #[test] fn context_chaining() { let sample = SampleError::new(5).to_string(); fn err() -> Result { - Err(ANNError::new(ANNErrorKind::Tagged(42), SampleError::new(5))) + Err(ANNError::new(SampleError::new(5))) } fn ok() -> Result { @@ -1121,8 +604,8 @@ mod ann_result_test { let message = chained.to_string(); assert!(message.contains("with context"), "got: {}", message); assert!(message.contains(&sample), "got: {}", message); - assert_eq!(chained.kind(), ANNErrorKind::Tagged(42)); assert_eq!(chained.downcast_ref::().unwrap().value, 5); + assert!(chained.is::()); } // Context not applied if okay. @@ -1147,7 +630,6 @@ mod ann_result_test { let message = chained.to_string(); assert!(message.contains("with context"), "got: {}", message); assert!(message.contains(&sample), "got: {}", message); - assert_eq!(chained.kind(), ANNErrorKind::Tagged(42)); assert_eq!(chained.downcast_ref::().unwrap().value, 5); } @@ -1173,7 +655,7 @@ mod ann_result_test { let err = err.context("more context"); let expected = format!( - "ANNError: Tagged(0) + "ANNError more context -- ({}:{}) @@ -1199,14 +681,14 @@ Caused by: let file = file!(); let l0 = line!() + 1; - let err = ANNError::new(ANNErrorKind::Tagged(0), sample); + let err = ANNError::new(sample); let l1 = line!() + 1; let err = err.context("some context"); let l2 = line!() + 1; let err = err.context("more context"); let expected = format!( - "ANNError: Tagged(0) + "ANNError more context -- ({}:{}) @@ -1232,10 +714,10 @@ Caused by: let file = file!(); let l0 = line!() + 1; - let err = ANNError::new(ANNErrorKind::Tagged(0), sample); + let err = ANNError::new(sample); let expected = format!( - "ANNError: Tagged(0) + "ANNError SampleChainedError {{ 10 }} -- ({}:{}) @@ -1252,310 +734,4 @@ Caused by: expected ); } - - ///////////////////////// - // Direct Constructors // - ///////////////////////// - - #[test] - fn test_log_disk_io_request_alignment_error() { - let err_msg = "Disk I/O request alignment error"; - let ann_err = ANNError::log_disk_io_request_alignment_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::DiskIOAlignmentError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_mem_alloc_layout_error() { - let layout_err = std::alloc::Layout::from_size_align(0, 0).unwrap_err(); - let formatted = layout_err.to_string(); - let ann_err = ANNError::log_mem_alloc_layout_error(layout_err); - assert_eq!(ANNErrorKind::MemoryAllocLayoutError, ann_err.kind()); - assert!(ann_err.to_string().contains(&formatted)); - } - - #[test] - fn test_log_lock_poison_error() { - let err_msg = "Lock poison error"; - let ann_err = ANNError::log_lock_poison_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::LockPoisonError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_serde_error() { - let op = "serialize"; - let err = "custom error".to_string(); - let ann_err = ANNError::log_serde_error(op.to_string(), &err); - assert_eq!(ANNErrorKind::SerdeError, ann_err.kind()); - - let formatted = ann_err.to_string(); - assert!(formatted.contains(op)); - assert!(formatted.contains(&err)); - } - - #[test] - fn test_log_get_vertex_data_error() { - let id = "vertex_id".to_string(); - let data_t = "data_type".to_string(); - let ann_err = ANNError::log_get_vertex_data_error(id.clone(), data_t.clone()); - assert_eq!(ANNErrorKind::GetVertexDataError, ann_err.kind()); - - let formatted = ann_err.to_string(); - assert!(formatted.contains(&id)); - assert!(formatted.contains(&data_t)); - } - - #[test] - fn test_log_parse_slice_error() { - let parsing_source = "source".to_string(); - let parsing_target = "target".to_string(); - let err = "error".to_string(); - let ann_err = ANNError::log_parse_slice_error( - parsing_source.clone(), - parsing_target.clone(), - err.clone(), - ); - assert_eq!(ANNErrorKind::ParseSliceError, ann_err.kind()); - - let formatted = ann_err.to_string(); - assert!(formatted.contains(&parsing_source)); - assert!(formatted.contains(&parsing_target)); - assert!(formatted.contains(&err)); - } - - #[test] - fn test_log_try_from_slice_error() { - let mut bytes: [u8; 3] = [1, 0, 2]; - let bytes_head = <[u8; 2]>::try_from(&mut bytes[1..2]); - let ann_err = ANNError::log_try_from_slice_error(bytes_head.unwrap_err()); - assert_eq!(ANNErrorKind::TryFromSliceError, ann_err.kind()); - } - - #[test] - fn test_log_try_from_int_error() { - let err = u8::try_from(-1i8); - let ann_err = ANNError::log_try_from_int_error(err.unwrap_err()); - assert_eq!(ANNErrorKind::TryFromIntError, ann_err.kind()); - } - - #[test] - fn test_thread_pool_error() { - let err_msg = "Thread pool error"; - let ann_err = ANNError::log_thread_pool_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::ThreadPoolError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_invalid_operation_error() { - let err_msg = "Invalid operation error"; - let ann_err = ANNError::log_invalid_operation_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::InvalidOperation, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_async_error() { - let err_msg = "Async error"; - let ann_err = ANNError::log_async_error(err_msg); - assert_eq!(ANNErrorKind::AsyncError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_async_index_error() { - let err_msg = "Async index error"; - let ann_err = ANNError::log_async_index_error(err_msg); - assert_eq!(ANNErrorKind::AsyncIndexError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_async_shutdown_error() { - let err_msg = "Async shutdown error"; - let ann_err = ANNError::log_async_shutdown_error(err_msg); - assert_eq!(ANNErrorKind::AsyncShutdownError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_async_runtime_error() { - let err_msg = "Async runtime error"; - let ann_err = ANNError::log_async_runtime_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::RustRuntimeError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_dimension_mismatch_error() { - let err_msg = "Dimension mismatch error"; - let ann_err = ANNError::log_dimension_mismatch_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::DimensionMismatchError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_profiler_error() { - let err_msg = "Profiler error"; - let ann_err = ANNError::log_profiler_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::ProfilerError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_pq_schema_registration_error() { - let err_msg = "PQ schema registration error"; - let ann_err = ANNError::log_pq_schema_registration_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::PQSchemaRegistrationError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_kmeans_error() { - let err_msg = "KMeans error"; - let ann_err = ANNError::log_kmeans_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::KMeansError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_io_send_error() { - let err_msg = "IO send error"; - let send_err: mpsc::SendError = mpsc::SendError(err_msg.to_string()); - let expected = send_err.to_string(); - let ann_err = ANNError::log_io_send_error(send_err); - assert_eq!(ANNErrorKind::IOSendError, ann_err.kind()); - assert!(ann_err.to_string().contains(&expected)); - } - - #[test] - fn test_log_file_handle_error() { - let err_msg = "File handle error"; - let ann_err = ANNError::log_file_handle_error(err_msg); - assert_eq!(ANNErrorKind::FileHandleError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_file_not_found_error() { - let err_msg = "File not found error"; - let ann_err = ANNError::log_file_not_found_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::FileNotFoundError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_log_ground_truth_error() { - let err_msg = "Ground truth error"; - let ann_err = ANNError::log_ground_truth_error(err_msg.to_string()); - assert_eq!(ANNErrorKind::GroundTruthError, ann_err.kind()); - assert!(ann_err.to_string().contains(err_msg)); - } - - #[test] - fn test_io_error_to_ann_error() { - let io_err = io::Error::other("test error"); - let expected = io_err.to_string(); - let ann_err: ANNError = ANNError::from(io_err); - assert_eq!(ann_err.kind(), ANNErrorKind::IOError); - assert!(ann_err.to_string().contains(&expected)); - } - - #[test] - fn test_send_error_to_ann_error() { - let send_err = mpsc::SendError(()); - let expected = send_err.to_string(); - let ann_err: ANNError = send_err.into(); - assert_eq!(ann_err.kind(), ANNErrorKind::IOSendError); - assert!(ann_err.to_string().contains(&expected)); - } - - #[test] - fn test_layout_error_to_ann_error() { - let layout_err = Layout::from_size_align(1, 0).unwrap_err(); - let ann_err: ANNError = layout_err.into(); - assert_eq!(ann_err.kind(), ANNErrorKind::MemoryAllocLayoutError); - } - - #[test] - fn test_try_from_int_error_to_ann_error() { - let err = u8::try_from(1_000usize).unwrap_err(); - let ann_err: ANNError = err.into(); - assert_eq!(ann_err.kind(), ANNErrorKind::TryFromIntError); - } - - #[test] - fn test_try_from_slice_error_to_ann_error() { - let slice: &[u8] = &[1, 2, 3]; - let slice_err: Result<[u8; 4], TryFromSliceError> = slice.try_into(); - let err = slice_err.unwrap_err(); - let ann_err: ANNError = err.into(); - assert_eq!(ann_err.kind(), ANNErrorKind::TryFromSliceError); - } - - #[test] - fn test_display_ann_error() { - let err = ANNErrorKind::IndexError; - assert_eq!(format!("{:?}", err), "DiskANN(IndexError)"); - } - - #[test] - fn test_invaild_file_format_error() { - let err_msg = String::from("Invalid file format error"); - let ann_err = ANNError::log_invalid_file_format(err_msg.clone()); - assert_eq!(ann_err.kind(), ANNErrorKind::InvalidFileFormatError); - } - - #[test] - fn test_build_interrupted() { - let message = "BuildIndicesOnShards"; - let ann_err = ANNError::log_build_interrupted(message); - assert_eq!(ann_err.kind(), ANNErrorKind::BuildInterrupted); - } - - #[test] - fn from_read_bin_error() { - let err = diskann_utils::io::ReadBinError::SizeMismatch { - expected: 100, - available: 50, - npoints: 10, - ndims: 5, - type_size: 2, - }; - let ann_err = ANNError::from(err); - assert_eq!(ann_err.kind(), ANNErrorKind::IOError); - } - - #[test] - fn from_save_bin_error() { - let err = diskann_utils::io::SaveBinError::DimensionOverflow { nrows: 1, ncols: 1 }; - let ann_err = ANNError::from(err); - assert_eq!(ann_err.kind(), ANNErrorKind::IOError); - } - - #[test] - fn from_metadata_error() { - let err = diskann_utils::io::Metadata::new(u64::MAX, 1u32).unwrap_err(); - let ann_err = ANNError::from(err); - assert_eq!(ann_err.kind(), ANNErrorKind::IOError); - } - - #[test] - fn from_try_from_error_light() { - let data: &[f32] = &[1.0, 2.0, 3.0]; - let light = diskann_utils::views::MatrixView::try_from(data, 2, 2) - .unwrap_err() - .as_static(); - let ann_err = ANNError::from(light); - assert_eq!(ann_err.kind(), ANNErrorKind::DimensionMismatchError); - } - - #[test] - fn from_try_from_error() { - let data: &[f32] = &[1.0, 2.0, 3.0]; - let err = diskann_utils::views::MatrixView::try_from(data, 2, 2).unwrap_err(); - let ann_err = ANNError::from(err); - assert_eq!(ann_err.kind(), ANNErrorKind::DimensionMismatchError); - } } diff --git a/diskann/src/error/mod.rs b/diskann/src/error/mod.rs index 3a9f9ab50..3a40a9e8b 100644 --- a/diskann/src/error/mod.rs +++ b/diskann/src/error/mod.rs @@ -4,7 +4,7 @@ */ pub(crate) mod ann_error; -pub use ann_error::{ANNError, ANNErrorKind, ANNResult, DiskANNError, ErrorContext, IntoANNResult}; +pub use ann_error::{ANNError, ANNResult, ErrorContext, IntoANNResult}; pub(crate) mod ranked; pub use ranked::{ErrorExt, Infallible, NeverTransient, RankedError, ToRanked, TransientError}; @@ -14,11 +14,8 @@ impl StandardError for T where T: std::error::Error + Send + Sync + 'static + #[cfg(any(test, feature = "testing"))] macro_rules! message { - ($kind:ident, $($args:tt)*) => { - $crate::ANNError::message($kind, format!($($args)*)) - }; ($($args:tt)*) => { - $crate::ANNError::message($crate::ANNErrorKind::Opaque, format!($($args)*)) + $crate::ANNError::message(format!($($args)*)) }; } diff --git a/diskann/src/error/ranked.rs b/diskann/src/error/ranked.rs index b10253e18..9ae0ab20a 100644 --- a/diskann/src/error/ranked.rs +++ b/diskann/src/error/ranked.rs @@ -455,12 +455,7 @@ mod tests { #[error("generic error message: {0}")] struct AlwaysEscalate(usize); - impl From for ANNError { - fn from(value: AlwaysEscalate) -> ANNError { - ANNError::log_index_error(value) - } - } - + crate::convert_error!(AlwaysEscalate); always_escalate!(AlwaysEscalate); #[test] @@ -559,8 +554,8 @@ mod tests { impl From> for ANNError { #[track_caller] - fn from(value: Disarmed<'_>) -> ANNError { - ANNError::log_index_error(&value) + fn from(err: Disarmed<'_>) -> Self { + Self::message(err.to_string()) } } @@ -842,7 +837,7 @@ mod tests { // We can't actually construct an Infallible value to test this directly since it's unconstructable // But we can verify the From implementation exists by checking the type constraint fn _test_infallible_into_ann_error(_: Infallible) -> ANNError { - ANNError::log_index_error("This should never be called") + ANNError::message("This should never be called") } } } diff --git a/diskann/src/flat/strategy.rs b/diskann/src/flat/strategy.rs index c6e3f527d..1cdb6957f 100644 --- a/diskann/src/flat/strategy.rs +++ b/diskann/src/flat/strategy.rs @@ -103,7 +103,7 @@ mod tests { use diskann_vector::{PreprocessedDistanceFunction, distance::Metric}; use super::*; - use crate::{ANNError, always_escalate, error::Infallible, utils::VectorRepr}; + use crate::{always_escalate, convert_error, error::Infallible, utils::VectorRepr}; /// Sample dataset shared by every test below. fn sample_items() -> Vec<(u32, Vec)> { @@ -182,13 +182,7 @@ mod tests { struct Boom(u32); always_escalate!(Boom); - - impl From for ANNError { - #[track_caller] - fn from(boom: Boom) -> ANNError { - ANNError::opaque(boom) - } - } + convert_error!(Boom); /// Scans `items`, but returns `Err(Boom(id))` exactly once after `fail_after` /// successful yields. diff --git a/diskann/src/flat/test/provider.rs b/diskann/src/flat/test/provider.rs index 6f8a14136..3bd0f3d31 100644 --- a/diskann/src/flat/test/provider.rs +++ b/diskann/src/flat/test/provider.rs @@ -18,7 +18,7 @@ use diskann_vector::{PreprocessedDistanceFunction, distance::Metric}; use thiserror::Error; use crate::{ - ANNError, always_escalate, + always_escalate, convert_error, error::{RankedError, ToRanked, TransientError}, flat::{DistancesUnordered, SearchStrategy}, graph::test::synthetic::Grid, @@ -36,12 +36,7 @@ pub enum ProviderError { ZeroDimension, } -impl From for ANNError { - #[track_caller] - fn from(err: ProviderError) -> ANNError { - ANNError::opaque(err) - } -} +convert_error!(ProviderError); ////////////// // Provider // @@ -154,13 +149,7 @@ impl ExecutionContext for Context { pub struct InvalidId(pub u32); always_escalate!(InvalidId); - -impl From for ANNError { - #[track_caller] - fn from(err: InvalidId) -> ANNError { - ANNError::opaque(err) - } -} +convert_error!(InvalidId); /// Transient access error injected by [`Visitor::flaky`]. /// @@ -372,12 +361,7 @@ pub struct StrategyError { pub actual: usize, } -impl From for ANNError { - #[track_caller] - fn from(err: StrategyError) -> ANNError { - ANNError::opaque(err) - } -} +convert_error!(StrategyError); /// Factory of [`Visitor`]s that validates dimensions and optionally injects /// transient errors into the scan. diff --git a/diskann/src/graph/config/mod.rs b/diskann/src/graph/config/mod.rs index b6a32895c..6888cb2eb 100644 --- a/diskann/src/graph/config/mod.rs +++ b/diskann/src/graph/config/mod.rs @@ -433,11 +433,7 @@ pub struct ConfigError { inner: ConfigErrorInner, } -impl From for crate::ANNError { - fn from(error: ConfigError) -> Self { - crate::ANNError::new(crate::ANNErrorKind::IndexConfigError, error) - } -} +crate::convert_error!(ConfigError); #[derive(Debug, Clone, Error)] enum ConfigErrorInner { diff --git a/diskann/src/graph/index.rs b/diskann/src/graph/index.rs index 1df48072f..83134d984 100644 --- a/diskann/src/graph/index.rs +++ b/diskann/src/graph/index.rs @@ -39,7 +39,7 @@ use super::{ }; use crate::{ - ANNError, ANNErrorKind, ANNResult, + ANNError, ANNResult, error::{ErrorExt, IntoANNResult}, internal, neighbor::{self, Neighbor, NeighborQueue}, @@ -493,13 +493,10 @@ where { async move { if batch.len() != ids.len() { - return Err(ANNError::new( - ANNErrorKind::IndexError, - BatchIdMismatch { - batch_len: batch.len(), - ids_len: ids.len(), - }, - )); + return Err(ANNError::new(BatchIdMismatch { + batch_len: batch.len(), + ids_len: ids.len(), + })); } let partitions = async_tools::PartitionIter::new(batch.len(), ntasks); @@ -526,9 +523,7 @@ where let mut guards = Vec::with_capacity(batch.len()); for h in handles { - let processed = h - .await - .map_err(|err| ANNError::new(ANNErrorKind::IndexError, err))??; + let processed = h.await.map_err(ANNError::new)??; for guard in processed { guards.push(guard); } @@ -1401,7 +1396,7 @@ where #[error("Spawning a task failed in inplace-delete: {0}")] struct LocalError(tokio::task::JoinError); - ANNError::log_async_error(LocalError(err)) + ANNError::new(LocalError(err)) }); edge_collection.push(res); } @@ -1458,7 +1453,7 @@ where loop { let result = { let mut guard = edges_clone.lock().map_err(|_| { - ANNError::log_async_error("Poisoned mutex during construction") + ANNError::message("Poisoned mutex during construction") })?; guard.next() }; diff --git a/diskann/src/graph/internal/prune.rs b/diskann/src/graph/internal/prune.rs index f5148be96..39c727e88 100644 --- a/diskann/src/graph/internal/prune.rs +++ b/diskann/src/graph/internal/prune.rs @@ -7,9 +7,7 @@ use thiserror::Error; use super::SortedNeighbors; -use crate::{ - ANNError, ANNErrorKind, error, graph::AdjacencyList, neighbor::Neighbor, utils::VectorId, -}; +use crate::{ANNError, error, graph::AdjacencyList, neighbor::Neighbor, utils::VectorId}; /// Options provided to prune. See the field-level documentation for more details. /// @@ -114,7 +112,7 @@ where where D: std::fmt::Display, { - ANNError::new(ANNErrorKind::IndexError, self).context(why.to_string()) + ANNError::new(self).context(why.to_string()) } } diff --git a/diskann/src/graph/search/inline_filter_search.rs b/diskann/src/graph/search/inline_filter_search.rs index d2e84bb08..cfcb66b68 100644 --- a/diskann/src/graph/search/inline_filter_search.rs +++ b/diskann/src/graph/search/inline_filter_search.rs @@ -10,7 +10,7 @@ use thiserror::Error; use super::{Knn, Search, record::SearchRecord, scratch::SearchScratch}; use crate::{ - ANNError, ANNErrorKind, ANNResult, + ANNResult, convert_error, error::IntoANNResult, graph::{ glue::{self, FilteredAccessor, SearchPostProcess, SearchStrategy}, @@ -31,12 +31,7 @@ pub enum AdaptiveLSearchError { SampleCountZero, } -impl From for ANNError { - #[track_caller] - fn from(err: AdaptiveLSearchError) -> Self { - Self::new(ANNErrorKind::IndexError, err) - } -} +convert_error!(AdaptiveLSearchError); /// Adaptive L for inline filtered search. #[derive(Debug, Clone)] diff --git a/diskann/src/graph/search/knn_search.rs b/diskann/src/graph/search/knn_search.rs index 9f61f1a10..73379d67f 100644 --- a/diskann/src/graph/search/knn_search.rs +++ b/diskann/src/graph/search/knn_search.rs @@ -12,7 +12,7 @@ use thiserror::Error; use super::Search; use crate::{ - ANNError, ANNErrorKind, ANNResult, + ANNResult, convert_error, error::IntoANNResult, graph::{ glue::{SearchAccessor, SearchPostProcess, SearchStrategy}, @@ -36,12 +36,7 @@ pub enum KnnSearchError { LZero, } -impl From for ANNError { - #[track_caller] - fn from(err: KnnSearchError) -> Self { - Self::new(ANNErrorKind::IndexError, err) - } -} +convert_error!(KnnSearchError); /// Standard k-NN (k-nearest neighbor) graph-based search parameters. /// diff --git a/diskann/src/graph/search/paged.rs b/diskann/src/graph/search/paged.rs index e2cff5464..08aff2382 100644 --- a/diskann/src/graph/search/paged.rs +++ b/diskann/src/graph/search/paged.rs @@ -56,14 +56,12 @@ where ) -> impl SendFuture>>> { async move { if k > self.search_param_l { - return ANNResult::Err(ANNError::log_paged_search_error( - "k should be less than or equal to search_param_l".to_string(), + return ANNResult::Err(ANNError::message( + "k should be less than or equal to search_param_l", )); } if k == 0 { - return ANNResult::Err(ANNError::log_paged_search_error( - "k should be greater than 0".to_string(), - )); + return ANNResult::Err(ANNError::message("k should be greater than 0")); } let mut result = Vec::with_capacity(k); diff --git a/diskann/src/graph/search/range_search.rs b/diskann/src/graph/search/range_search.rs index dbe5fec78..3b3df35b5 100644 --- a/diskann/src/graph/search/range_search.rs +++ b/diskann/src/graph/search/range_search.rs @@ -10,7 +10,7 @@ use thiserror::Error; use super::{Search, scratch::SearchScratch}; use crate::{ - ANNError, ANNErrorKind, ANNResult, + ANNResult, convert_error, error::IntoANNResult, graph::{ glue::{self, SearchAccessor, SearchStrategy}, @@ -40,12 +40,7 @@ pub enum RangeSearchError { MaxReturnedLessThanInitialL, } -impl From for ANNError { - #[track_caller] - fn from(err: RangeSearchError) -> Self { - Self::new(ANNErrorKind::IndexError, err) - } -} +convert_error!(RangeSearchError); /// Parameters for range-based search. /// diff --git a/diskann/src/graph/test/provider.rs b/diskann/src/graph/test/provider.rs index dd0e7082c..c1432bf1c 100644 --- a/diskann/src/graph/test/provider.rs +++ b/diskann/src/graph/test/provider.rs @@ -18,7 +18,7 @@ use diskann_vector::{PreprocessedDistanceFunction, distance::Metric}; use thiserror::Error; use crate::{ - ANNError, ANNResult, default_post_processor, + ANNResult, convert_error, default_post_processor, error::ranked::ErrorExt, error::{Infallible, RankedError, StandardError, ToRanked, TransientError, message}, graph::{AdjacencyList, SearchOutputBuffer, glue, test::synthetic, workingset}, @@ -28,6 +28,9 @@ use crate::{ utils::VectorRepr, }; +#[cfg(any(test, feature = "testing"))] +use crate::ANNError; + /// A starting point for graph search algorithms. /// /// # Examples @@ -173,12 +176,7 @@ pub enum ConfigError { MaxDegreeCannotBeZero, } -impl From for ANNError { - #[track_caller] - fn from(err: ConfigError) -> Self { - ANNError::opaque(err) - } -} +convert_error!(ConfigError); /// A test data provider for validating DiskANN API guarantees. /// @@ -457,7 +455,7 @@ impl Provider { neighbors.overwrite_trusted(&v.neighbors); Ok(()) } - None => Err(ANNError::opaque(AccessedInvalidId(id))), + None => Err(ANNError::new(AccessedInvalidId(id))), } } @@ -665,13 +663,7 @@ pub enum InvalidId { } crate::always_escalate!(InvalidId); - -impl From for ANNError { - #[track_caller] - fn from(err: InvalidId) -> ANNError { - ANNError::opaque(err) - } -} +convert_error!(InvalidId); impl provider::DataProvider for Provider { type Context = Context; @@ -774,13 +766,7 @@ impl provider::SetElement<&[f32]> for Provider { } crate::always_escalate!(SetError); - - impl From for ANNError { - #[track_caller] - fn from(err: SetError) -> Self { - Self::new(crate::ANNErrorKind::IndexError, err) - } - } + convert_error!(SetError); // Ensure that the assigned vector has the correct length. if element.len() != self.dim() { @@ -810,13 +796,7 @@ impl provider::SetElement<&[f32]> for Provider { pub struct AccessedInvalidId(u32); crate::always_escalate!(AccessedInvalidId); - -impl From for ANNError { - #[track_caller] - fn from(err: AccessedInvalidId) -> Self { - Self::opaque(err) - } -} +convert_error!(AccessedInvalidId); /// A transient error from the test accessor — the ID exists but the retrieval /// temporarily failed. Must be acknowledged or escalated before being dropped. @@ -975,7 +955,7 @@ impl provider::NeighborAccessorMut for NeighborAccessor<'_> { Ok(()) } } - None => Err(ANNError::opaque(AccessedInvalidId(id))), + None => Err(ANNError::new(AccessedInvalidId(id))), } } @@ -1002,7 +982,7 @@ impl provider::NeighborAccessorMut for NeighborAccessor<'_> { Ok(()) } } - None => Err(ANNError::opaque(AccessedInvalidId(id))), + None => Err(ANNError::new(AccessedInvalidId(id))), } } } @@ -1195,12 +1175,7 @@ pub struct DimMismatch { expected: usize, } -impl From for ANNError { - #[track_caller] - fn from(mismatch: DimMismatch) -> Self { - ANNError::opaque(mismatch) - } -} +convert_error!(DimMismatch); impl provider::HasId for Accessor<'_> { type Id = u32; diff --git a/diskann/src/lib.rs b/diskann/src/lib.rs index 5931f404f..388dd30b1 100644 --- a/diskann/src/lib.rs +++ b/diskann/src/lib.rs @@ -17,7 +17,7 @@ pub mod flat; pub mod graph; // Top level exports. -pub use error::ann_error::{ANNError, ANNErrorKind, ANNResult}; +pub use error::ann_error::{ANNError, ANNResult}; /// Returns the version of the DiskANN crate. pub fn version() -> &'static str { diff --git a/diskann/src/test/cmp.rs b/diskann/src/test/cmp.rs index e3f9dd488..19cc670bd 100644 --- a/diskann/src/test/cmp.rs +++ b/diskann/src/test/cmp.rs @@ -130,7 +130,7 @@ macro_rules! impl_via_partial_eq { impl $crate::test::cmp::VerboseEq for $T { fn verbose_eq(&self, other: &Self) -> $crate::ANNResult<()> { if self != other { - Err($crate::ANNError::opaque(NotEq(self.clone(), other.clone()))) + Err($crate::ANNError::new(NotEq(self.clone(), other.clone()))) } else { Ok(()) } @@ -201,7 +201,7 @@ where use crate::error::ErrorContext; if self.len() != other.len() { - return Err(ANNError::opaque(UnequalLengths(self.len(), other.len()))); + return Err(ANNError::new(UnequalLengths(self.len(), other.len()))); } self.iter() @@ -244,7 +244,7 @@ where match (self, other) { (Some(lhs), Some(rhs)) => lhs.verbose_eq(rhs), (None, None) => Ok(()), - _ => Err(ANNError::opaque(NotEq(self.clone(), other.clone()))), + _ => Err(ANNError::new(NotEq(self.clone(), other.clone()))), } } } diff --git a/diskann/src/utils/async_tools.rs b/diskann/src/utils/async_tools.rs index 87f72b7e0..1a667bf0c 100644 --- a/diskann/src/utils/async_tools.rs +++ b/diskann/src/utils/async_tools.rs @@ -14,7 +14,7 @@ use std::{ use thiserror::Error; -use crate::ANNError; +use crate::convert_error; //////////// // Around // @@ -370,11 +370,7 @@ pub struct PartitionError { task: usize, } -impl From for ANNError { - fn from(err: PartitionError) -> Self { - Self::log_async_error(err) - } -} +convert_error!(PartitionError); #[cfg(test)] mod tests { diff --git a/diskann/src/utils/vector_repr.rs b/diskann/src/utils/vector_repr.rs index 4f33caab3..1b8e8dbda 100644 --- a/diskann/src/utils/vector_repr.rs +++ b/diskann/src/utils/vector_repr.rs @@ -110,14 +110,7 @@ pub struct NativeTypeLengthError { dst: usize, } -impl From for ANNError { - fn from(err: NativeTypeLengthError) -> ANNError { - ANNError::log_index_error(format!( - "Unable to set full-precision vector of length {} into slice of length {}", - err.src, err.dst - )) - } -} +crate::convert_error!(NativeTypeLengthError); macro_rules! default_impl { (