feat: add source to TableNotFound errors (#2765)

This will make it easier to see if there are underlying problems. We
should see the actual object store HTTP request error within the error
chain after this.
This commit is contained in:
Will Jones
2025-11-04 15:31:45 -08:00
committed by GitHub
parent aed4a7c98e
commit 7ef8bafd51
6 changed files with 44 additions and 17 deletions

View File

@@ -51,8 +51,11 @@ pub enum Error {
DatasetAlreadyExists { uri: String, location: Location }, DatasetAlreadyExists { uri: String, location: Location },
#[snafu(display("Table '{name}' already exists"))] #[snafu(display("Table '{name}' already exists"))]
TableAlreadyExists { name: String }, TableAlreadyExists { name: String },
#[snafu(display("Table '{name}' was not found"))] #[snafu(display("Table '{name}' was not found: {source}"))]
TableNotFound { name: String }, TableNotFound {
name: String,
source: Box<dyn std::error::Error + Send + Sync>,
},
#[snafu(display("Invalid table name '{name}': {reason}"))] #[snafu(display("Invalid table name '{name}': {reason}"))]
InvalidTableName { name: String, reason: String }, InvalidTableName { name: String, reason: String },
#[snafu(display("Embedding function '{name}' was not found: {reason}, {location}"))] #[snafu(display("Embedding function '{name}' was not found: {reason}, {location}"))]
@@ -191,7 +194,7 @@ impl From<lancedb::Error> for Error {
message, message,
location: std::panic::Location::caller().to_snafu_location(), location: std::panic::Location::caller().to_snafu_location(),
}, },
lancedb::Error::TableNotFound { name } => Self::TableNotFound { name }, lancedb::Error::TableNotFound { name, source } => Self::TableNotFound { name, source },
lancedb::Error::TableAlreadyExists { name } => Self::TableAlreadyExists { name }, lancedb::Error::TableAlreadyExists { name } => Self::TableAlreadyExists { name },
lancedb::Error::EmbeddingFunctionNotFound { name, reason } => { lancedb::Error::EmbeddingFunctionNotFound { name, reason } => {
Self::EmbeddingFunctionNotFound { Self::EmbeddingFunctionNotFound {

View File

@@ -455,6 +455,7 @@ impl ListingDatabase {
// `remove_dir_all` may be used to remove something not be a dataset // `remove_dir_all` may be used to remove something not be a dataset
lance::Error::NotFound { .. } => Error::TableNotFound { lance::Error::NotFound { .. } => Error::TableNotFound {
name: name.to_owned(), name: name.to_owned(),
source: Box::new(err),
}, },
_ => Error::from(err), _ => Error::from(err),
})?; })?;

View File

@@ -6,6 +6,8 @@ use std::sync::PoisonError;
use arrow_schema::ArrowError; use arrow_schema::ArrowError;
use snafu::Snafu; use snafu::Snafu;
type BoxError = Box<dyn std::error::Error + Send + Sync>;
#[derive(Debug, Snafu)] #[derive(Debug, Snafu)]
#[snafu(visibility(pub(crate)))] #[snafu(visibility(pub(crate)))]
pub enum Error { pub enum Error {
@@ -14,7 +16,7 @@ pub enum Error {
#[snafu(display("Invalid input, {message}"))] #[snafu(display("Invalid input, {message}"))]
InvalidInput { message: String }, InvalidInput { message: String },
#[snafu(display("Table '{name}' was not found"))] #[snafu(display("Table '{name}' was not found"))]
TableNotFound { name: String }, TableNotFound { name: String, source: BoxError },
#[snafu(display("Database '{name}' was not found"))] #[snafu(display("Database '{name}' was not found"))]
DatabaseNotFound { name: String }, DatabaseNotFound { name: String },
#[snafu(display("Database '{name}' already exists."))] #[snafu(display("Database '{name}' already exists."))]

View File

@@ -515,11 +515,8 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
.client .client
.post(&format!("/v1/table/{}/describe/", identifier)); .post(&format!("/v1/table/{}/describe/", identifier));
let (request_id, rsp) = self.client.send_with_retry(req, None, true).await?; let (request_id, rsp) = self.client.send_with_retry(req, None, true).await?;
if rsp.status() == StatusCode::NOT_FOUND { let rsp =
return Err(crate::Error::TableNotFound { RemoteTable::<S>::handle_table_not_found(&request.name, rsp, &request_id).await?;
name: identifier.clone(),
});
}
let rsp = self.client.check_response(&request_id, rsp).await?; let rsp = self.client.check_response(&request_id, rsp).await?;
let version = parse_server_version(&request_id, &rsp)?; let version = parse_server_version(&request_id, &rsp)?;
let table_identifier = build_table_identifier( let table_identifier = build_table_identifier(

View File

@@ -336,16 +336,33 @@ impl<S: HttpSend> RemoteTable<S> {
Ok(res) Ok(res)
} }
pub(super) async fn handle_table_not_found(
table_name: &str,
response: reqwest::Response,
request_id: &str,
) -> Result<reqwest::Response> {
let status = response.status();
if status == StatusCode::NOT_FOUND {
let body = response.text().await.ok().unwrap_or_default();
let request_error = Error::Http {
source: body.into(),
request_id: request_id.into(),
status_code: Some(status),
};
return Err(Error::TableNotFound {
name: table_name.to_string(),
source: Box::new(request_error),
});
}
Ok(response)
}
async fn check_table_response( async fn check_table_response(
&self, &self,
request_id: &str, request_id: &str,
response: reqwest::Response, response: reqwest::Response,
) -> Result<reqwest::Response> { ) -> Result<reqwest::Response> {
if response.status() == StatusCode::NOT_FOUND { let response = Self::handle_table_not_found(&self.name, response, request_id).await?;
return Err(Error::TableNotFound {
name: self.identifier.clone(),
});
}
self.client.check_response(request_id, response).await self.client.check_response(request_id, response).await
} }
@@ -681,8 +698,9 @@ impl<S: HttpSend> BaseTable for RemoteTable<S> {
.map_err(|e| match e { .map_err(|e| match e {
// try to map the error to a more user-friendly error telling them // try to map the error to a more user-friendly error telling them
// specifically that the version does not exist // specifically that the version does not exist
Error::TableNotFound { name } => Error::TableNotFound { Error::TableNotFound { name, source } => Error::TableNotFound {
name: format!("{} (version: {})", name, version), name: format!("{} (version: {})", name, version),
source,
}, },
e => e, e => e,
})?; })?;
@@ -1575,7 +1593,11 @@ mod tests {
for result in results { for result in results {
let result = result.await; let result = result.await;
assert!(result.is_err()); assert!(result.is_err());
assert!(matches!(result, Err(Error::TableNotFound { name }) if name == "my_table")); assert!(
matches!(&result, &Err(Error::TableNotFound { ref name, .. }) if name == "my_table")
);
let full_error_report = snafu::Report::from_error(result.unwrap_err()).to_string();
assert!(full_error_report.contains("table my_table not found"));
} }
} }
@@ -2884,7 +2906,7 @@ mod tests {
let res = table.checkout(43).await; let res = table.checkout(43).await;
println!("{:?}", res); println!("{:?}", res);
assert!( assert!(
matches!(res, Err(Error::TableNotFound { name }) if name == "my_table (version: 43)") matches!(res, Err(Error::TableNotFound { name, .. }) if name == "my_table (version: 43)")
); );
} }

View File

@@ -1534,6 +1534,7 @@ impl NativeTable {
.map_err(|e| match e { .map_err(|e| match e {
lance::Error::DatasetNotFound { .. } => Error::TableNotFound { lance::Error::DatasetNotFound { .. } => Error::TableNotFound {
name: name.to_string(), name: name.to_string(),
source: Box::new(e),
}, },
source => Error::Lance { source }, source => Error::Lance { source },
})?; })?;
@@ -1554,6 +1555,7 @@ impl NativeTable {
.file_stem() .file_stem()
.ok_or(Error::TableNotFound { .ok_or(Error::TableNotFound {
name: uri.to_string(), name: uri.to_string(),
source: format!("Could not extract table name from URI: '{}'", uri).into(),
})? })?
.to_str() .to_str()
.ok_or(Error::InvalidTableName { .ok_or(Error::InvalidTableName {