Skip to content

Commit

Permalink
feat: add location for all error (#1475)
Browse files Browse the repository at this point in the history
Closed #1275
  • Loading branch information
Weijun-H authored Nov 2, 2023
1 parent c82edbc commit 6224356
Show file tree
Hide file tree
Showing 38 changed files with 288 additions and 93 deletions.
4 changes: 4 additions & 0 deletions python/src/dataset/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

use std::fmt::Debug;

use snafu::{location, Location};

use lance_core::io::commit::{CommitError, CommitLease, CommitLock};
use lance_core::Error;

Expand All @@ -36,6 +38,7 @@ fn handle_error(py_err: PyErr, py: Python) -> CommitError {
Err(import_error) => {
return CommitError::OtherError(Error::Internal {
message: format!("Error importing from pylance {}", import_error),
location: location!(),
})
}
};
Expand All @@ -45,6 +48,7 @@ fn handle_error(py_err: PyErr, py: Python) -> CommitError {
} else {
CommitError::OtherError(Error::Internal {
message: format!("Error from commit handler: {}", py_err),
location: location!(),
})
}
}
Expand Down
1 change: 1 addition & 0 deletions rust/lance-core/src/datatypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ fn parse_timeunit(unit: &str) -> Result<TimeUnit> {
"ns" => Ok(TimeUnit::Nanosecond),
_ => Err(Error::Arrow {
message: format!("Unsupported TimeUnit: {unit}"),
location: location!(),
}),
}
}
Expand Down
2 changes: 2 additions & 0 deletions rust/lance-core/src/datatypes/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ impl Field {
"Attempt to intersect different fields: {} and {}",
self.name, other.name,
),
location: location!(),
});
}
let self_type = self.data_type();
Expand Down Expand Up @@ -347,6 +348,7 @@ impl Field {
"Attempt to intersect different fields: ({}, {}) and ({}, {})",
self.name, self_type, other.name, other_type
),
location: location!(),
});
}

Expand Down
3 changes: 3 additions & 0 deletions rust/lance-core/src/encodings/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ impl<'a> DictionaryDecoder<'a> {
} else {
return Err(Error::Arrow {
message: format!("Not a dictionary type: {}", self.data_type),
location: location!(),
});
};

Expand All @@ -160,6 +161,7 @@ impl<'a> DictionaryDecoder<'a> {
DataType::UInt64 => self.make_dict_array::<UInt64Type>(keys).await,
_ => Err(Error::Arrow {
message: format!("Dictionary encoding does not support index type: {index_type}",),
location: location!(),
}),
}
}
Expand Down Expand Up @@ -196,6 +198,7 @@ impl<'a> AsyncIndex<usize> for DictionaryDecoder<'a> {
source: "DictionaryDecoder does not support get()"
.to_string()
.into(),
location: location!(),
})
}
}
Expand Down
70 changes: 47 additions & 23 deletions rust/lance-core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,56 +27,77 @@ pub fn box_error(e: impl std::error::Error + Send + Sync + 'static) -> BoxedErro
#[derive(Debug, Snafu)]
#[snafu(visibility(pub))]
pub enum Error {
#[snafu(display("Invalid user input: {source}"))]
InvalidInput { source: BoxedError },
#[snafu(display("Dataset already exists: {uri}"))]
DatasetAlreadyExists { uri: String },
#[snafu(display("Invalid user input: {source}, {location}"))]
InvalidInput {
source: BoxedError,
location: Location,
},
#[snafu(display("Dataset already exists: {uri}, {location}"))]
DatasetAlreadyExists { uri: String, location: Location },
// #[snafu(display("Append with different schema: original={original} new={new}"))]
#[snafu(display("Append with different schema:"))]
SchemaMismatch {},
#[snafu(display("Dataset at path {path} was not found: {source}"))]
DatasetNotFound { path: String, source: BoxedError },
#[snafu(display("Encountered corrupt file {path}: {source}"))]
#[snafu(display("Dataset at path {path} was not found: {source}, {location}"))]
DatasetNotFound {
path: String,
source: BoxedError,
location: Location,
},
#[snafu(display("Encountered corrupt file {path}: {source}, {location}"))]
CorruptFile {
path: object_store::path::Path,
source: BoxedError,
location: Location,
// TODO: add backtrace?
},
#[snafu(display("Not supported: {source}"))]
NotSupported { source: BoxedError },
#[snafu(display("Commit conflict for version {version}: {source}"))]
CommitConflict { version: u64, source: BoxedError },
#[snafu(display("Encountered internal error. Please file a bug report at https://github.com/lancedb/lance/issues. {message}"))]
Internal { message: String },
#[snafu(display("A prerequisite task failed: {message}"))]
PrerequisiteFailed { message: String },
#[snafu(display("LanceError(Arrow): {message}"))]
Arrow { message: String },
#[snafu(display("Not supported: {source}, {location}"))]
NotSupported {
source: BoxedError,
location: Location,
},
#[snafu(display("Commit conflict for version {version}: {source}, {location}"))]
CommitConflict {
version: u64,
source: BoxedError,
location: Location,
},
#[snafu(display("Encountered internal error. Please file a bug report at https://github.com/lancedb/lance/issues. {message}, {location}"))]
Internal { message: String, location: Location },
#[snafu(display("A prerequisite task failed: {message}, {location}"))]
PrerequisiteFailed { message: String, location: Location },
#[snafu(display("LanceError(Arrow): {message}, {location}"))]
Arrow { message: String, location: Location },
#[snafu(display("LanceError(Schema): {message}, {location}"))]
Schema { message: String, location: Location },
#[snafu(display("Not found: {uri}, {location}"))]
NotFound { uri: String, location: Location },
#[snafu(display("LanceError(IO): {message}, {location}"))]
IO { message: String, location: Location },
#[snafu(display("LanceError(Index): {message}"))]
Index { message: String },
#[snafu(display("LanceError(Index): {message}, {location}"))]
Index { message: String, location: Location },
/// Stream early stop
Stop,
}

impl Error {
pub fn corrupt_file(path: object_store::path::Path, message: impl Into<String>) -> Self {
pub fn corrupt_file(
path: object_store::path::Path,
message: impl Into<String>,
location: Location,
) -> Self {
let message: String = message.into();
Self::CorruptFile {
path,
source: message.into(),
location,
}
}

pub fn invalid_input(message: impl Into<String>) -> Self {
pub fn invalid_input(message: impl Into<String>, location: Location) -> Self {
let message: String = message.into();
Self::InvalidInput {
source: message.into(),
location,
}
}
}
Expand All @@ -87,6 +108,7 @@ impl From<ArrowError> for Error {
fn from(e: ArrowError) -> Self {
Self::Arrow {
message: e.to_string(),
location: location!(),
}
}
}
Expand All @@ -95,6 +117,7 @@ impl From<&ArrowError> for Error {
fn from(e: &ArrowError) -> Self {
Self::Arrow {
message: e.to_string(),
location: location!(),
}
}
}
Expand Down Expand Up @@ -157,6 +180,7 @@ impl From<serde_json::Error> for Error {
fn from(e: serde_json::Error) -> Self {
Self::Arrow {
message: e.to_string(),
location: location!(),
}
}
}
Expand All @@ -171,10 +195,10 @@ fn arrow_io_error_from_msg(message: String) -> ArrowError {
impl From<Error> for ArrowError {
fn from(value: Error) -> Self {
match value {
Error::Arrow { message } => arrow_io_error_from_msg(message), // we lose the error type converting to LanceError
Error::Arrow { message, .. } => arrow_io_error_from_msg(message), // we lose the error type converting to LanceError
Error::IO { message, .. } => arrow_io_error_from_msg(message),
Error::Schema { message, .. } => Self::SchemaError(message),
Error::Index { message } => arrow_io_error_from_msg(message),
Error::Index { message, .. } => arrow_io_error_from_msg(message),
Error::Stop => arrow_io_error_from_msg("early stop".to_string()),
e => arrow_io_error_from_msg(e.to_string()), // Find a more scalable way of doing this
}
Expand Down
2 changes: 2 additions & 0 deletions rust/lance-core/src/format/page_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use arrow_array::builder::Int64Builder;
use arrow_array::{Array, Int64Array};
use arrow_schema::DataType;
use snafu::{location, Location};
use std::collections::BTreeMap;
use tokio::io::AsyncWriteExt;

Expand Down Expand Up @@ -96,6 +97,7 @@ impl PageTable {
if self.pages.is_empty() {
return Err(Error::InvalidInput {
source: "empty page table".into(),
location: location!(),
});
}

Expand Down
2 changes: 2 additions & 0 deletions rust/lance-core/src/io/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ pub fn parse_version_from_path(path: &Path) -> Result<u64> {
.and_then(|(version, _)| version.parse::<u64>().ok())
.ok_or(crate::Error::Internal {
message: format!("Expected manifest file, but found {}", path),
location: location!(),
})
}

Expand Down Expand Up @@ -248,6 +249,7 @@ impl From<CommitError> for Error {
match e {
CommitError::CommitConflict => Self::Internal {
message: "Commit conflict".to_string(),
location: location!(),
},
CommitError::OtherError(e) => e,
}
Expand Down
5 changes: 4 additions & 1 deletion rust/lance-core/src/io/deletion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use bytes::Buf;
use object_store::path::Path;
use rand::Rng;
use roaring::bitmap::RoaringBitmap;
use snafu::ResultExt;
use snafu::{location, Location, ResultExt};

use super::object_store::ObjectStore;
use crate::error::{box_error, CorruptFileSnafu};
Expand Down Expand Up @@ -315,6 +315,7 @@ pub async fn read_deletion_file(
"Expected exactly one batch in deletion file, got {}",
batches.len()
),
location!(),
));
}

Expand All @@ -327,6 +328,7 @@ pub async fn read_deletion_file(
deletion_arrow_schema(),
batch.schema()
),
location!(),
));
}

Expand All @@ -343,6 +345,7 @@ pub async fn read_deletion_file(
return Err(Error::corrupt_file(
path,
"Null values are not allowed in deletion files",
location!(),
));
}
}
Expand Down
8 changes: 7 additions & 1 deletion rust/lance-core/src/io/object_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ impl ObjectStore {
source:
"`s3+ddb://` scheme and custom commit handler are mutually exclusive"
.into(),
location: location!(),
});
}

Expand All @@ -473,6 +474,7 @@ impl ObjectStore {
source:
"`s3+ddb://` scheme and expects exactly one query `ddbTableName`"
.into(),
location: location!(),
});
}
match url.query_pairs().next() {
Expand All @@ -484,13 +486,15 @@ impl ObjectStore {
source:
"`s3+ddb://` scheme requires non empty dynamodb table name"
.into(),
location: location!(),
});
}
Some(table_name)
}
_ => {
return Err(Error::InvalidInput {
source: "`s3+ddb://` scheme and expects exactly one query `ddbTableName`".into()
source: "`s3+ddb://` scheme and expects exactly one query `ddbTableName`".into(),
location: location!(),
});
}
}
Expand Down Expand Up @@ -519,6 +523,7 @@ impl ObjectStore {
return Err(Error::InvalidInput {
source: "`s3+ddb://` scheme requires `dynamodb` feature to be enabled"
.into(),
location: location!(),
});
}
None => params
Expand All @@ -530,6 +535,7 @@ impl ObjectStore {
// before creating the OSObjectStore we need to rewrite the url to drop ddb related parts
url.set_scheme("s3").map_err(|()| Error::Internal {
message: "could not set scheme".into(),
location: location!(),
})?;
url.set_query(None);

Expand Down
1 change: 1 addition & 0 deletions rust/lance-core/src/io/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl FileReader {
"File {} not found in fragment {:?}",
path, fragment
),
location: location!(),
})?
.fields,
)
Expand Down
2 changes: 2 additions & 0 deletions rust/lance-index/src/vector/ivf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,15 @@ impl Ivf {
) -> Result<RecordBatch> {
let vector_arr = batch.column_by_name(column).ok_or(Error::Index {
message: format!("Column {} does not exist.", column),
location: location!(),
})?;
let data = vector_arr.as_fixed_size_list_opt().ok_or(Error::Index {
message: format!(
"Column {} is not a vector type: {}",
column,
vector_arr.data_type()
),
location: location!(),
})?;
let matrix = MatrixView::<Float32Type>::try_from(data)?;
let part_ids = self.compute_partitions(&matrix).await;
Expand Down
6 changes: 3 additions & 3 deletions rust/lance-index/src/vector/kmeans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::sync::Arc;

use arrow_array::{builder::Float32Builder, FixedSizeListArray, Float32Array};
use lance_arrow::FixedSizeListArrayExt;
use log::info;
use rand::{seq::IteratorRandom, Rng};
use snafu::{location, Location};
use std::sync::Arc;

use lance_core::{Error, Result};
use lance_linalg::{
Expand All @@ -42,7 +42,7 @@ pub async fn train_kmeans(
if num_rows < k {
return Err(Error::Index{message: format!(
"KMeans: can not train {k} centroids with {num_rows} vectors, choose a smaller K (< {num_rows}) instead"
)});
),location: location!()});
}
// Ony sample sample_rate * num_clusters. See Faiss
let data = if num_rows > sample_rate * k {
Expand Down
2 changes: 2 additions & 0 deletions rust/lance-index/src/vector/pq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use lance_linalg::distance::{cosine_distance_batch, dot_distance_batch, l2_dista
use lance_linalg::kernels::argmin;
use lance_linalg::{distance::MetricType, MatrixView};
use rand::SeedableRng;
use snafu::{location, Location};
pub mod transform;

use super::kmeans::train_kmeans;
Expand Down Expand Up @@ -357,6 +358,7 @@ impl ProductQuantizer {
data.num_columns(),
params.num_sub_vectors
),
location: location!(),
});
}
assert_eq!(data.data().null_count(), 0);
Expand Down
Loading

0 comments on commit 6224356

Please sign in to comment.