MarkusJx / node-java-bridge

A bridge between Node.js and Java
https://markusjx.github.io/node-java-bridge/
MIT License
115 stars 6 forks source link

[feat] Add JS stacktrace #77

Closed sosoba closed 1 year ago

sosoba commented 1 year ago

Hello Markus. Thank you for great project.

Is your feature request related to a problem? Please describe. Currently, the stack trace contains information about the Java code but not the location in the JS code. Ex:

import jb from 'java-bridge';
const javaURL = jb.importClass('java.net.URL');
await javaURL.newInstanceAsync('');
[Error: java.net.MalformedURLException: no protocol:
    at java.base/java.net.URL.<init>(URL.java:674)
    at java.base/java.net.URL.<init>(URL.java:569)
    at java.base/java.net.URL.<init>(URL.java:516)
    at C:\Users\runneradmin\.cargo\git\checkouts\java-rs-8df0abe5d5de942a\6fc9654\src\java\java_env_wrapper.rs:1281
    at C:\Users\runneradmin\.cargo\git\checkouts\java-rs-8df0abe5d5de942a\6fc9654\src\java\java_env_wrapper.rs:278] {
  code: 'GenericFailure'
}

Describe the solution you'd like

Error: java.net.MalformedURLException: no protocol:
    at java.base/java.net.URL.<init>(URL.java:674)
    at java.base/java.net.URL.<init>(URL.java:569)
    at java.base/java.net.URL.<init>(URL.java:516)
    at async file:///C:/workspace/test.js:3:1 {
  code: 'GenericFailure'
}

Describe alternatives you've considered Currently I surround each of the calls with manual code:

await javaURL.newInstanceAsync('').catch(onrejected);
const onrejected = reason => {
  if (reason instanceof Error) {
    Error.captureStackTrace(reason, onrejected);
  }
  throw reason;
};

Could you add similar wrappers automatically to every Js to Java call?

MarkusJx commented 1 year ago

That is an interesting observation, I guess. This issue only seems to be present with async function calls, sync calls do show a stack trace.

I'll need to take a deeper look into these async routines, this may be caused by the async wrappers in napi-rs.

But adding wrappers to each returned value from the addon is not a viable option in my opinion, I did this once and it was a huge pain, plus the result was basically unmaintainable, since every method in every object needs to be replaced.

I'll post further updates in here, so please be patient, this may take a while

MarkusJx commented 1 year ago

I've looked a bit into this issue. There has been a similar discussion about this issue in node-addon-api (which internally uses the node api, I think): https://github.com/nodejs/node-addon-api/issues/595.

It seems like nodejs doesn't include a stack trace from the javascript process if the error has been created inside another thread, which is the case with errors returned by promises. The solution for this issue is also mentioned in the previously linked thread: SImply create an error in the main thread before starting the promise thread. If an error is thrown inside the promise thread, copy all properties of this error to the error created in the main thread. Only downside to this approach is that a new error has to be created with every promise returned, which seems suboptimal.

I've created a small poc in rust for napi-rs, I'll probably create a PR at some point. Performance impact doesn't seem to be that bad.

In the meantime, another solution for this specific issue would be using trace. Seems to work fine, don't know what the performance impact of that package is.

MarkusJx commented 1 year ago

Update: https://github.com/napi-rs/napi-rs/pull/1637 has been merged, so we just have to wait until the next version of napi-rs has been released. Then I can enable stack traces from async contexts. Hopefully.