feat(node): enable logging and show full errors (#1775)

This exposes the `LANCEDB_LOG` environment variable in node, so that
users can now turn on logging.

In addition, fixes a bug where only the top-level error from Rust was
being shown. This PR makes sure the full error chain is included in the
error message. In the future, will improve this so the error chain is
set on the [cause](https://nodejs.org/api/errors.html#errorcause)
property of JS errors https://github.com/lancedb/lancedb/issues/1779

Fixes #1774
This commit is contained in:
Will Jones
2024-10-29 15:13:34 -07:00
committed by GitHub
parent 55104c5bae
commit a324f4ad7a
13 changed files with 155 additions and 82 deletions

View File

@@ -18,6 +18,7 @@ use std::str::FromStr;
use napi::bindgen_prelude::*;
use napi_derive::*;
use crate::error::{convert_error, NapiErrorExt};
use crate::table::Table;
use crate::ConnectionOptions;
use lancedb::connection::{
@@ -86,12 +87,7 @@ impl Connection {
builder = builder.host_override(&host_override);
}
Ok(Self::inner_new(
builder
.execute()
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?,
))
Ok(Self::inner_new(builder.execute().await.default_error()?))
}
#[napi]
@@ -123,9 +119,7 @@ impl Connection {
if let Some(limit) = limit {
op = op.limit(limit);
}
op.execute()
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))
op.execute().await.default_error()
}
/// Create table from a Apache Arrow IPC (file) buffer.
@@ -156,17 +150,13 @@ impl Connection {
}
if let Some(data_storage_option) = data_storage_options.as_ref() {
builder = builder.data_storage_version(
LanceFileVersion::from_str(data_storage_option)
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?,
LanceFileVersion::from_str(data_storage_option).map_err(|e| convert_error(&e))?,
);
}
if let Some(enable_v2_manifest_paths) = enable_v2_manifest_paths {
builder = builder.enable_v2_manifest_paths(enable_v2_manifest_paths);
}
let tbl = builder
.execute()
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?;
let tbl = builder.execute().await.default_error()?;
Ok(Table::new(tbl))
}
@@ -195,17 +185,13 @@ impl Connection {
}
if let Some(data_storage_option) = data_storage_options.as_ref() {
builder = builder.data_storage_version(
LanceFileVersion::from_str(data_storage_option)
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?,
LanceFileVersion::from_str(data_storage_option).map_err(|e| convert_error(&e))?,
);
}
if let Some(enable_v2_manifest_paths) = enable_v2_manifest_paths {
builder = builder.enable_v2_manifest_paths(enable_v2_manifest_paths);
}
let tbl = builder
.execute()
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?;
let tbl = builder.execute().await.default_error()?;
Ok(Table::new(tbl))
}
@@ -225,19 +211,13 @@ impl Connection {
if let Some(index_cache_size) = index_cache_size {
builder = builder.index_cache_size(index_cache_size);
}
let tbl = builder
.execute()
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))?;
let tbl = builder.execute().await.default_error()?;
Ok(Table::new(tbl))
}
/// Drop table with the name. Or raise an error if the table does not exist.
#[napi(catch_unwind)]
pub async fn drop_table(&self, name: String) -> napi::Result<()> {
self.get_inner()?
.drop_table(&name)
.await
.map_err(|e| napi::Error::from_reason(format!("{}", e)))
self.get_inner()?.drop_table(&name).await.default_error()
}
}

View File

@@ -7,6 +7,31 @@ pub trait NapiErrorExt<T> {
impl<T> NapiErrorExt<T> for std::result::Result<T, lancedb::Error> {
fn default_error(self) -> Result<T> {
self.map_err(|err| napi::Error::from_reason(err.to_string()))
self.map_err(|err| convert_error(&err))
}
}
pub fn convert_error(err: &dyn std::error::Error) -> napi::Error {
let mut message = err.to_string();
// Append causes
let mut cause = err.source();
let mut indent = 2;
while let Some(err) = cause {
let cause_message = format!("Caused by: {}", err);
message.push_str(&indent_string(&cause_message, indent));
cause = err.source();
indent += 2;
}
napi::Error::from_reason(message)
}
fn indent_string(s: &str, amount: usize) -> String {
let indent = " ".repeat(amount);
s.lines()
.map(|line| format!("{}{}", indent, line))
.collect::<Vec<_>>()
.join("\n")
}

View File

@@ -14,6 +14,7 @@
use std::collections::HashMap;
use env_logger::Env;
use napi_derive::*;
mod connection;
@@ -77,3 +78,11 @@ pub struct WriteOptions {
pub struct OpenTableOptions {
pub storage_options: Option<HashMap<String, String>>,
}
#[napi::module_init]
fn init() {
let env = Env::new()
.filter_or("LANCEDB_LOG", "trace")
.write_style("LANCEDB_LOG_STYLE");
env_logger::init_from_env(env);
}

View File

@@ -2,6 +2,8 @@ use lancedb::{arrow::IntoArrow, ipc::ipc_file_to_batches, table::merge::MergeIns
use napi::bindgen_prelude::*;
use napi_derive::napi;
use crate::error::convert_error;
#[napi]
#[derive(Clone)]
/// A builder used to create and run a merge insert operation
@@ -35,14 +37,18 @@ impl NativeMergeInsertBuilder {
pub async fn execute(&self, buf: Buffer) -> napi::Result<()> {
let data = ipc_file_to_batches(buf.to_vec())
.and_then(IntoArrow::into_arrow)
.map_err(|e| napi::Error::from_reason(format!("Failed to read IPC file: {}", e)))?;
.map_err(|e| {
napi::Error::from_reason(format!("Failed to read IPC file: {}", convert_error(&e)))
})?;
let this = self.clone();
this.inner
.execute(data)
.await
.map_err(|e| napi::Error::from_reason(format!("Failed to execute merge insert: {}", e)))
this.inner.execute(data).await.map_err(|e| {
napi::Error::from_reason(format!(
"Failed to execute merge insert: {}",
convert_error(&e)
))
})
}
}

View File

@@ -22,6 +22,7 @@ use lancedb::query::VectorQuery as LanceDbVectorQuery;
use napi::bindgen_prelude::*;
use napi_derive::napi;
use crate::error::convert_error;
use crate::error::NapiErrorExt;
use crate::iterator::RecordBatchIterator;
use crate::util::parse_distance_type;
@@ -93,7 +94,10 @@ impl Query {
.execute_with_options(execution_opts)
.await
.map_err(|e| {
napi::Error::from_reason(format!("Failed to execute query stream: {}", e))
napi::Error::from_reason(format!(
"Failed to execute query stream: {}",
convert_error(&e)
))
})?;
Ok(RecordBatchIterator::new(inner_stream))
}
@@ -101,7 +105,10 @@ impl Query {
#[napi]
pub async fn explain_plan(&self, verbose: bool) -> napi::Result<String> {
self.inner.explain_plan(verbose).await.map_err(|e| {
napi::Error::from_reason(format!("Failed to retrieve the query plan: {}", e))
napi::Error::from_reason(format!(
"Failed to retrieve the query plan: {}",
convert_error(&e)
))
})
}
}
@@ -190,7 +197,10 @@ impl VectorQuery {
.execute_with_options(execution_opts)
.await
.map_err(|e| {
napi::Error::from_reason(format!("Failed to execute query stream: {}", e))
napi::Error::from_reason(format!(
"Failed to execute query stream: {}",
convert_error(&e)
))
})?;
Ok(RecordBatchIterator::new(inner_stream))
}
@@ -198,7 +208,10 @@ impl VectorQuery {
#[napi]
pub async fn explain_plan(&self, verbose: bool) -> napi::Result<String> {
self.inner.explain_plan(verbose).await.map_err(|e| {
napi::Error::from_reason(format!("Failed to retrieve the query plan: {}", e))
napi::Error::from_reason(format!(
"Failed to retrieve the query plan: {}",
convert_error(&e)
))
})
}
}

View File

@@ -72,10 +72,7 @@ impl Table {
/// Return Schema as empty Arrow IPC file.
#[napi(catch_unwind)]
pub async fn schema(&self) -> napi::Result<Buffer> {
let schema =
self.inner_ref()?.schema().await.map_err(|e| {
napi::Error::from_reason(format!("Failed to create IPC file: {}", e))
})?;
let schema = self.inner_ref()?.schema().await.default_error()?;
let mut writer = FileWriter::try_new(vec![], &schema)
.map_err(|e| napi::Error::from_reason(format!("Failed to create IPC file: {}", e)))?;
writer
@@ -100,12 +97,7 @@ impl Table {
return Err(napi::Error::from_reason(format!("Invalid mode: {}", mode)));
};
op.execute().await.map_err(|e| {
napi::Error::from_reason(format!(
"Failed to add batches to table {}: {}",
self.name, e
))
})
op.execute().await.default_error()
}
#[napi(catch_unwind)]
@@ -114,22 +106,12 @@ impl Table {
.count_rows(filter)
.await
.map(|val| val as i64)
.map_err(|e| {
napi::Error::from_reason(format!(
"Failed to count rows in table {}: {}",
self.name, e
))
})
.default_error()
}
#[napi(catch_unwind)]
pub async fn delete(&self, predicate: String) -> napi::Result<()> {
self.inner_ref()?.delete(&predicate).await.map_err(|e| {
napi::Error::from_reason(format!(
"Failed to delete rows in table {}: predicate={}",
self.name, e
))
})
self.inner_ref()?.delete(&predicate).await.default_error()
}
#[napi(catch_unwind)]
@@ -187,12 +169,7 @@ impl Table {
self.inner_ref()?
.add_columns(transforms, None)
.await
.map_err(|err| {
napi::Error::from_reason(format!(
"Failed to add columns to table {}: {}",
self.name, err
))
})?;
.default_error()?;
Ok(())
}
@@ -213,12 +190,7 @@ impl Table {
self.inner_ref()?
.alter_columns(&alterations)
.await
.map_err(|err| {
napi::Error::from_reason(format!(
"Failed to alter columns in table {}: {}",
self.name, err
))
})?;
.default_error()?;
Ok(())
}
@@ -228,12 +200,7 @@ impl Table {
self.inner_ref()?
.drop_columns(&col_refs)
.await
.map_err(|err| {
napi::Error::from_reason(format!(
"Failed to drop columns from table {}: {}",
self.name, err
))
})?;
.default_error()?;
Ok(())
}