karate.callSingle/callonce crashes where karate.call succeeds

[zgael]

Hi,
In our karate-config, I use a lot of Java.type('XXX') to call custom Java code.
As it is somehow "heavy" (authentication against a provider), I want this to get called only once for all my tests, and not before each of my 400+ tests (thus the callSingle in karate-config, instead of having all the code in karate-config).

In karate 0.9.6, the following worked :

karate-config.js :

function configure_karate() {
    var load_config=karate.callSingle('classpath:load-config.js')
    return load_config
}

load-config.js :

function load_config() {
    var config = {}
    config.spec = Java.type('package.CustomJavaClass').spec // spec if a static function of CustomJavaClass
    return config
}

while it crashes using the latest Karate releases (tried with both 1.0.1 and 1.1.0.RC2) with following stacktrace :

org.graalvm.polyglot.PolyglotException
- com.intuit.karate.graal.JsFunction.<init>(JsFunction.java:39)
- com.intuit.karate.core.ScenarioEngine.recurseAndDetachAndDeepClone(ScenarioEngine.java:1208)
- com.intuit.karate.core.ScenarioEngine.lambda$recurseAndDetachAndDeepClone$15(ScenarioEngine.java:1225)
- java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:723)
- com.intuit.karate.core.ScenarioEngine.recurseAndDetachAndDeepClone(ScenarioEngine.java:1225)
- com.intuit.karate.core.ScenarioEngine.recurseAndDetachAndDeepClone(ScenarioEngine.java:1201)
- com.intuit.karate.core.ScenarioBridge.callSingle(ScenarioBridge.java:235)

I know there have been huge changes from 0.9.6 to 1+, but what bugs me is that using karate.call('classpath:load-config.js') works in karate 1+ versions.

So, the behavior is different between karate.call and karate.callSingle.

After some digging into karate inner sources, starting from ScenarioBridge, and some testing, the problem is due to the fact
spec is a function, and karate.call and karate.callSingle handle functions differently.

To my understanding, those functions should have the same behavior, with the latter caching the result for subsequent calls.
Am I wrong ?
I set up a minimal project in this repo for easier reproduction.
https://github.com/zgael/karateconfig

Feel free to ask for clarifications if needed !

Thanks!

[ptrthomas]

@zgael so I've reached the limit of my patience with #1558 and am declaring this a won't fix. you can read that thread for the gory details. but you are super-most-welcome to dive into the code and see what we may have missed. I've added tests that replicate this problem - just comment out 1 line in karate-config.js in the test-package: com.intuit.karate.core.parallel

so please don't mix java functions into a callSingle. we struggled to get java class-types into a callSingle.
methods seem to "bring along" that $#@% graal context and promptly throws the Multi threaded access requested by thread error. I think calling methods on a Java.type() (a Java class instance) work okay though but now not so sure.

why on earth do people use a call-single and mix Java methods into it >_< please don't. I'm tempted to declare that callSingle() will only support plain-old-JSON-objects from now on

cc @joelpramos and @ericdriggs - if you can re-test your project that had the problem in the other thread that would help, because I changed things around again

[ptrthomas]

wow the build failed with this now. so I'll revert the removal of the synchronize in the next commit:

EDIT: I re-ran the build and it passed. so I hope this was a one-off flake. will watch it though

[joelpramos]

@ptrthomas it is frustrating but from a capability perspective not sure whether we should call it a won't fix - probably something we should keep jumping through the hurdles and assume one day we'll cover all the cases. There aren't a lot of reports coming out so the end should be near.

Since I've revisited this piece a few times I'll try my best to check this example in the coming weeks when I have some time.

[ptrthomas]

@joelpramos sure. I still think that putting ANY kind of Java (class / or methods) does not make sense for a callSingle()

[joelpramos]

I just realized what you meant with Java functions... The results of Java functions calls can still be stored / used it's just storing a reference to a function that does not work. I'm inclined to agree with you that's not THAT problematic in that case (if really needed store the class) :)

I just tried a fix by wrapping the Java function with a Graal Proxy. Saw something similar here https://lburgazzoli.github.io/2018/08/06/Adventures-in-GraalVM-invoke-Java-code-from-JS-in-native-image.html which reminded me that I read somewhere before when investigating this that proxying calls was a viable solution.

Seems to pass the unit test but would need @zgael to confirm and your review.

[ptrthomas]

@joelpramos wow, it seems to work ! that is a good find, I had no idea. re-opening for @zgael to confirm

[ptrthomas]

@joelpramos build just started failing consistently :| looks like github actions upgraded the os or jvm

[zgael]

so please don't mix java functions into a callSingle. we struggled to get java class-types into a callSingle.
methods seem to "bring along" that $#@% graal context and promptly throws the Multi threaded access requested by thread error. I think calling methods on a Java.type() (a Java class instance) work okay though but now not so sure.

We have a workaround, that is indeed "call methods on Java.type()" (and that works!).
We wrote that in 0.9.6 for readability, coz that's a function (let's name it specialize) that is omnipresent, so calling just specialize() in our feature backgrounds is easier to write/read than myclass.specialize().

Now that it doesn't work in 1+ versions, we obviously can use the workaround, but I prefered to open an issue to improve karate :)

I'll be happy to test and confirm it works in my case, but I'm unavailable until the end of the month, so I can only test it by then.
Hope it's not too late for you.

[ptrthomas]

sure. one of the points above is that you shouldn't put Java methods and such in a callSingle() do it as many times as you want in a normal call. callSingle() is designed for caching data, not java code.

for readability, you can wrap it into a feature following the instructions here: https://github.com/intuit/karate#multiple-functions-in-one-file

no not too late, as of now I consider we are done with the fixes here. FWIW here is what we have in the documentation in the next version - screenshot below: https://github.com/intuit/karate/tree/develop#karatecallsingle

[joelpramos]

@joelpramos build just started failing consistently :| looks like github actions upgraded the os or jvm

ugh

any ideas on how to triage that don't involve keep using github actions? do you know if there's a page where they post any sort of us / java updates?

[ptrthomas]

am only aware of pages like this @joelpramos https://jdk.java.net/java-se-ri/11

[zgael]

I'm back with good news ! Latest develop branch (commit a383a00) works for my case.
I also tested something else, just to see how it goes :

public class CustomJavaClass {
  public static String fails(){return "MyString";}
  public Function<String,String> works=MyJavaClass::test2;
}

// inside callSingle JS file
config.fails = Java.type('package.CustomJavaClass').fails
config.works = Java.type('package.CustomJavaClass').works

In "old" karate versions (between 1.0.0 and 1.1.0.RC2), the fails function fails to translate from java to js (see the beginning of this issue).
But declaring a Function type works even with those "old" versions.

Plus, both of those work with latest develop branch (commit a383a00), so I guess that's a good job with all those attempts @ptrthomas @joelpramos !
I'll let you handle the workflow if you only close issues when you merge into master, but that's all for me!

Thanks for your time.

[ptrthomas]

@zgael okay, that's great to hear. but there is a catch, if you invoke fails() or works() at the same time in 2 different parallel threads, and the JS context in which you called Java.type() is also active - it is very likely to fail. I tried a lot to solve this, but failed - just so you know.

[ptrthomas]

to be clear. you can pass a "java type" around. just that "java method references" are not recommended because they retain the originating JS context.

so you should be able to do this:

config.fails = function(x){  return Java.type('package.CustomJavaClass').fails(x)  }

but again, the recommendation is that you should not return functions (even JS ones) from a callSingle(). unless you can help us with the source code of course ;)

[ptrthomas]

attention all cc @zgael @joelpramos @aleruz

I figured out a way where you can use Java functions safely even within a callonce or karate.callSingle(), it is not completely straightforward, but I think a very reasonable compromise. it makes me relieved because I am confident Karate can be made to work for advanced JMS and Kafka kinds of use-cases in a stable way, where earlier I thought graal would very badly get in the way

see doc screenshot below, and direct link is here: https://github.com/intuit/karate/tree/develop#java-function-references