test(bridge): test import and method not found errors (#105)

* test(bridge): test import and method not found errors Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> * test(bridge): increase number of retries Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> * chore: run prettier Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> * feat(bridge): expose Java throwable from importClassAsync Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> * chore: run prettier Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com> --------- Signed-off-by: Markus <28785953+MarkusJx@users.noreply.github.com>
MarkusJx · Apr 21, 2024 · ec3d822 · ec3d822
1 parent b0a7fae
commit ec3d822
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 58 deletions.
diff --git 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,36 +103,50 @@ 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<ClassConfiguration>,
     ) -> napi::Result<JsFunction> {
-        let proxy = MutAppState::<ClassCache>::get_or_insert_default()
+        let proxy_result = MutAppState::<ClassCache>::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)
     /// 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<object>")]
     pub fn import_class_async(
-        &'static mut self,
+        &self,
         env: Env,
         class_name: String,
         config: Option<ClassConfiguration>,
     ) -> napi::Result<JsObject> {
-        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::<ClassCache>::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
@@ -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<F>(env: Env, proxy: Arc<JavaClassProxy>, func: F) -> napi::Result<JsObject>
-    where
-        F: (FnOnce() -> ResultType<JavaCallResult>) + 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<F, Res, R>(
-        env: Env,
-        async_java_exception_objects: bool,
-        func: F,
-        resolver: Res,
-    ) -> napi::Result<JsObject>
-    where
-        F: (FnOnce() -> ResultType<R>) + Send + Sync + 'static,
-        Res: FnOnce(&mut Env, R) -> napi::Result<JsUnknown> + 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<JsObject> {
     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
@@ -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<T> = Result<T, Box<dyn Error + Send + Sync>>;
 
@@ -72,3 +76,42 @@ pub fn parse_classpath_args(cp: &[String], args: &mut Vec<String>) -> String {
         cp.join(separator::CLASSPATH_SEPARATOR)
     )
 }
+
+pub fn call_async_method<F>(env: Env, proxy: Arc<JavaClassProxy>, func: F) -> napi::Result<JsObject>
+where
+    F: (FnOnce() -> ResultType<JavaCallResult>) + 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<F, Res, R, V>(
+    env: Env,
+    async_java_exception_objects: bool,
+    func: F,
+    resolver: Res,
+) -> napi::Result<JsObject>
+where
+    F: (FnOnce() -> ResultType<R>) + Send + Sync + 'static,
+    Res: FnOnce(&mut Env, R) -> napi::Result<V> + 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/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);
+    })
+        .timeout(timeout)
+        .retries(3);
 
     it('Delete deleted instance', () => {
         const JString = importClass<typeof StringClass>('java.lang.String');

diff --git a/test/StringTest.test.ts b/test/StringTest.test.ts
@@ -183,4 +183,59 @@ 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');
+    });
+
+    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');
+    });
 });