From b8a17191743fb44870c35fdd81c5646aef6f9e96 Mon Sep 17 00:00:00 2001 From: Cory Grinstead Date: Mon, 1 Jul 2024 09:28:10 -0500 Subject: [PATCH] feat(nodejs): catch unwinds in node bindings (#1414) this bumps napi version to 2.16 which contains a few bug fixes. Additionally, it adds `catch_unwind` to any method that may unintentionally panic. `catch_unwind` will unwind the panics and return a regular JS error instead of panicking. --- nodejs/Cargo.toml | 6 +++--- nodejs/package-lock.json | 12 ++++++------ nodejs/package.json | 2 +- nodejs/src/connection.rs | 10 +++++----- nodejs/src/iterator.rs | 2 +- nodejs/src/merge.rs | 2 +- nodejs/src/query.rs | 4 ++-- nodejs/src/table.rs | 38 +++++++++++++++++++------------------- 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/nodejs/Cargo.toml b/nodejs/Cargo.toml index dd430853..da304dbe 100644 --- a/nodejs/Cargo.toml +++ b/nodejs/Cargo.toml @@ -15,11 +15,11 @@ crate-type = ["cdylib"] arrow-ipc.workspace = true futures.workspace = true lancedb = { path = "../rust/lancedb" } -napi = { version = "2.15", default-features = false, features = [ - "napi7", +napi = { version = "2.16.8", default-features = false, features = [ + "napi9", "async", ] } -napi-derive = "2" +napi-derive = "2.16.4" # Prevent dynamic linking of lzma, which comes from datafusion lzma-sys = { version = "*", features = ["static"] } diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index ac8b4d32..6c8015f1 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -1,12 +1,12 @@ { "name": "@lancedb/lancedb", - "version": "0.5.2", + "version": "0.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@lancedb/lancedb", - "version": "0.5.2", + "version": "0.6.0", "cpu": [ "x64", "arm64" @@ -29,7 +29,7 @@ "@aws-sdk/client-s3": "^3.33.0", "@biomejs/biome": "^1.7.3", "@jest/globals": "^29.7.0", - "@napi-rs/cli": "^2.18.0", + "@napi-rs/cli": "^2.18.3", "@types/axios": "^0.14.0", "@types/jest": "^29.1.2", "@types/tmp": "^0.2.6", @@ -3662,9 +3662,9 @@ } }, "node_modules/@napi-rs/cli": { - "version": "2.18.0", - "resolved": "https://registry.npmjs.org/@napi-rs/cli/-/cli-2.18.0.tgz", - "integrity": "sha512-lfSRT7cs3iC4L+kv9suGYQEezn5Nii7Kpu+THsYVI0tA1Vh59LH45p4QADaD7hvIkmOz79eEGtoKQ9nAkAPkzA==", + "version": "2.18.3", + "resolved": "https://registry.npmjs.org/@napi-rs/cli/-/cli-2.18.3.tgz", + "integrity": "sha512-L0f4kP0dyG8W5Qtc7MtP73VvLLrOLyRcUEBzknIfu8Jk4Jfhrsx1ItMHgyalYqMSslWdY3ojEfAaU5sx1VyeQQ==", "dev": true, "bin": { "napi": "scripts/index.js" diff --git a/nodejs/package.json b/nodejs/package.json index 75ed1537..65611ef4 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -37,7 +37,7 @@ "@aws-sdk/client-dynamodb": "^3.33.0", "@biomejs/biome": "^1.7.3", "@jest/globals": "^29.7.0", - "@napi-rs/cli": "^2.18.0", + "@napi-rs/cli": "^2.18.3", "@types/jest": "^29.1.2", "@types/tmp": "^0.2.6", "apache-arrow-old": "npm:apache-arrow@13.0.0", diff --git a/nodejs/src/connection.rs b/nodejs/src/connection.rs index 56de5783..c19453db 100644 --- a/nodejs/src/connection.rs +++ b/nodejs/src/connection.rs @@ -89,7 +89,7 @@ impl Connection { } /// List all tables in the dataset. - #[napi] + #[napi(catch_unwind)] pub async fn table_names( &self, start_after: Option, @@ -113,7 +113,7 @@ impl Connection { /// - name: The name of the table. /// - buf: The buffer containing the IPC file. /// - #[napi] + #[napi(catch_unwind)] pub async fn create_table( &self, name: String, @@ -141,7 +141,7 @@ impl Connection { Ok(Table::new(tbl)) } - #[napi] + #[napi(catch_unwind)] pub async fn create_empty_table( &self, name: String, @@ -173,7 +173,7 @@ impl Connection { Ok(Table::new(tbl)) } - #[napi] + #[napi(catch_unwind)] pub async fn open_table( &self, name: String, @@ -197,7 +197,7 @@ impl Connection { } /// Drop table with the name. Or raise an error if the table does not exist. - #[napi] + #[napi(catch_unwind)] pub async fn drop_table(&self, name: String) -> napi::Result<()> { self.get_inner()? .drop_table(&name) diff --git a/nodejs/src/iterator.rs b/nodejs/src/iterator.rs index 0fbe6b2e..21eb3176 100644 --- a/nodejs/src/iterator.rs +++ b/nodejs/src/iterator.rs @@ -30,7 +30,7 @@ impl RecordBatchIterator { Self { inner } } - #[napi] + #[napi(catch_unwind)] pub async unsafe fn next(&mut self) -> napi::Result> { if let Some(rst) = self.inner.next().await { let batch = rst.map_err(|e| { diff --git a/nodejs/src/merge.rs b/nodejs/src/merge.rs index 18c6a2ec..f13091fc 100644 --- a/nodejs/src/merge.rs +++ b/nodejs/src/merge.rs @@ -31,7 +31,7 @@ impl NativeMergeInsertBuilder { this } - #[napi] + #[napi(catch_unwind)] pub async fn execute(&self, buf: Buffer) -> napi::Result<()> { let data = ipc_file_to_batches(buf.to_vec()) .and_then(IntoArrow::into_arrow) diff --git a/nodejs/src/query.rs b/nodejs/src/query.rs index a2046ce6..012cd125 100644 --- a/nodejs/src/query.rs +++ b/nodejs/src/query.rs @@ -62,7 +62,7 @@ impl Query { Ok(VectorQuery { inner }) } - #[napi] + #[napi(catch_unwind)] pub async fn execute( &self, max_batch_length: Option, @@ -136,7 +136,7 @@ impl VectorQuery { self.inner = self.inner.clone().limit(limit as usize); } - #[napi] + #[napi(catch_unwind)] pub async fn execute( &self, max_batch_length: Option, diff --git a/nodejs/src/table.rs b/nodejs/src/table.rs index 664a46dc..1817abf6 100644 --- a/nodejs/src/table.rs +++ b/nodejs/src/table.rs @@ -70,7 +70,7 @@ impl Table { } /// Return Schema as empty Arrow IPC file. - #[napi] + #[napi(catch_unwind)] pub async fn schema(&self) -> napi::Result { let schema = self.inner_ref()?.schema().await.map_err(|e| { @@ -86,7 +86,7 @@ impl Table { })?)) } - #[napi] + #[napi(catch_unwind)] pub async fn add(&self, buf: Buffer, mode: String) -> napi::Result<()> { let batches = ipc_file_to_batches(buf.to_vec()) .map_err(|e| napi::Error::from_reason(format!("Failed to read IPC file: {}", e)))?; @@ -108,7 +108,7 @@ impl Table { }) } - #[napi] + #[napi(catch_unwind)] pub async fn count_rows(&self, filter: Option) -> napi::Result { self.inner_ref()? .count_rows(filter) @@ -122,7 +122,7 @@ impl Table { }) } - #[napi] + #[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!( @@ -132,7 +132,7 @@ impl Table { }) } - #[napi] + #[napi(catch_unwind)] pub async fn create_index( &self, index: Option<&Index>, @@ -151,7 +151,7 @@ impl Table { builder.execute().await.default_error() } - #[napi] + #[napi(catch_unwind)] pub async fn update( &self, only_if: Option, @@ -167,17 +167,17 @@ impl Table { op.execute().await.default_error() } - #[napi] + #[napi(catch_unwind)] pub fn query(&self) -> napi::Result { Ok(Query::new(self.inner_ref()?.query())) } - #[napi] + #[napi(catch_unwind)] pub fn vector_search(&self, vector: Float32Array) -> napi::Result { self.query()?.nearest_to(vector) } - #[napi] + #[napi(catch_unwind)] pub async fn add_columns(&self, transforms: Vec) -> napi::Result<()> { let transforms = transforms .into_iter() @@ -196,7 +196,7 @@ impl Table { Ok(()) } - #[napi] + #[napi(catch_unwind)] pub async fn alter_columns(&self, alterations: Vec) -> napi::Result<()> { for alteration in &alterations { if alteration.rename.is_none() && alteration.nullable.is_none() { @@ -222,7 +222,7 @@ impl Table { Ok(()) } - #[napi] + #[napi(catch_unwind)] pub async fn drop_columns(&self, columns: Vec) -> napi::Result<()> { let col_refs = columns.iter().map(String::as_str).collect::>(); self.inner_ref()? @@ -237,7 +237,7 @@ impl Table { Ok(()) } - #[napi] + #[napi(catch_unwind)] pub async fn version(&self) -> napi::Result { self.inner_ref()? .version() @@ -246,7 +246,7 @@ impl Table { .default_error() } - #[napi] + #[napi(catch_unwind)] pub async fn checkout(&self, version: i64) -> napi::Result<()> { self.inner_ref()? .checkout(version as u64) @@ -254,17 +254,17 @@ impl Table { .default_error() } - #[napi] + #[napi(catch_unwind)] pub async fn checkout_latest(&self) -> napi::Result<()> { self.inner_ref()?.checkout_latest().await.default_error() } - #[napi] + #[napi(catch_unwind)] pub async fn restore(&self) -> napi::Result<()> { self.inner_ref()?.restore().await.default_error() } - #[napi] + #[napi(catch_unwind)] pub async fn optimize(&self, older_than_ms: Option) -> napi::Result { let inner = self.inner_ref()?; @@ -318,7 +318,7 @@ impl Table { }) } - #[napi] + #[napi(catch_unwind)] pub async fn list_indices(&self) -> napi::Result> { Ok(self .inner_ref()? @@ -330,14 +330,14 @@ impl Table { .collect::>()) } - #[napi] + #[napi(catch_unwind)] pub async fn index_stats(&self, index_name: String) -> napi::Result> { let tbl = self.inner_ref()?.as_native().unwrap(); let stats = tbl.index_stats(&index_name).await.default_error()?; Ok(stats.map(IndexStatistics::from)) } - #[napi] + #[napi(catch_unwind)] pub fn merge_insert(&self, on: Vec) -> napi::Result { let on: Vec<_> = on.iter().map(String::as_str).collect(); Ok(self.inner_ref()?.merge_insert(on.as_slice()).into())