From 80f8f7e27a49b4b868f71881fb091d37dab9a47c Mon Sep 17 00:00:00 2001 From: MarkusJx <28785953+markusjx@users.noreply.github.com> Date: Sat, 20 Apr 2024 18:07:53 +0200 Subject: [PATCH 1/5] test(bridge): test import and method not found errors Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --- README.md | 1 + crates/java-bridge/src/node/java.rs | 10 +++++---- test/StringTest.test.ts | 32 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ac0ecea..ff25754 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ need to enable the `asyncJavaExceptionObjects` [config option](https://markusjx.github.io/node-java-bridge/variables/config.html) before or while importing the class. Enabling this will cause the stack trace of the JavaScript error to be lost. +The Java throwable will never be available from the `importClassAsync` function. ```ts import { importClass } from 'java-bridge'; diff --git a/crates/java-bridge/src/node/java.rs b/crates/java-bridge/src/node/java.rs index 16c70e0..41f9d3c 100644 --- a/crates/java-bridge/src/node/java.rs +++ b/crates/java-bridge/src/node/java.rs @@ -107,11 +107,13 @@ impl Java { class_name: String, config: Option, ) -> napi::Result { - let proxy = MutAppState::::get_or_insert_default() + let proxy_result = MutAppState::::get_or_insert_default() .get_mut() - .get_class_proxy(&self.root_vm, class_name, config) - .map_napi_err(Some(env))?; - JavaClassInstance::create_class_instance(&env, proxy) + .get_class_proxy(&self.root_vm, class_name, config); + + // Map the result here in order to release the lock + // on the class cache before mapping the error. + JavaClassInstance::create_class_instance(&env, proxy_result.map_napi_err(Some(env))?) } /// Import a java class (async) diff --git a/test/StringTest.test.ts b/test/StringTest.test.ts index 529dff7..fb5a642 100644 --- a/test/StringTest.test.ts +++ b/test/StringTest.test.ts @@ -183,4 +183,36 @@ describe('StringTest', () => { ); } }); + + it('invalid number of arguments', () => { + // @ts-expect-error + expect(() => new JavaString('a', 'b')) + .to.throw() + .and.to.have.property('message') + .which.satisfies((val: string) => + val.startsWith('No constructor found with matching signature') + ); + + // @ts-expect-error + expect(() => new JavaString('a', 'b')) + .to.throw() + .and.to.not.have.property('cause'); + }); + + it('import non-existing class', () => { + expect(() => importClass('java.lang.NonExistingClass')) + .to.throw() + .and.to.have.property('message') + .which.satisfies((val: string) => + val.startsWith( + 'java.lang.ClassNotFoundException: java.lang.NonExistingClass' + ) + ); + + expect(() => importClass('java.lang.NonExistingClass')) + .to.throw() + .and.to.have.property('cause') + .which.has.property('printStackTrace') + .which.is.a('function'); + }); }); From 380db06b2bd9b93f277f63dd3eb87cf79100850a Mon Sep 17 00:00:00 2001 From: MarkusJx <28785953+markusjx@users.noreply.github.com> Date: Sat, 20 Apr 2024 18:14:20 +0200 Subject: [PATCH 2/5] test(bridge): increase number of retries Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --- test/DeleteTest.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/DeleteTest.test.ts b/test/DeleteTest.test.ts index b6df91d..dd1b08d 100644 --- a/test/DeleteTest.test.ts +++ b/test/DeleteTest.test.ts @@ -32,7 +32,7 @@ describe('DeleteTest', () => { System.gcSync(); const end = getUsedMemory(Runtime); expect(end).to.be.lessThan(after - 10_000_000); - }).timeout(timeout); + }).timeout(timeout).retries(3); it('Delete deleted instance', () => { const JString = importClass('java.lang.String'); @@ -48,7 +48,7 @@ describe('DeleteTest', () => { expect(() => string.toString()).to.throw(); }).timeout(timeout); - it('Check proxy memory leak', async function () { + it('Check proxy memory leak', async function() { if (isCi && (process.arch === 'arm64' || process.arch === 'arm')) { this.skip(); } @@ -67,7 +67,7 @@ describe('DeleteTest', () => { apply: () => { return generateRandomString(1024 * 1024 * 10); }, - } + }, ); System.gcSync(); From 25abdf6e61044fe7bffc9ce15f8c855c14571525 Mon Sep 17 00:00:00 2001 From: MarkusJx <28785953+markusjx@users.noreply.github.com> Date: Sat, 20 Apr 2024 18:32:23 +0200 Subject: [PATCH 3/5] chore: run prettier Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --- test/DeleteTest.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/DeleteTest.test.ts b/test/DeleteTest.test.ts index dd1b08d..c417396 100644 --- a/test/DeleteTest.test.ts +++ b/test/DeleteTest.test.ts @@ -32,7 +32,9 @@ describe('DeleteTest', () => { System.gcSync(); const end = getUsedMemory(Runtime); expect(end).to.be.lessThan(after - 10_000_000); - }).timeout(timeout).retries(3); + }) + .timeout(timeout) + .retries(3); it('Delete deleted instance', () => { const JString = importClass('java.lang.String'); @@ -48,7 +50,7 @@ describe('DeleteTest', () => { expect(() => string.toString()).to.throw(); }).timeout(timeout); - it('Check proxy memory leak', async function() { + it('Check proxy memory leak', async function () { if (isCi && (process.arch === 'arm64' || process.arch === 'arm')) { this.skip(); } @@ -67,7 +69,7 @@ describe('DeleteTest', () => { apply: () => { return generateRandomString(1024 * 1024 * 10); }, - }, + } ); System.gcSync(); From 2121c3c166982ec062db9d454cd7d666d2b4431f Mon Sep 17 00:00:00 2001 From: MarkusJx <28785953+markusjx@users.noreply.github.com> Date: Sun, 21 Apr 2024 11:29:27 +0200 Subject: [PATCH 4/5] feat(bridge): expose Java throwable from importClassAsync Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --- README.md | 43 +++++++++-------- crates/java-bridge/src/node/java.rs | 33 +++++++++---- .../src/node/java_class_instance.rs | 46 ++----------------- crates/java-bridge/src/node/util/helpers.rs | 45 +++++++++++++++++- test/StringTest.test.ts | 23 ++++++++++ 5 files changed, 115 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index ff25754..06e6d75 100644 --- a/README.md +++ b/README.md @@ -46,12 +46,12 @@ using `npm i`, you can skip this._ In order to build this project, you should install -- Node.js -- npm -- rustc, the rust compiler -- cargo -- Java JDK 8+ -- clang +- Node.js +- npm +- rustc, the rust compiler +- cargo +- Java JDK 8+ +- clang Then, to build the project, run: @@ -66,15 +66,15 @@ npm run build > available_ | Operating System | i686 | x64 | arm | arm64 | -| ---------------- | :--: | :-: | :-: | :---: | -| Linux | - | ✅ | - | ✅ | -| Windows | ✅ | ✅ | - | - | -| macOS | - | ✅ | - | ✅ | +|------------------|:----:|:---:|:---:|:-----:| +| Linux | - | ✅ | - | ✅ | +| Windows | ✅ | ✅ | - | - | +| macOS | - | ✅ | - | ✅ | ### Known working linux distros | Distro | Version | -| :----: | :-----------: | +|:------:|:-------------:| | Ubuntu | `>= 20.04` | | Debian | `>= bullseye` | @@ -143,12 +143,12 @@ electron-builder, you can do this by adding the following to your `package.json` ```json { - "build": { - "asarUnpack": [ - "node_modules/java-bridge/**", - "node_modules/java-bridge-*/**" - ] - } + "build": { + "asarUnpack": [ + "node_modules/java-bridge/**", + "node_modules/java-bridge-*/**" + ] + } } ``` @@ -333,7 +333,6 @@ need to enable the `asyncJavaExceptionObjects` [config option](https://markusjx.github.io/node-java-bridge/variables/config.html) before or while importing the class. Enabling this will cause the stack trace of the JavaScript error to be lost. -The Java throwable will never be available from the `importClassAsync` function. ```ts import { importClass } from 'java-bridge'; @@ -359,10 +358,10 @@ all features enabled. Logged events include: -- Class loading -- Method calls -- Class instance creation -- Method and class lookup +- Class loading +- Method calls +- Class instance creation +- Method and class lookup **Note:** Logging affects the performance of the module. Thus, it is recommended to only enable logging when debugging. diff --git a/crates/java-bridge/src/node/java.rs b/crates/java-bridge/src/node/java.rs index 41f9d3c..7b68e22 100644 --- a/crates/java-bridge/src/node/java.rs +++ b/crates/java-bridge/src/node/java.rs @@ -8,9 +8,10 @@ use crate::node::java_class_instance::{JavaClassInstance, CLASS_PROXY_PROPERTY, use crate::node::java_class_proxy::JavaClassProxy; use crate::node::java_options::JavaOptions; use crate::node::stdout_redirect::StdoutRedirect; -use crate::node::util::helpers::{list_files, parse_array_or_string, parse_classpath_args}; +use crate::node::util::helpers::{ + call_async_method_with_resolver, list_files, parse_array_or_string, parse_classpath_args, +}; use app_state::{stateful, AppStateTrait, MutAppState, MutAppStateLock}; -use futures::future; use java_rs::java_env::JavaEnv; use java_rs::java_vm::JavaVM; use java_rs::objects::args::AsJavaArg; @@ -102,7 +103,7 @@ impl Java { /// The imported class will be cached for future use. #[napi(ts_return_type = "object")] pub fn import_class( - &mut self, + &self, env: Env, class_name: String, config: Option, @@ -118,22 +119,34 @@ impl Java { /// Import a java class (async) /// Will return a promise that resolves to the class instance. + /// + /// If the underlying Java throwable should be contained in the error object, + /// set `asyncJavaExceptionObjects` to `true`. This will cause the JavaScript + /// stack trace to be lost. Setting this option in the global config will + /// **not** affect this method, this option has to be set each time this + /// method is called. + /// /// @see importClass #[napi(ts_return_type = "Promise")] pub fn import_class_async( - &'static mut self, + &self, env: Env, class_name: String, config: Option, ) -> napi::Result { - env.execute_tokio_future( - future::lazy(|_| { + let root_vm = self.root_vm.clone(); + call_async_method_with_resolver( + env, + config + .as_ref() + .and_then(|c| c.async_java_exception_objects) + .unwrap_or_default(), + move || { MutAppState::::get_or_insert_default() .get_mut() - .get_class_proxy(&self.root_vm, class_name, config) - .map_napi_err(None) - }), - |&mut env, proxy| JavaClassInstance::create_class_instance(&env, proxy), + .get_class_proxy(&root_vm, class_name, config) + }, + |env, res| JavaClassInstance::create_class_instance(env, res), ) } diff --git a/crates/java-bridge/src/node/java_class_instance.rs b/crates/java-bridge/src/node/java_class_instance.rs index c2729e1..ec49724 100644 --- a/crates/java-bridge/src/node/java_class_instance.rs +++ b/crates/java-bridge/src/node/java_class_instance.rs @@ -6,7 +6,7 @@ use crate::node::helpers::napi_ext::{load_napi_library, uv_run, uv_run_mode}; use crate::node::interface_proxy::proxies::interface_proxy_exists; use crate::node::java::Java; use crate::node::java_class_proxy::JavaClassProxy; -use crate::node::util::helpers::ResultType; +use crate::node::util::helpers::{call_async_method, call_async_method_with_resolver}; use crate::node::util::traits::UnwrapOrEmpty; use java_rs::java_call_result::JavaCallResult; use java_rs::java_type::JavaType; @@ -328,7 +328,7 @@ impl JavaClassInstance { let env = proxy.vm.attach_thread().map_napi_err(Some(*ctx.env))?; let args = call_context_to_java_args(ctx, method.parameter_types(), &env)?; - Self::call_async_method(*ctx.env, proxy, move || { + call_async_method(*ctx.env, proxy, move || { let args_ref = call_results_to_args(&args); method.call_static(args_ref.as_slice()) }) @@ -402,7 +402,7 @@ impl JavaClassInstance { let env = proxy.vm.attach_thread().map_napi_err(Some(*ctx.env))?; let args = call_context_to_java_args(ctx, method.parameter_types(), &env)?; - Self::call_async_method(*ctx.env, proxy, move || { + call_async_method(*ctx.env, proxy, move || { let args_ref = call_results_to_args(&args); method.call(&obj, args_ref.as_slice()) }) @@ -440,44 +440,6 @@ impl JavaClassInstance { )?, ) } - - fn call_async_method(env: Env, proxy: Arc, func: F) -> napi::Result - where - F: (FnOnce() -> ResultType) + Send + Sync + 'static, - { - Self::call_async_method_with_resolver( - env, - proxy.async_java_exception_objects(), - func, - move |&mut env, res| { - let j_env = proxy.vm.attach_thread().map_napi_err(Some(env))?; - res.to_napi_value(&j_env, &env).map_napi_err(Some(env)) - }, - ) - } - - fn call_async_method_with_resolver( - env: Env, - async_java_exception_objects: bool, - func: F, - resolver: Res, - ) -> napi::Result - where - F: (FnOnce() -> ResultType) + Send + Sync + 'static, - Res: FnOnce(&mut Env, R) -> napi::Result + Send + Sync + 'static, - R: Send + Sync + 'static, - { - if async_java_exception_objects { - env.execute_tokio_future(futures::future::lazy(|_| Ok(func())), move |env, res| { - resolver(env, res.map_napi_err(Some(*env))?) - }) - } else { - env.execute_tokio_future( - futures::future::lazy(move |_| func().map_napi_err(None)), - resolver, - ) - } - } } #[js_function(255usize)] @@ -521,7 +483,7 @@ fn new_instance(ctx: CallContext) -> napi::Result { let env = proxy.vm.attach_thread().map_napi_err(Some(*ctx.env))?; let args = call_context_to_java_args(&ctx, constructor.parameter_types(), &env)?; - JavaClassInstance::call_async_method_with_resolver( + call_async_method_with_resolver( *ctx.env, proxy.async_java_exception_objects(), move || { diff --git a/crates/java-bridge/src/node/util/helpers.rs b/crates/java-bridge/src/node/util/helpers.rs index fb67f9d..bd25cea 100644 --- a/crates/java-bridge/src/node/util/helpers.rs +++ b/crates/java-bridge/src/node/util/helpers.rs @@ -1,7 +1,11 @@ +use crate::node::extensions::java_call_result_ext::ToNapiValue; use crate::node::helpers::napi_error::{MapToNapiError, StrIntoNapiError}; +use crate::node::java_class_proxy::JavaClassProxy; use glob::glob; -use napi::{JsString, JsUnknown}; +use java_rs::java_call_result::JavaCallResult; +use napi::{Env, JsObject, JsString, JsUnknown, NapiRaw}; use std::error::Error; +use std::sync::Arc; pub type ResultType = Result>; @@ -72,3 +76,42 @@ pub fn parse_classpath_args(cp: &[String], args: &mut Vec) -> String { cp.join(separator::CLASSPATH_SEPARATOR) ) } + +pub fn call_async_method(env: Env, proxy: Arc, func: F) -> napi::Result +where + F: (FnOnce() -> ResultType) + Send + Sync + 'static, +{ + call_async_method_with_resolver( + env, + proxy.async_java_exception_objects(), + func, + move |&mut env, res| { + let j_env = proxy.vm.attach_thread().map_napi_err(Some(env))?; + res.to_napi_value(&j_env, &env).map_napi_err(Some(env)) + }, + ) +} + +pub fn call_async_method_with_resolver( + env: Env, + async_java_exception_objects: bool, + func: F, + resolver: Res, +) -> napi::Result +where + F: (FnOnce() -> ResultType) + Send + Sync + 'static, + Res: FnOnce(&mut Env, R) -> napi::Result + Send + Sync + 'static, + R: Send + Sync + 'static, + V: NapiRaw + 'static, +{ + if async_java_exception_objects { + env.execute_tokio_future(futures::future::lazy(|_| Ok(func())), move |env, res| { + resolver(env, res.map_napi_err(Some(*env))?) + }) + } else { + env.execute_tokio_future( + futures::future::lazy(move |_| func().map_napi_err(None)), + resolver, + ) + } +} diff --git a/test/StringTest.test.ts b/test/StringTest.test.ts index fb5a642..933f8b1 100644 --- a/test/StringTest.test.ts +++ b/test/StringTest.test.ts @@ -215,4 +215,27 @@ describe('StringTest', () => { .which.has.property('printStackTrace') .which.is.a('function'); }); + + it('import non-existing class async', async () => { + await expect( + importClassAsync('java.lang.NonExistingClass', { + asyncJavaExceptionObjects: true, + }) + ) + .to.eventually.be.rejected.and.to.have.property('message') + .which.satisfies((val: string) => + val.startsWith( + 'java.lang.ClassNotFoundException: java.lang.NonExistingClass' + ) + ); + + await expect( + importClassAsync('java.lang.NonExistingClass', { + asyncJavaExceptionObjects: true, + }) + ) + .to.eventually.be.rejected.and.to.have.property('cause') + .which.has.property('printStackTrace') + .which.is.a('function'); + }); }); From a786b18fe200c31fb195c19016bf501e6999b6b4 Mon Sep 17 00:00:00 2001 From: MarkusJx <28785953+markusjx@users.noreply.github.com> Date: Sun, 21 Apr 2024 11:30:24 +0200 Subject: [PATCH 5/5] chore: run prettier Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --- README.md | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 06e6d75..ac0ecea 100644 --- a/README.md +++ b/README.md @@ -46,12 +46,12 @@ using `npm i`, you can skip this._ In order to build this project, you should install -- Node.js -- npm -- rustc, the rust compiler -- cargo -- Java JDK 8+ -- clang +- Node.js +- npm +- rustc, the rust compiler +- cargo +- Java JDK 8+ +- clang Then, to build the project, run: @@ -66,15 +66,15 @@ npm run build > available_ | Operating System | i686 | x64 | arm | arm64 | -|------------------|:----:|:---:|:---:|:-----:| -| Linux | - | ✅ | - | ✅ | -| Windows | ✅ | ✅ | - | - | -| macOS | - | ✅ | - | ✅ | +| ---------------- | :--: | :-: | :-: | :---: | +| Linux | - | ✅ | - | ✅ | +| Windows | ✅ | ✅ | - | - | +| macOS | - | ✅ | - | ✅ | ### Known working linux distros | Distro | Version | -|:------:|:-------------:| +| :----: | :-----------: | | Ubuntu | `>= 20.04` | | Debian | `>= bullseye` | @@ -143,12 +143,12 @@ electron-builder, you can do this by adding the following to your `package.json` ```json { - "build": { - "asarUnpack": [ - "node_modules/java-bridge/**", - "node_modules/java-bridge-*/**" - ] - } + "build": { + "asarUnpack": [ + "node_modules/java-bridge/**", + "node_modules/java-bridge-*/**" + ] + } } ``` @@ -358,10 +358,10 @@ all features enabled. Logged events include: -- Class loading -- Method calls -- Class instance creation -- Method and class lookup +- Class loading +- Method calls +- Class instance creation +- Method and class lookup **Note:** Logging affects the performance of the module. Thus, it is recommended to only enable logging when debugging.