feat: more accessible errors (#1025)

The fact that we convert errors to strings makes them really hard to
work with. For example, in SaaS we want to know whether the underlying
`lance::Error` was the `InvalidInput` variant, so we can return a 400
instead of a 500.
This commit is contained in:
Will Jones
2024-03-05 07:57:11 -08:00
committed by Weston Pace
parent 10089481c0
commit 05f9a77baf
6 changed files with 47 additions and 43 deletions

View File

@@ -13,7 +13,7 @@
// limitations under the License.
use pyo3::{
exceptions::{PyOSError, PyRuntimeError, PyValueError},
exceptions::{PyIOError, PyNotImplementedError, PyOSError, PyRuntimeError, PyValueError},
PyResult,
};
@@ -41,11 +41,15 @@ impl<T> PythonErrorExt<T> for std::result::Result<T, LanceError> {
LanceError::Schema { .. } => self.value_error(),
LanceError::CreateDir { .. } => self.os_error(),
LanceError::TableAlreadyExists { .. } => self.runtime_error(),
LanceError::Store { .. } => self.runtime_error(),
LanceError::ObjectStore { .. } => Err(PyIOError::new_err(err.to_string())),
LanceError::Lance { .. } => self.runtime_error(),
LanceError::Runtime { .. } => self.runtime_error(),
LanceError::Http { .. } => self.runtime_error(),
LanceError::Arrow { .. } => self.runtime_error(),
LanceError::NotSupported { .. } => {
Err(PyNotImplementedError::new_err(err.to_string()))
}
LanceError::Other { .. } => self.runtime_error(),
},
}
}

View File

@@ -486,7 +486,7 @@ impl Database {
engine = Some(value.to_string());
} else if key == MIRRORED_STORE {
if cfg!(windows) {
return Err(Error::Lance {
return Err(Error::NotSupported {
message: "mirrored store is not supported on windows".into(),
});
}

View File

@@ -39,46 +39,50 @@ pub enum Error {
Runtime { message: String },
// 3rd party / external errors
#[snafu(display("object_store error: {message}"))]
Store { message: String },
#[snafu(display("lance error: {message}"))]
Lance { message: String },
#[snafu(display("object_store error: {source}"))]
ObjectStore { source: object_store::Error },
#[snafu(display("lance error: {source}"))]
Lance { source: lance::Error },
#[snafu(display("Http error: {message}"))]
Http { message: String },
#[snafu(display("Arrow error: {message}"))]
Arrow { message: String },
#[snafu(display("Arrow error: {source}"))]
Arrow { source: ArrowError },
#[snafu(display("LanceDBError: not supported: {message}"))]
NotSupported { message: String },
#[snafu(whatever, display("{message}"))]
Other {
message: String,
#[snafu(source(from(Box<dyn std::error::Error + Send + Sync>, Some)))]
source: Option<Box<dyn std::error::Error + Send + Sync>>,
},
}
pub type Result<T> = std::result::Result<T, Error>;
impl From<ArrowError> for Error {
fn from(e: ArrowError) -> Self {
Self::Arrow {
message: e.to_string(),
}
fn from(source: ArrowError) -> Self {
Self::Arrow { source }
}
}
impl From<lance::Error> for Error {
fn from(e: lance::Error) -> Self {
Self::Lance {
message: e.to_string(),
}
fn from(source: lance::Error) -> Self {
// TODO: Once Lance is changed to preserve ObjectStore, DataFusion, and Arrow errors, we can
// pass those variants through here as well.
Self::Lance { source }
}
}
impl From<object_store::Error> for Error {
fn from(e: object_store::Error) -> Self {
Self::Store {
message: e.to_string(),
}
fn from(source: object_store::Error) -> Self {
Self::ObjectStore { source }
}
}
impl From<object_store::path::Error> for Error {
fn from(e: object_store::path::Error) -> Self {
Self::Store {
message: e.to_string(),
fn from(source: object_store::path::Error) -> Self {
Self::ObjectStore {
source: object_store::Error::InvalidPath { source },
}
}
}

View File

@@ -32,8 +32,9 @@ pub fn ipc_file_to_batches(buf: Vec<u8>) -> Result<impl RecordBatchReader> {
/// Convert record batches to Arrow IPC file
pub fn batches_to_ipc_file(batches: &[RecordBatch]) -> Result<Vec<u8>> {
if batches.is_empty() {
return Err(Error::Store {
return Err(Error::Other {
message: "No batches to write".to_string(),
source: None,
});
}
let schema = batches[0].schema();

View File

@@ -38,6 +38,7 @@ use lance::io::WrappingObjectStore;
use lance_index::IndexType;
use lance_index::{optimize::OptimizeOptions, DatasetIndexExt};
use log::info;
use snafu::whatever;
use crate::error::{Error, Result};
use crate::index::vector::{VectorIndex, VectorIndexStatistics};
@@ -586,9 +587,7 @@ impl NativeTable {
lance::Error::DatasetNotFound { .. } => Error::TableNotFound {
name: name.to_string(),
},
e => Error::Lance {
message: e.to_string(),
},
source => Error::Lance { source },
})?;
let dataset = DatasetConsistencyWrapper::new_latest(dataset, read_consistency_interval);
@@ -693,9 +692,7 @@ impl NativeTable {
lance::Error::DatasetAlreadyExists { .. } => Error::TableAlreadyExists {
name: name.to_string(),
},
e => Error::Lance {
message: e.to_string(),
},
source => Error::Lance { source },
})?;
Ok(Self {
name: name.to_string(),
@@ -865,13 +862,10 @@ impl NativeTable {
}
let dataset = self.dataset.get().await?;
let index_stats = dataset.index_statistics(&index.unwrap().index_name).await?;
let index_stats: VectorIndexStatistics =
serde_json::from_str(&index_stats).map_err(|e| Error::Lance {
message: format!(
"error deserializing index statistics {}: {}",
e, index_stats
),
})?;
let index_stats: VectorIndexStatistics = whatever!(
serde_json::from_str(&index_stats),
"error deserializing index statistics {index_stats}",
);
Ok(Some(index_stats))
}
@@ -940,12 +934,12 @@ impl TableInternal for NativeTable {
let arrow_schema = Schema::from(ds_ref.schema());
default_vector_column(&arrow_schema, Some(query_vector.len() as i32))?
};
let field = ds_ref.schema().field(&column).ok_or(Error::Store {
let field = ds_ref.schema().field(&column).ok_or(Error::Schema {
message: format!("Column {} not found in dataset schema", column),
})?;
if !matches!(field.data_type(), arrow_schema::DataType::FixedSizeList(f, dim) if f.data_type().is_floating() && dim == query_vector.len() as i32)
{
return Err(Error::Store {
return Err(Error::Schema {
message: format!(
"Vector column '{}' does not match the dimension of the query vector: dim={}",
column,

View File

@@ -21,8 +21,9 @@ impl PatchStoreParam for Option<ObjectStoreParams> {
) -> Result<Option<ObjectStoreParams>> {
let mut params = self.unwrap_or_default();
if params.object_store_wrapper.is_some() {
return Err(Error::Lance {
return Err(Error::Other {
message: "can not patch param because object store is already set".into(),
source: None,
});
}
params.object_store_wrapper = Some(wrapper);
@@ -80,11 +81,11 @@ pub(crate) fn default_vector_column(schema: &Schema, dim: Option<i32>) -> Result
})
.collect::<Vec<_>>();
if candidates.is_empty() {
Err(Error::Store {
Err(Error::Schema {
message: "No vector column found to create index".to_string(),
})
} else if candidates.len() != 1 {
Err(Error::Store {
Err(Error::Schema {
message: format!(
"More than one vector columns found, \
please specify which column to create index: {:?}",