Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraalVM JavaScript replaces Nashorn, breaking Blockly and existing Nashorn rules #2433

Closed
rkoshak opened this issue Jul 20, 2021 · 200 comments
Closed

Comments

@rkoshak
Copy link

rkoshak commented Jul 20, 2021

Is there a way to have Nashorn and the GraalVM JavaScript add-on live side by side? Now it appears to completely replace Nashorn which breaks things.

I've had to deal with two problems in two days caused by the fact that GraalVM JavaScript is installed and then the users are trying to use a Nashorn rule or to use Blockly.

If it is not possible, I'll move this issue over to the webuis repo to have Blockly removed as an option to build rules when the add-on is installed. However, that's not a very satisfying approach. Installing an add-on shouldn't replace or break stuff that is built into openHAB be default in my opinion.

@wborn
Copy link
Member

wborn commented Sep 26, 2021

Is there a way to have Nashorn and the GraalVM JavaScript add-on live side by side?

I don't think so currently. But it would be nice if the core would support multiple versions of script engine languages. That way you could slowly migrate your rules from one major version to another. But it would also mean that script engine versions need to be stored for rules created in the UI and there needs to be a way to determine the script engine version for scripts in files.

Because Nashorn has been removed from Java 15, there won't be any ECMAScript 5.1 script engine available in newer Java versions (unless an Automation add-on is created that provides Nashorn (openhab/openhab-distro#1316 (comment))).

Eventually the Blockly integration will also need to support generating code for newer ECMAScript versions. So it wouldn't hurt if the UI enables Blockly depending on the available ECMAScript engine version. It would be even nicer if the Blockly integration supports generating code for multiple ECMAScript versions. 🙂

@rkoshak
Copy link
Author

rkoshak commented Sep 27, 2021

Given how different the GraalVM add-on implements the interactions between rules and openHAB itself compared to Rules DSL, Nashorn, and Jython, this might be a chance to step back and think about how we want it to work. To a large extent, it's going to be way more than just migration from ECMAScript 5.1 to ECMAScript whatever GraalVM supports. It's going to be reimplementation in how everyone needs to write rules.

In the traditional languages all sorts of things are imported and available to the rules coder by default:

  • items
  • events
  • event
  • actions
  • itemRegistry/ir
  • thingRegistry

There's more but those are the most used. In GraalVM JavaScript a design decision was made to import nothing. And it's not clear in the current set of documentation how to access that stuff. But to access that stuff means each and every rule/Script Action/Script Condition is going to require a whole bunch of import statements/load statements just to do simple stuff like logging or sending a command to an Item.

It might be a good idea to bring this up as part of any conversation when moving to Java 15+. GraalVM isn't just an updated JavaScript support.

@ghys
Copy link
Member

ghys commented Sep 27, 2021

I downloaded the newest distribution, installed the GraalVM JS add-on, and tried a new Blockly script and indeed it doesn't work but this is the actual error:

20:39:21.003 [ERROR] [ript.internal.ScriptEngineManagerImpl] - Error while creating ScriptEngine
org.graalvm.polyglot.PolyglotException: Error: Invalid CommonJS root folder: I:\dev\OPC442~1.0-S\conf\automation\lib\javascript\personal
        at org.graalvm.polyglot.Context.eval(Context.java:345) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSScriptEngine.evalInternal(GraalJSScriptEngine.java:319) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.initGlobal(GraalJSBindings.java:94) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.initContext(GraalJSBindings.java:90) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.requireContext(GraalJSBindings.java:84) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.put(GraalJSBindings.java:127) ~[?:?]
        at javax.script.SimpleScriptContext.setAttribute(SimpleScriptContext.java:246) ~[java.scripting:?]
        at org.openhab.core.automation.module.script.internal.ScriptEngineManagerImpl.addAttributeToScriptContext(ScriptEngineManagerImpl.java:254) ~[?:?]
        at org.openhab.core.automation.module.script.internal.ScriptEngineManagerImpl.createScriptEngine(ScriptEngineManagerImpl.java:144) ~[?:?]
        at org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler.createScriptEngine(AbstractScriptModuleHandler.java:92) ~[?:?]
        at org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler.getScriptEngine(AbstractScriptModuleHandler.java:88) ~[?:?]
        at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.execute(ScriptActionHandler.java:59) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.executeActions(RuleEngineImpl.java:1183) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1035) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1051) ~[?:?]
        at org.openhab.core.automation.rest.internal.RuleResource.runNow(RuleResource.java:327) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
        at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
        at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
        at org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:179) ~[bundleFile:3.4.3]
        at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96) ~[bundleFile:3.4.3]
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:201) ~[bundleFile:3.4.3]
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:104) ~[bundleFile:3.4.3]
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:59) ~[bundleFile:3.4.3]
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:96) ~[bundleFile:3.4.3]
        at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:265) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:225) ~[bundleFile:3.4.3]        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:298) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:217) ~[bundleFile:3.4.3]        at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) ~[bundleFile:3.1.0]
        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:273) ~[bundleFile:3.4.3]
        at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(HttpServiceServletHandler.java:71) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:602) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:294) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:82) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.Server.handle(Server.java:516) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:386) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034) [bundleFile:9.4.43.v20210629]
        at java.lang.Thread.run(Thread.java:834) [?:?]
20:39:21.063 [WARN ] [re.automation.internal.RuleEngineImpl] - Fail to execute action: script
java.lang.NullPointerException: null
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.put(GraalJSBindings.java:128) ~[?:?]
        at javax.script.SimpleScriptContext.setAttribute(SimpleScriptContext.java:246) ~[java.scripting:?]
        at org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler.setExecutionContext(AbstractScriptModuleHandler.java:117) ~[?:?]
        at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.lambda$0(ScriptActionHandler.java:60) ~[?:?]
        at java.util.Optional.ifPresent(Optional.java:183) ~[?:?]
        at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.execute(ScriptActionHandler.java:59) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.executeActions(RuleEngineImpl.java:1183) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1035) ~[?:?]
        at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1051) ~[?:?]
        at org.openhab.core.automation.rest.internal.RuleResource.runNow(RuleResource.java:327) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
        at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
        at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
        at org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:179) ~[bundleFile:3.4.3]
        at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96) ~[bundleFile:3.4.3]
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:201) ~[bundleFile:3.4.3]
        at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:104) ~[bundleFile:3.4.3]
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:59) ~[bundleFile:3.4.3]
        at org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:96) ~[bundleFile:3.4.3]
        at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:265) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:225) ~[bundleFile:3.4.3]        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:298) ~[bundleFile:3.4.3]
        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:217) ~[bundleFile:3.4.3]        at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) ~[bundleFile:3.1.0]
        at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:273) ~[bundleFile:3.4.3]
        at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(HttpServiceServletHandler.java:71) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:602) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:294) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) ~[bundleFile:9.4.43.v20210629]
        at org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:82) ~[bundleFile:?]
        at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.Server.handle(Server.java:516) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388) ~[bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:386) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883) [bundleFile:9.4.43.v20210629]
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034) [bundleFile:9.4.43.v20210629]
        at java.lang.Thread.run(Thread.java:834) [?:?]

So from my point of view, maybe we could focus on this error and try to fix it before bailing out and saying installing the GraalVM add-on breaks Blockly because of compatibility issues? In terms of the ECMAScript language, if Blockly generates code targeting a certain language version, subsequent versions should be able to interpret it still. It seems that the JSScripting add-on expects some directories and will fail if they are not present, this seems easy enough to fix?

@rkoshak
Copy link
Author

rkoshak commented Sep 28, 2021

It's significantly more involved than just an upgrade to the ECMAScript issue. In addition to using a different folder structure (which is the root of the error above), the GraalVM JavaScript add-on does not import anything from openHAB. In Nashorn we are used to having:

  • items dict for Item states
  • event for the event that triggered the rule
  • events to send commands and updates
  • ir/itemRegistry to access Item Objects when you need more than just the state
  • actions to access binding and built in actions
  • etc.

GraalVM JavaScript makes none of that available. You have to go and manually import all of those things manually if you want to use them in your script.

So even when that error gets fixed, Blockly would still need to be significantly changed to produce code that will work for the GraalVM JavaScript and that code will not be compatible with Nashorn.

For all intents and purposes Nashorn JavaScript and GraalVM JavaScript may as well be two completely different languages.

@ghys
Copy link
Member

ghys commented Sep 28, 2021

I see, thanks for the explanation. I would have thought these were provided by DefaultScopeProvider and available regardless of the script engine.

Still, it's not that far of a stretch to adapt Blockly code to make these imports when needed - it's the case already when you use the "log" block (https://github.com/openhab/openhab-webui/blob/d8a16db016b149d007ef31a320fcdc000b8c51dc/bundles/org.openhab.ui/web/src/assets/definitions/blockly/ohblocks.js#L124-L132).

My point is that with Nashorn deprecated, Jython unmaintained (and a EOL 2.7 version of Python), DSL Rules being legacy, and Groovy being kind of niche, GraalVM might be the only way viable option left (with JRuby?) as a embedded scripting engine going forward, so we should try to make it work.

@rkoshak
Copy link
Author

rkoshak commented Sep 28, 2021

The developer went out of his way to avoid polluting the namespaces so rules could be pure JavaScript. There are legitimate reasons for doing so but it's not clear to me how the DefaultScopeProvider is used but it's not in the same way as it happens in Nashorn. In Nashorn the variables are just there thanks to the provider but in GraalVM JS rules they are not.

I don't disagree that we should try to make it work with something that will have support in the long run. But in the mean time GraalVM JavaScript will continue to break anything that is written for Nashorn which will trip up any user who wants to use Blockly or existing Nashorn code. If it's going to be months or years before Blockly can be

Given that GraalVM JavaScript has almost no documentation yet, pretty much only those who have the skills and willingness to study @jpg0's library are able to really build anything with it which makes moving Blockly over to support it somewhat of a challenge. I also don't know if anyone is trying to build rules in the UI with it so there may be gotchas we are unaware of. I know the jRuby folks too are ignoring UI rules. I haven't tried to keep up with their documentation though. They might be a bit more well documented but I don't know if they've even submitted jRuby to be considered as an official add-on yet.

I filed an issue on openhab-core to see if we could get Nashorn and GraalVM to exist side-by-side which would be the best way to bridge between what we have now to where we need to go but that doesn't seem to be possible. It's either or, not both.

I'm worried about months and months or even years between now and when Blockly can support something more future proof where users can't figure out why stuff is broken and not working because they installed an add-on.

I'd love to hear @jpg0's opinion on how to best to proceed.

@jpg0
Copy link
Contributor

jpg0 commented Sep 28, 2021

Firstly, I'd like to apologize for the slow progress on the add-on and docs; I've recently moved house (well, moved the family from one side of the world to the other) so have had other priorities.

As for work on the GraalVM add-on, the plan is to update the folder structure (to make it compatible with the JS ecosystem), then write the docs.

@rkoshak is correct that the add-on diverges from the Nashorn by not importing things by default. Default things can be imported explicitly if required - they are at @runtime, as opposed to @runtime/<something>. It would be possible to add some boilerplate at the top of GraalVM scripts to import default things at which point they would probably be mostly compatible with Nashorn (but not 100%, as Nashorn is not 100% compatible with JS or GraalVM).

Anyway, regarding side-by-side installs - when you install the GraalJS add-on, in fact it doesn't replace the Nashorn engine (that engine is actually still used for JS transformations), however when openHAB attempts to find an engine for a script it looks it up by file extension, and only one engine can be returned.

To address your concern, it seems that the challenge is that neither Nashorn nor Graal can accept each others' scripts. Ideally we would have a backwards-compatible upgrade. The options that I can see are:

  • create an intermediate Graal which does import things, then a 2nd version which removes the imports (although is this really solving anything, or just deferring the issue?)
  • explicitly allow concurrent execution, either by allowing switching of the engine within openHAB core, or by choosing 2nd file extension to disambiguate engines
  • Attempt to assist users through this non-compatible upgrade - with things like modifying existing scripts and very explicit error messages with the assumption that failures may occur due to old scripts (you tried to reference foo but it's not explicitly imported any longer, try: let foo = ...)

Of course there is also the option to never move to this 'everything explicit' model, but that forever shuts out the ability to rely on and integrate with existing 3rd party JS.

@rkoshak
Copy link
Author

rkoshak commented Sep 28, 2021

I don't think you need to apologize. We all have life commitments. It is certainly not the only place where openHAB docs are lacking.

or just deferring the issue?

Deferring the issue though could be a good thing if it keeps people from being confused and breaking their OH in the time between now and a final solution is implemented. It really is causing problems for all sorts of users, even OH users who have been using OH for years.

explicitly allow concurrent execution, either by allowing switching of the engine within openHAB core, or by choosing 2nd file extension to disambiguate engines

That only addresses text based rules, not UI rules. There's no file in that case. The engine is chosen, presumably, based on the "type" property of the action configuration. However, the version of the language doesn't seem to be a part of that. It's just application/javascript for a Nashorn Script Action/Condition.

This does seem like it might be an OK approach long term perhaps. I can imagine that there might be similar incompatible versions of languages that might gain support sometime in the future.

But that's all stuff that has to happen over on openhab-core I think.

@ghys
Copy link
Member

ghys commented Sep 29, 2021

So in the Blockly case, after reading openhab/openhab-addons#11221 and creating the missing directory (something that should IMHO be needed but probably easy to fix), given that the code generated by Blockly only uses a couple objects for now (namely itemRegistry and events), we could either:

  1. prepend this code or similar to all scripts to make these objects available as global variables when using GraalVM:
var runtime = (typeof(require) === "function") ? require("@runtime") : undefined;
if (runtime) { 
  var itemRegistry = runtime.itemRegistry, events = runtime.events; // etc.
}
  1. prepend this code or similar to all scripts to make these objects available under runtime in both GraalVM and Nashorn (and use runtime.itemRegistry/runtime.events instead of itemRegistry/events in generated blocks:
var runtime = (typeof(require) === "function") ? require("@runtime") : {
  itemRegistry: itemRegistry,
  events: events,
  // etc.
};

The trick is that require is not defined in Nashorn so that way we can detect which engine is used to run the code.

Then I confirmed that (in the 1st alternative):

print(itemRegistry);
print(events);

resp. in the 2nd alternative:

print(runtime.itemRegistry);
print(runtime.events);

yields the same result in both Nashorn & GraalVM:

org.openhab.core.internal.items.ItemRegistryImpl@d5a9c50
org.openhab.core.automation.module.script.internal.defaultscope.ScriptBusEvent@1c23f2fd

With that simple addition to Blockly-generated code we could then still offer it when the JSScripting add-on is installed (the rest of the generated ECMAScript code being simple enough should hopefully work in both ECMAScript versions). Maybe ithis add-on could even become installed as part of the standard package in the future, like RRD4j persistence is today.

@jpg0
Copy link
Contributor

jpg0 commented Sep 29, 2021

@ghys you may also want to merge everything from the runtime into the current context (with something like Object.apply(this, runtime) so as to ensure everything is available, but either way, your approach should work fine.

@rkoshak Another thing I was thinking about was the option to explicitly version, possibly via adding to the mime types in this case (I realised that if we do change the root directories for JS with GraalVM, then we can use this to indicate the version for files). I believe that the mime-types are simply strings, so it would be perfectly possible for the new implementation to register with something like application/javascript;version=2021. Additionally, the script engine manager can actually determine the language version for each engine (it does so when it first sees one), although it currently doesn't actually do anything with it. This would mean that it would technically be possible for language versioning to be added to the ScriptEngineManager, so that it could choose to register script engines with a versioned mime type in addition to their normal one, at which point consumers would be able to use versioned mime types to specify which script engine to use. Note however that this versioned string is not standard; whilst it's fine to use as a Content-Type parameter in HTTP, it's not usually part of the actual mime-type. (Note that it would be possible therefore to also add explicit language versions to the script engine manager directly, but that would require consumer changes to specify another parameter if they want an explicit version.)

@rkoshak
Copy link
Author

rkoshak commented Sep 29, 2021

@jpg0 I like the idea of being able to define the version number somehow. It seems to me that this is only the first time that we will run into this problem but indeed it will come up again and again. For example, what happens when ECMAScript 2022 or what ever comes out? Having the version that a given rule/script was written for can also help with your third proposal with error messages. "I see you used foo. In the new JavaScript XXXX you should now use bar."

@ghys, this is fantastic news. I would really really like to be able to move towards selecting a "default" language that isn't already deprecated and will disappear as soon as OH needs to move to a new JVM and it makes sense that the default would be the same thing backing Blockly. GraalVM JavaScript is a good fit for this, I've just not had the time to figure out how to use it yet. But I agree, having it installed as a default like rrd4j seems to be a good idea once we can ensure that doing so doesn't break Nashorn.

Doing so in the near term will give users time to migrate to GraalVM JavaScript (or maybe other languages too, Python would be greatly appreciated by a lot of users). Heck, I'll even write some tutorials for how to do it and contribute to some docs if I can. The trouble for me right now is time but I've got some off work time coming up so maybe I can look into it then.

I've been relatively unhappy with the thought of writing rule templates for the marketplace using languages that are basically deprecated and will need to be rewritten.

@ghys
Copy link
Member

ghys commented Sep 29, 2021

@ghys you may also want to merge everything from the runtime into the current context (with something like Object.apply(this, runtime) so as to ensure everything is available, but either way, your approach should work fine.

Yes, thanks, it makes sense! For now as I've mentioned earlier only 2 objects are needed from the openHAB API in Blockly so it's also reasonable to expose only those two but as we eventually add functionality it may become the go-to option. The good thing is that the actual JS code generated by Blockly can be changed on a whim because it's regenerated in full whenever the source blocks change.

creating the missing directory (something that should IMHO be needed but probably easy to fix)

I obviously meant not be needed, at least manually.

I would really really like to be able to move towards selecting a "default" language that isn't already deprecated and will disappear as soon as OH needs to move to a new JVM and it makes sense that the default would be the same thing backing Blockly

Completely agree with this and all of your points. I'm almost ready to add rule template support in the UI and eventually open the marketplace to the public, but I'm actually worried about the stability of the script languages that might be used in them - not only languages but also API differences to address OH objects since we have 2 JS engines and they are incompatibilities between them. So before we do that we might have to settle on a common API and stick to it - or even maybe mandate GraalVM for rule templates that have JS scripts in them so every script would have to require(@runtime) - fine by me.
Again I don't see another viable option other than GraalVM for a future-proof way to go for embedded scripting in openHAB, since it's the only one that is actively maintained and offers a modern language.

@digitaldan
Copy link
Contributor

Sorry if this is has been discussed before (I'm sure it has) , but is there a reason we are not injecting the runtime objects by default into the GraalVM instance? Or at least making it an option (turned on by default)? My assumption is that nearly every user (90%+ maybe?) switching to use GraalVM would need to add those to their scripts , seems like a lot of forced boiler plate code to make users copy and paste. I'm sure there's a reason not too, hence maybe making it configurable. I for one really struggled with this when i was trying it out a few months ago, it made what is a super awesome feature feel not production ready.

@rkoshak
Copy link
Author

rkoshak commented Sep 29, 2021

but is there a reason we are not injecting the runtime objects by default into the GraalVM instance?

There's a balancing act that has to be taken between injecting stuff and potentially polluting the namespace with the ability to support third party libraries which might use the same variables. My understanding is that @jpg0 choose the side that maintains the maximum ability to use any third party libraries (e.g. like libraries installed using npm, not just OH specific libraries) over making less work for rules developers who can rely on openHAB stuff just being there.

I personally am not sure where the best balance is. I do agree that it does seem like a lot fo boiler plate that has to be added, especially when working in the UI where one could have more than one Script Action and Script Condition per rule and each one would have to re-include the boilerplate. I'd almost suggest that with UI scripts that stuff be included by default no matter what through some behind the scenes magic or make it configurable as you suggest. People writing rules in the UI are unlikely to ever install a third party library via npm or any other way and the boiler plate is most glaring there.

But even with Nashorn we are stuck with a bunch of boiler plate already. I can't call logInfo, I have to first Java.type(var Log = Java.type("org.openhab.core.model.script.actions.Log"); to even get access to the logger and then I have to call Log.logInfo(). And how do I know that's the right class to type? Because I'm aware of the JavaDocs and know enough programming for how to use them. Most of our users don't though.

So even though it's a lot of boiler plate, it's not really any more boiler plate than we already have to do. To anyone switching from Jython or Nashorn JavaScript it's just a different set of boiler plate, not really more boiler plate. And anyone switching from Rules DSL to any other language already has to add a lot of boiler plate to their rules.

And really, this is what the Helper Libraries are all about. A way to abstract and consolidate all of that boiler plate and even more boilerplate once you look at what it takes to actually create a rule in a text file in any of these languages. Without them (or their equivalent in GraalVM JavaScript or jRuby) writing even simple rules requires a level of understanding for openHAB internal classes and structure that is beyond what should be needed for our users. It has always been my opinion that the automation add-on by itself has never been sufficient on it's own. A helper library for that language is also required to make the interactions between OH and the script seamless. To date, none of the helper libraries that have been written are even part a part of the OH project though. And while I think it's theoretically possible to package a helper library as separate add-on there are none who have done so yet. @jpg0, correct me if I'm wrong but your helper library is installed through NPM. The jRuby helper library is installed through ruby gems. The Nashorn/JavaScript/Groovy? helper libraries are installed by cloning the github repo and copying the files to the OH conf folder. Why do I need to resort to an external tool to install the library that makes the language usable to write rules?

Were I king for a day, the helper libraries for these would be a part of the add-on so you get both. Then the docs become easier to write, the code gets easier for end users to write, and the overall experience is much nicer. But every time I bring this up I seem to get pushback which I don't understand.

Anyway, I'm off my soapbox. Rant over, for now. ;-)

@digitaldan
Copy link
Contributor

I wonder if it would make sense to have a generic "Javascript" binding which is more like a "meta" package, so mostly empty, but depends on both the GraalVM binding and a "GraalVM Runtime Helpers" binding which simply injects the runtime objects, logging ,etc... Most user users would install this generic "Javascript" binding (and get these two as dependencies) and be happy. Advanced users could install the raw GraalVM binding and have more control over the namespace.

@jpg0
Copy link
Contributor

jpg0 commented Oct 1, 2021

@rkoshak spot on with your explanation. Helper libraries are essential, and the standard JS module loading (require("mylibrary")) is so much better than the non-standard, fragile and overly complex code required in Nashorn.

@digitaldan My fear is that bifurcating script runtimes will mean that we have two 'flavours' of script which will lead to greater confusion in the future when users share their scripts etc. From many years of development experience I firmly believe that too much 'magic' always bites you in the end.

As a pretty trivial example, there is nothing stopping there being new variables added to the default context - in fact any part of openHAB core or any add-on can do it (and it is designed this way) - what do they choose as the name for the variable? Because if it has already been used in a script somewhere, this can cause nasty problems (and with existing names like event, I expect the chance of collision is high). My belief therefore is to keep things explicit, but as short as possible.

All you need at the top of your script is a statement like Object.assign(this, require('@runtime')); to choose to merge all defaults into the context. This would mean that existing scripts would continue to work, although TBH I would discourage it because it's explicitly choosing the dangerous path of defining who-knows-what.

Other, better, standard options:

  • const { itemRegistry } = require('@runtime'); itemRegistry.<something> //explicitly choose what is imported
  • const runtime = require('@runtime'); runtime.itemRegistry.<something> //explicitly choose namespace
  • require('@runtime').itemRegistry.<something> //explicitly reference from the module

(Note that it is even possible to put your code in a with statement that merges this context explicitly for a subset of the code:
with(require('@runtime')) { itemRegistry.<something> }) but note that (as in the link above) this is deprecated (and I'd advise against it) because it's a source of bugs/ambiguity/confusion - although less that the current openHAB approach!)

@rkoshak
Copy link
Author

rkoshak commented Oct 1, 2021

To plays devil's advocate here a little though. While I agree that too much magic can become a problem, and in fact I think it is a problem in Rules DSL, too much boilerplate can also be a problem.

When one is writing rules in text files where you might have 100+ lines of code with two or three rules defined adding one or two lines to the top of the file to import stuff you will almost always need is no big deal. It's just the cost of writing code that will work with openHAB.

However, when it comes to UI rules it becomes much more onerous. In my experience a typical Script Condition in Nashorn right now is about three lines of code that does real work plus another four to five lines of code to import the stuff I need even with all the magic that Nashorn provides with items and event and whatnot.

It's not quite so bad in Script Actions as those, again in my experience, average around 20 lines of code that do real work again with another four to five lines of code to import stuff. But that still means roughly 1/5th of the code I need to write for each and every Script Action (and there can be more than one Script Action in a single rule) is boilerplate. I'm always gonna want to log. I'm always gonna want to interact with Items. I frequently need to interact with the ItemRegistry and openHAB core Actions (which are harder to use than binding provided Actions today). So I'll have to import these for each and every Script Action I write.

If you assume one Script Condition and one Script Action per rule and a modest 15 rules, that's a lot of boiler plate. On the low end that's 120 lines of code just to get started. We haven't even begun to actually implement anything useful, we've just imported the stuff we need to interact with OH.

At least in the context of UI written rules, anything but the "dangerous path" described above is placing a pretty big burden on our least technically adept users. Even with the dangerous path we are looking at 30 lines of code out of 375 lines of production code being boilerplate. That's almost 10%.

This sort of thing is also why I always bring up "have you thought about how it will work in UI rules?" and why I get so frustrated when the answer is invariably "don't know. haven't thought about it. probably but that's not my focus." These kind of choices impose a pretty big burden on the OH users who are least able to bear it.

If one is writing rules in the UI one is most likely not an advanced coder. What one is doing is relatively simple and the likelihood they'll ever need to import a third party library is pretty small. So why impose a 10-20% burden on these users if we can avoid it?

@jpg0
Copy link
Contributor

jpg0 commented Oct 1, 2021

An area which is maybe an extreme version of your example is JS transformations (defined inline, not in a distinct file). My belief is that for many of these, users just want a simple line of JS, such as input.name.toUppercase() or something which relies on openHAB such as itemRegistry.getItem(input).getTag('foo') or whatever. This seems like an extreme case of needing something as concise as possible, where we really only want a single line.

One thing that I would note is that with ES6 you can import everything you need on a single line: let { thing1, thing2, ... thingN} = require('my_library'). You cannot do that with ES5, so I would hope that general imports are vastly reduced. It would be interesting to see if you existing Nashorn scripts would grow or shrink with the current GraalVM version.

Maybe one option to help this would be to allow users to define a custom header for all their scripts in the UI? It could default with a load of things imported. They could add things to the list if they want, for all their scripts. I would just want to ensure that openHAB never updates the list itself (which it does today) as that can break existing scripts.

@rkoshak
Copy link
Author

rkoshak commented Oct 1, 2021

I would expect them to shrink quite a bit, though it would still be irksome. We will find out soon I suspect, especially if Yannick's proposal to limit rule templates to GraalVM JS comes to pass.

Even though I focused on lines of code, that's only part of the problem though. The users also need to know what to import in the first place. That too is a burden, especially for the less skilled users. It can be greatly alleviated with a Helper Library but without it one needs to know of and how to use the Java and OH Javadocs just to know what to import and from where and how to use them.

One thing that I would note is that with ES6 you can import everything you need on a single line

One question out of ignorance. That imports the OH Java classes we might need, JS libraries, or both? Could I use that same line to import say java.time.ZonedDateTime as well as my personal metadta.js library in $OH_CONF/automation/blah/blah/blah?

I like any approach that makes creating rules in the UI better for the less technical users.

@jpg0
Copy link
Contributor

jpg0 commented Oct 1, 2021

To answer your last question, it allows importing any symbols that have been exported by the JS library that is being referenced. It does not allow importing Java classes directly (that could not be part of standard JS), however there is nothing preventing the JS library exporting Java classes for the script to import. It would probably even be possible to create a library allow importation of arbitrary java classes, which could be used like: let { java_time_ZonedDateTime } = require('js-library-to-access-java-classes').

Note that this style of import requires the name of the thing being imported, and in this case a . is not part of a valid JS identifier (hence my switch to _, which is probably not safe as Java also allows it in class names, but I'm sure there would be another option). It is possible to rename the local reference though - let { zonedDateTime:java_time_ZonedDateTime } works.

@ghys
Copy link
Member

ghys commented Oct 1, 2021

When making openhab/openhab-webui#1170 I found that Java.type() still works in GraalVM like in Nashorn to import Java types.

i.e.

image
translates to:

if (typeof(require) === "function") {
  ctx = this;
  var runtime = require("@runtime");
  itemRegistry = runtime.itemRegistry;
  events = runtime.events;
}

function mathRandomInt(a, b) {
  if (a > b) {
    // Swap a and b to ensure a is smaller.
    var c = a;
    a = b;
    b = c;
  }
  return Math.floor(Math.random() * (b - a + 1) + a);
}

var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID);


events.sendCommand('NewItem', (String(mathRandomInt(1, 100))));
logger.info(itemRegistry.getItem('NewItem').getState());

...and the logging works.

@rkoshak
Copy link
Author

rkoshak commented Oct 1, 2021

So it's looking like my original assertion that an automation add-on without corresponding helper libraries is going to be pretty unusable for many OH users still. The average OH rules developer shouldn't have to know that ZonedDateTime needs to be imported one way and personal library handled a completely different way. They shouldn't have to spend time looking through the JavaSE Javadocs and the openHAB Javadocs to know what needs to be imported from where and then figure out how.

I know this complaint is bigger than this particular issue but my biggest concern with Jython, Nashorn JavaScript, GraalVM JavaScript and all the rest is that we require the end users to know and understand far too much about differences in types (just today I had to explain why someone can't just multiple a Number Item's state by 1000 because the state is a java.lang.Number but 1000 is a Python primitive int). They have to know far too much about openHAB internals and they have to pay close attention to know when they are working with a Java Object or a language native entity. Helper libraries can greatly help with these but, as I said, those are all outside of the OH project.

Take creating a Timer for example. First I have to find where createTimer is defined. OK a search of the openHAB Javadocs shows its in org.openhab.core.model.script.actions.ScriptExecution. Now I need to know that I have to Java.type that class to bring it into scope so I can use it. But createTimer requires a ZonedDateTime. So most of the time (but not always) I'll have to also do a Java.type on java.lang.ZonedDateTime so I can get access to now. That's already a good deal of research and two "imports" required just to create a Timer. Of course a Helper Library would be able to hide that. You could even pass it something like "2h3m5s" as the "time" and the helper library can handle all that conversion.

That's what's missing and what I think is absolutely required to make these languages usable. Without them it's just too hard. You have to be a pretty experienced programmer to get anywhere or be a pretty lucky cargo-cult-programmer.

So I wonder if the discussion about imports and such is heading down the wrong path. Maybe we need to focus on making these helper libraries a part of the OH project itself and making them as easy to install as an add-on, or even better make them a part of the automation add-ons themselves. A whole lot of these problems can be handled in the helper libraries.

@digitaldan
Copy link
Contributor

So I wonder if the discussion about imports and such is heading down the wrong path. Maybe we need to focus on making these helper libraries a part of the OH project itself and making them as easy to install as an add-on, or even better make them a part of the automation add-ons themselves. A whole lot of these problems can be handled in the helper libraries.

I completely agree with this, our focus should be "What's the best experience for the majority of users?" Right now, this is not an ideal solution for the vast majority, which is a shame, as i think this could be one of our best features!

My fear is that bifurcating script runtimes will mean that we have two 'flavours' of script which will lead to greater confusion in the future when users share their scripts etc. From many years of development experience I firmly believe that too much 'magic' always bites you in the end.

So i would agree more choices is not desirable here, my suggestion was under the assumption that there is a use case for having a runtime without global namespace pollution . But i believe this only benefits a minority of users, so to quote Rich, if i were king for a day, there would be one javascript binding, and that would have sane global objects that would allow me to access runtime features like logging, items, etc... Without this, the GraalVM binding is not useable, even to experienced users.

Could this conversation not pivot to what that global injection namespace looks like? Could we not restrict this to a few common ones that would stay constant, but whose internal functions may grow over time (like a runtime object) ?

I am fully aware many here have vast previous experience discussing this, and i am not suggesting anything new or revolutionary, but i'm also feeling like we have been stuck for awhile in this situation, and I for one would really, really like to start using this binding.

@jpg0
Copy link
Contributor

jpg0 commented Oct 2, 2021

So it's looking like my original assertion that an automation add-on without corresponding helper libraries is going to be pretty unusable for many OH users still.

Interestingly, when I first started looking at using JS with openHAB, this was a point of contention between myself and Scott (at the time). I did agree with him that we should push most functionality into Java, however there are too many rough edges in the Java-to-other-language conversions that makes using Java objects directly fraught with challenges (often due to type issues). I do agree that we should have helper libraries for each language, and that these should be the primary interface to openHAB for script writers. I couldn't agree more that script writers should not have to know Java! Personally I have no problem with helper libraries diverging between languages, because this allows idiomatic usage of each language. As for whether these are bundled in the add-on, or installed via the package manager for the language in question, I don't really mind. I'm aware that this isn't a new discussion. (Oh, and I'm not sure about libraries being part of automation itself, as this would make updating them really hard and they would likely fall behind.)

Could this conversation not pivot to what that global injection namespace looks like? Could we not restrict this to a few common ones that would stay constant, but whose internal functions may grow over time (like a runtime object) ?

This is certainly something which has been raised before. However we could not use anything else that could clash with existing names, and I'm sure that runtime is widely used elsewhere. Typically languages use nasty things like __openhab_runtime__ for symbols like this, although I know that won't be popular! Note that even when injecting something with a non-clashing name, you are still going to cause problems for skilled JS developers because it will cause problems with their toolchains (linting, typescript, even auto-completion).

My view is that we can use a combination of the all the discussed approaches. What I would love to see is:

  • Bundle the helper libraries so that they are always available and typically used for the primary interface to openHAB
  • Establish a convention on how they are typically referenced, I'd suggest something like openHABRuntime or even openHAB (need not be the same for each language)
  • Put this reference into a header definition for UI & Blockly rules (const openHAB = require('@runtime');) so that beginners don't even see or care about it
  • Modify the JS engine so that if it encounters a script which attempts to reference the above symbol (openHAB) without first importing it, to display an error message saying exactly what needs to be done (e.g. add to the top of the script).

My hope is that this should be able to serve both beginner and advanced users. I do believe that advanced users are worth supporting because I expect them to generate community libraries. For example, I have my system to calculate and adjust light temperatures throughout the day, for which I use chroma.js, and it would be great to make this available to the community. (And maybe a supporting point is the lack of libraries for ES5 because the dev experience is so poor.)

the GraalVM binding is not useable, even to experienced users

I agree with this! Although maybe not for the same reasons - there are bugs, a broken directory layout and no docs, all of which need fixing before I would declare it ready. (I am confident in it's stability and capability due to having using it extensively over more than a year.)

@ghys
Copy link
Member

ghys commented Oct 2, 2021

I think it's a sensible approach if we do the transition in phases and make proper announcements - something like:

  • in openHAB 3.2 (end 2021) the built-in Nashorn engine will expose all openHAB objects under openhab so you should modify your existing scripts and libraries so use e. g. openhab.events. sendCommand instead of events. sendCommand. The global objects will remain available but their use is deprecated. You are encouraged to try out the GraalVM script engine. The equivalent of openhab in the built-in engine must be explicitly imported.
  • in openHAB 3.3 (mid-2022) the global objects will be removed from the Nashorn built-in engine and your scripts and libraries that still use them will break.
  • in openHAB 2.4 (end 2022) the Nashorn script engine will be removed and you must install the GraalVM engine for JS Scripting (it is installed by default for new installations).

Also I think there could be a case for the new marketplace to distribute scripting libraries as add-ons, and even custom Blockly blocks made by the community (described declaratively, like UI widgets or rule templates - defining that could prove challenging but is not impossible). A dedicated developer tool in the UI would allow defining the block's structure and the code they generate, in YAML, like widgets.
These blocks would make Blockly have even more an "educational" value to less experienced users, as they would be able to learn from the code that was generated and have a better sense of how things are done.

@J-N-K
Copy link
Member

J-N-K commented Oct 2, 2021

The suggested timeline ties openHAB to Java 11 for nearly 1.5 years, since Nashorn was removed in Java 15.

@lewie
Copy link

lewie commented Oct 3, 2021

I'm essentially still scratching around with my 3 year old very rudimentary scripts that I handed over to the openhab-helper-libraries back in the day. Since then I'm waiting for the firm integration of the really incredible great work you've done with GraalVM in openHab. A few times I set up a test system with it and I had tears in my eyes, when I finally will be able to use these things in production systems. Disappointed I am back to my old systems from many years ago.

Unfortunately I and many people can't use this ingenious GraalVM work of yours, because the basic architecture of the integration in openHab is still in flux after now three years and even the contradictory documentation drives me crazy - and I'm tough. For an own clean implementation my dusty Java architecture knowledge is not enough.

Get rid of the nashorn-engine and agree on fixed rules for the connection and include the GraalVM as standard. The resentment of some users will surely come, be limited and quickly subside. The gratitude of the newcomers should outweigh by orders of magnitude.

The once rewrite of no matter how many scripts is not the problem! I think it's not just me. The horror is the constant adjustment with several systems because constantly something changes or something is missing or there it is missing, then it is again outdated in the next version, because nashorn this or that etc..

Shorten the changeover. Finally make a cut with the old nashorn-engine, please. Preferably today and here.

Yes, I know, that is much too heretical... ;-)
Or isn't it?

Please do not be angry at my so clear words.... I hope I can dedicate myself as soon as possible again much more with the further development of openHab and make a contribution. But life is sometimes curious.

@digitaldan
Copy link
Contributor

in openHAB 3.2 (end 2021) the built-in Nashorn engine will expose all openHAB objects under openhab

Agreed , this sounds very reasonable and what i was thinking as well.

in openHAB 3.3 (mid-2022) the global objects will be removed from the Nashorn built-in engine

I don't think we have to go this far, i would rather us keep it there until we decide Nashorn is no longer included , I know there will be some migration woes as people need to change things like events.sendCommand to openhab.events.sendCommand when we remove Nashorn, , but i think this is minor inconvenience for the very large benefit of moving to GraalVM, and as @J-N-K mentions, we may want to get off Nashorn sooner and not be stuck with a old version of Java (again ). We have pretty long release cycles, so i feel we do a better job than most at communicating breaking changes.

I also want to clarify something , there was a mention of only injecting when using the openHAB UI? I'm not clear if thats actually the intention, but If that is the case, i think that would be a huge mistake. For one, it means one could not freely copy scripts to and from the UI to files, leading to all sorts of confusion. Also, while i don't think injecting a global object to interact with openHAb as "magic" by any means, doing so when only using the OH UI would be magic, as i would have a hard time explaining why that choice was made . I think we need our engine to behave one way, consistently, no matter how you configure openHAB (UI, files, or both). If this was not the intent, then please disregard this whole paragraph :-)

Not to jump the gun here, but if we can agree on using openhab as "the" global namespace object, can we move forward defining what this objects holds (event, item registry, logging, etc...) ?

@jpg0
Copy link
Contributor

jpg0 commented Oct 4, 2021

I also want to clarify something , there was a mention of only injecting when using the openHAB UI?

My suggestion is to never inject, rather to allow the UI to provide an explicit header which declares all the things that a script writer is likely to need. When viewing the script in the UI, this should be present.

Something that I'd also like to point out regarding a 'global namespace object': When I was porting much existing JS to the new engine (I think much written by @lewie - thanks very much), I found it much easier to not use a global object. The porting (or maybe: updating) process goes as follows:

  • try to run the code, get a " is not declared" error in file
  • import at the top of file
  • repeat

This did not require going to each reference of an openHAB object and modifying it to reference via some namespace. This could absolutely be done, but it's simpler to not do it (and leaves more concise code) because you don't need to find each reference.

@digitaldan
Copy link
Contributor

Strange, i'm playing with it now to see if i can reproduce

This is the rule that complained, many minuted after the reboot about require.

Hmm, you have a start level of 20 as a trigger, so i would assume it should have started earlier and complained? I agree both errors smell like Nashorn is handling the scripts. I'm not entirely clear how or when JSScripting becomes the default provider for JS scripts, i'm assuming its when its loaded (in osgi) and registers itself as a handler for the js mime type, but i guess that means Nashorn could have a window of opportunity to load scripts first on bootup?

I cannot yet reproduce on my system (i also added a start level 20 rule), not sure if its a race condition ?

@jpg0
Copy link
Contributor

jpg0 commented Dec 7, 2021

I have seen this too, however only when installing the new JS plugin, not once it's already installed and the system is started. However I have no rules at startlevel 20, although I did at startlevel 25.

@BobSilent
Copy link

BobSilent commented Dec 7, 2021

Hi,

First of all, I would like to express my big compliments and thank you for your work here.
The questions you ask and the answers you give go exactly in the right direction, in my opinion!
I've been an attentive reader here for quite a while.

Some background to not blow up this here

I've been there since OH2, but haven't had any rules so far, the first rules I wanted to make I kind of didn't like it. To cut a long story short, with ES2021 I wanted to start with rules, because I liked that best and what I know best from my dotnet world (or my Java times), for example.
Unfortunately, I couldn't really use that with OH3, or I was doing too stupidly.
With your work, however, everything I've planned has succeeded, just as I've imagined so far. Currently only as UI scripts, but I want to save code files.

I have update now to this addon version #2433 (comment) and also updated npm install openhab@latest

My questions

#log vs. Java.type("org.slf4j.LoggerFactory").getLogger.. vs. console
which one to use, when?

like @rkoshak I am using the logger, but i see differences:

var logger = log("org.openhab.ruleNew." + this.ruleUID);
var loggerOld = Java.type("org.slf4j.LoggerFactory").getLogger("org.openhab.rule." + this.ruleUID);
logger.warn("Hello world! {}", item.state);
loggerOld.warn("Hello world Old! {}", item.state);

Outputs in openhab-cli showlogs no special configuration, just defaults

2021-12-06 11:57:02.009 [WARN ] [pt.js.org.openhab.rulenew.49180d1df9] - Hello world! Off               [org.openhab.ruleNew.49180d1df9 at source <unknown>, line 53]
2021-12-06 11:57:02.014 [WARN ] [org.openhab.rule.49180d1df9         ] - Hello world Old! Off
logger.info("Hello world! {}", item.state);
loggerOld.info("Hello world Old! {}", item.state);

Outputs:

2021-12-06 12:01:16.768 [INFO ] [org.openhab.rule.49180d1df9         ] - Hello world Old! Off

so only the old logger output, not the new one.

Some remarks to the cache (#2433 (comment))
I think the documentation should be improved, as the cache is a global thing.
So in order to reduce naming clashes the example should show something like

let counter = cache.get(this.ruleUID + "counter");
if(counter == null){
     counter = {times: 0};
     cache.put(this.ruleUID + "counter", counter);
}
console.log("Count",counter.times++);

to prefix the key with the ruleUID, in order to make it rule unique. For me this is obvious but if someone follows the example and uses a common name which can lead to overwritten cache entries.
Personally I would create a global file with global cache names for me to share info across scripts.

@rkoshak
Copy link
Author

rkoshak commented Dec 7, 2021

I have update now to this addon version #2433 (comment) and also updated npm install openhab@latest

Install the version from a couple posts later and you don't need to install the helper library using NPM. You also don't need to import it in UI rules.

err, i think something is not right with the build i pointed you at, here's a local build

https://github.com/digitaldan/openhab-addons/releases/tag/jsscripting-OHJ

Do I have to do nmp -i openhab? Or can I remove it?

You can remove it if you install that later verison.

Are code files (.js) that I store also displayed in the UI or only the rules written in the UI?

All rules will be displayed in the UI. But if they are defined in .js files you will only be able to modify them in the .js file.

In Future I will store scripts in automation/js and my own shared code (libs)? in automation/lib/[javascript]/[personal]?

I believe personal libraries will go in automation/lib/js/personal. Libraries installed from npm will be installed to automation/lib/js/personal/node_modules.

I am still facing the var instead of let issue from GraalVM JavaScript replaces Nashorn, breaking Blockly and existing Nashorn rules #2433 (comment)

That one is probably going to take some time to fix. It requires a significant change in how UI rules are executed. For now you cannot use let or const in a Script Action or Script Condition in a UI rule.

#log vs. Java.type("org.slf4j.LoggerFactory").getLogger.. vs. console
which one to use, when?

I think console is not yet implemented. I recommend using log unless for some reason you want to use a different logger name than what is used when using log.

so only the old logger output, not the new one.

I think I mentioned this above in this thread. log doesn't log to a logger that is completely outside the configuration in log4j2.xml. You need to add a logger to that config to that file for script.js

               <Logger level="INFO" name="script.js"/>

to prefix the key with the ruleUID, in order to make it rule unique

One may not always want to make it unique to the rule. The cache will also be a way to share data (e.g. timers) between rules or between script actions and script conditions of the same rule. I don't have a problem with showing this as a way to make a key unique but I don't want to show it as the only way to define a key. Also, I suspect that, as with event, data passed from a calling rule and others, this.ruleUID will need to be encapsulated to avoid variable name conflicts.

@ghys
Copy link
Member

ghys commented Dec 7, 2021

With your handling of the MIME type in file based rules I would guess that all that would need to be done is to add an option in the UI to show GraalVM separately from Nashorn instead of the one generic "application/javascript". @ghys, do you have any comments? Is that feasible or problematic?

If it's done right and the types differ AFAIK there is nothing to do in the UI.

Correct. I set the temporary mime-type to x-application/javascript.new.

Apparently using x-* has been discouraged for some time. (https://en.wikipedia.org/wiki/Media_type#Unregistered_tree).
For our own media types we use the vendor tree (https://en.wikipedia.org/wiki/Media_type#Vendor_tree) i.e. application/vnd.openhab.* (but would still have to register them eventually here: https://www.iana.org/assignments/media-types/media-types.xhtml).

AFAIK parameters don't need to be registered so it might make sense to use those to differ between GraalVM JS and Nashorn JS (as in, application/javascript remains the "default" runtime, which for now means Nashorn, and GraalVM would use something like application/javascript;version=GraalVM until Nashorn is removed and it becomes the only engine, at which point it could handle application/javascript again).

@rkoshak
Copy link
Author

rkoshak commented Dec 7, 2021

I found another difference in behavior. If a timer expires and inside the lambda executed by the timer that timer is cancelled an error is generated with a Thread was interrupted exception. I'm able to work around it by checking to see if the timer is running in addition to whether it's active or hasn't terminated before trying to cancel it, though that assumes that the timer can clean up after itself if necessary.

It's not clear if this is JSScripting specific or not but given it's a PolyglotExcepion I'd guess it is.

2021-12-07 07:56:13.534 [WARN ] [ore.internal.scheduler.SchedulerImpl] - Scheduled job failed and stopped                                                                              
org.graalvm.polyglot.PolyglotException: Thread was interrupted.                                                                                                                        
        at <js>.TimerMgr.cancelAll(<eval>:104) ~[?:?]                                                                                                                                  
        at <js>.:anonymous(<eval>:265) ~[?:?]                                                                                                                                          
        at <js>.TimerMgr._notFlapping(<eval>:46) ~[?:?]                                                                                                                                
        at <js>.timer(<eval>:83) ~[?:?]                                                                                                                                                
        at com.oracle.truffle.polyglot.PolyglotFunctionProxyHandler.invoke(PolyglotFunctionProxyHandler.java:154) ~[bundleFile:?]                                                      
        at com.sun.proxy.$Proxy512.apply(Unknown Source) ~[?:?]                                                                                                                        
        at org.openhab.core.model.script.actions.ScriptExecution.lambda$1(ScriptExecution.java:100) ~[bundleFile:?]                                                                    
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$12(SchedulerImpl.java:184) ~[?:?]                                                                                  
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$1(SchedulerImpl.java:87) ~[?:?]                                                                                    
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]                                                                                               
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]                                                                                                              
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]                                                        
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]                                                                                       
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]                                                                                       
        at java.lang.Thread.run(Thread.java:829) [?:?]

@rkoshak
Copy link
Author

rkoshak commented Dec 7, 2021

To provide a bit more info I changed the start level for the rule I posted above to 100 and restarted openHAB. The following is what I see in the logs during startup (redacted to just show rules related stuff.

2021-12-07 08:35:41.563 [WARN ] [automation.internal.RuleRegistryImpl] - Cannot find reference for ${  } , it will remain the same.
2021-12-07 08:35:49.674 [INFO ] [e.automation.internal.RuleEngineImpl] - Rule engine started.
2021-12-07 08:35:54.214 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.227 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.229 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.265 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:54.303 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.306 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.454 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:54.585 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:56.496 [INFO ] [script.Rules.rules_tools.MQTT_Online] - Publishing ONLINE to LWT topic openhab/status
2021-12-07 08:35:57.667 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:57.743 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:57.749 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:57.752 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:57.755 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:58.100 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:58.215 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:58.217 [WARN ] [hab.model.script.rules_tools.MQTT_EB] - vDad_LR_Alarm does not exist!
2021-12-07 08:35:58.430 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:58.722 [INFO ] [model.script.rules_tools.Alarm Clock] - No alarm scheduled for AlarmClock: 2021-12-07T13:00:00.000+0000
2021-12-07 08:35:58.723 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'calculateElectricityBill' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:58.726 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'calculateElectricityBill' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:58.764 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_sm' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:58.819 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'calculateElectricityBill' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:58.893 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:58.953 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.012 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:59.018 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:59.030 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:59.034 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:59.036 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:59.081 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:59.111 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.367 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.398 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: TypeError: items.getItem is not a function in <eval> at line number 1
2021-12-07 08:35:59.432 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:59.459 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.472 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.476 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:35:59.723 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:35:59.858 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:36:00.236 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'dads_motion' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:36:00.240 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'dads_motion' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:36:01.166 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'tod_lights' failed: <eval>:11:18 Expected , but found =>
    .filter(light => light.state.toString() != st)
                  ^ in <eval> at line number 11 at column number 18
2021-12-07 08:36:01.747 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'christmas_lights' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:01.866 [ERROR] [ernal.handler.ScriptConditionHandler] - Script execution failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:36:02.296 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'zwave-alert' failed: ReferenceError: "require" is not defined in <eval> at line number 1
2021-12-07 08:36:03.535 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'thing_status_handler' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:05.001 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:08.542 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:10.558 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:18.614 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:30.536 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'thing_status_handler' failed: ReferenceError: "log" is not defined in <eval> at line number 1
2021-12-07 08:36:48.055 [WARN ] [.internal.OpenhabGraalJSScriptEngine] - Failed to retrieve script script dependency listener from engine bindings. Script dependency tracking will be disabled.
2021-12-07 08:36:48.952 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'humidifiers' failed: ReferenceError: "log" is not defined in <eval> at line number 1

From this point onward I get an error every time a rule runs. The rule I posted above that complained about require is tod_sm but pretty much all of my rules are generating an error.

Once I navigate to each rule and just click "Save" the errors go away.

The two things I notice are that it takes a bit before the errors start to list the rule's UID. At first it just errors without the reference. The second is that there are a couple of rules that work just fine.

@digitaldan
Copy link
Contributor

If a timer expires and inside the lambda executed by the timer that timer is cancelled an error is generated with a Thread was interrupted exception.

Out of curiosity, whats the use case from canceling a timer from inside its callback? Since timers are one shot deals, canceling after it's executed its callback would not seem to do anything?

I saw this error as well when implementing the setInterval support, which turns the timer into a recurring timer, but was I trying to reuse the same timer object and cancel future runs from happening, you can see that code here https://github.com/openhab/openhab-addons/blob/d60e0715e2fac5a2765835db860236a34b2c2af2/bundles/org.openhab.automation.jsscripting/src/main/resources/node_modules/%40jsscripting-globals.js#L173.

To provide a bit more info I changed the start level for the rule I posted above to 100 and restarted openHAB. The following is what I see in the logs during startup (redacted to just show rules related stuff.

This is so odd, i'm not sure why your system is behaving so differently. Can you check to make sure there are not multiple versions of the bundle loaded ?

@jpg0
Copy link
Contributor

jpg0 commented Dec 7, 2021

AFAIK parameters don't need to be registered so it might make sense to use those to differ between GraalVM JS and Nashorn JS (as in, application/javascript remains the "default" runtime, which for now means Nashorn, and GraalVM would use something like application/javascript;version=GraalVM until Nashorn is removed and it becomes the only engine, at which point it could handle application/javascript again).

Thanks! I was thinking that noone ever saw it so didn't put much thought into it, but then realised that it will show up in the UI. I think that the best option would be to use: application/javascript;version=ECMAScript-2021 as this matches the version that is actually supported.

@jpg0
Copy link
Contributor

jpg0 commented Dec 7, 2021

I believe personal libraries will go in automation/lib/js/personal. Libraries installed from npm will be installed to automation/lib/js/personal/node_modules.

@rkoshak the current proposal is that all modules will go into automation/js/node_modules. My most recent PR enables this. JS does not typically allow custom module directories, nor multiple library roots.

@jpg0
Copy link
Contributor

jpg0 commented Dec 7, 2021

@digitaldan you may want to take a look ScriptEngineManagerImpl - could it be building Nashorn engines for each script, then caching them before Graal gets loaded? I have no idea how UI scripts are loaded, but with the filesystem:

  • the watcher itself starts at level 20
  • scripts by default run at level 50, unless explicitly overridden per-script

@rkoshak
Copy link
Author

rkoshak commented Dec 7, 2021

Out of curiosity, whats the use case from canceling a timer from inside its callback? Since timers are one shot deals, canceling after it's executed its callback would not seem to do anything?

In this case it's a bit of an M. C. Escher self referential situation. In the time of day rule above I'm using TimerMgr which handles all the book keeping associated with Timers based on the states of a bunch of DateTime Items. That rule also gets triggered when ever one of those DateTime Items change state. These DateTime Items might change state in mass around midnight causing the rule to be triggered a bunch of times around midnight.

So I schedule a little debounce timer in that rule so it doesn't actually do anything until a minute after the last change to the DateTime Items. And since I have TimerMgr there, I use TimerMgr to schedule and manage that timer too.

The problem comes in because the first thing I need to do is cancel any already existing Timers before creating new ones. So in the body of the debounce Timer I call cancelAll() on the TimerMgr. But that means that I'll also be canceling the debounce timer that is currently running and calling cancelAll() too.

Ultimately the use case was that I was lazy and should probably have implemented the debounce timer separate from the DateTime timers. But it's kind of the same situation you describe when you saw the error too.

I don't think this will be a general error a lot of people will see but I have seen a large number of users (often beginners) try to cancel a Timer inside a Timer. Until now that's not caused a problem.

@rkoshak the current proposal is that all modules will go into automation/js/node_modules. My most recent PR enables this. JS does not typically allow custom module directories, nor multiple library roots.

As long as it's documented I don't really care where we need to put stuff. I was just going on the instructions for how to install the helper library with npm prior to them being included in the bundle.. I've always thought the automation folder was kind of a mess the way it's organized and never understood why the scripts and modules were kept in widely separate folders under automation. It always made sense to me that each language would have it's own folder under automation (or maybe we don't even need automation at all) and how it's organized after that be language specific. But when I brought that up way back when I was told it was too late. I'm happy to see us getting back to that.

could it be building Nashorn engines for each script, then caching them before Graal gets loaded? I have no idea how UI scripts are loaded

Just based solely on the start levels shown in the UI, the rules are loaded at level 40. But 40 is the earliest system trigger we can choose so one wonders what's executing them between runlevel 40 and 50 if the rule engine isn't started until level 50.

Can you check to make sure there are not multiple versions of the bundle loaded ?

I only see the one JSScripting bundle installed and active.

327 │ Active │  80 │ 3.2.0.202112052348    │ openHAB Add-ons :: Bundles :: Automation :: JSScripting

@digitaldan
Copy link
Contributor

@jpg0 so i have your changes in core and the binding installed, script files work great (i left some comments on the PR about creating the script dirs on activate). The problem now is that the Graal Script engine does not seem to be registering as a script action module type which the UI pulls from /rest/module-types?type=action, so the UI does not give the option to use it when adding scripts. I'm trying to track down how script engines register for this, i assumed it was done via mime type , but something is not working.

The rest endpoint i mentioned contains this in regards to the current script engines:

{
        "inputs": [],
        "outputs": [
            {
                "name": "result",
                "type": "java.lang.Object",
                "label": "result",
                "description": "the script result"
            }
        ],
        "uid": "script.ScriptAction",
        "visibility": "VISIBLE",
        "tags": [],
        "label": "execute a given script",
        "description": "Allows the execution of a user-defined script.",
        "configDescriptions": [
            {
                "description": "the scripting language used",
                "label": "Script Type",
                "name": "type",
                "required": true,
                "type": "TEXT",
                "readOnly": false,
                "multiple": false,
                "advanced": false,
                "verify": false,
                "limitToOptions": true,
                "options": [
                    {
                        "label": "ECMAScript (ECMAScript 262 Edition 11)",
                        "value": "application/javascript"
                    },
                    {
                        "label": "Rule DSL (v1)",
                        "value": "application/vnd.openhab.dsl.rule"
                    }
                ],
                "filterCriteria": []
            }

@rkoshak
Copy link
Author

rkoshak commented Dec 7, 2021

I ran into a problem with interacting with Item metadata. We need to pull the MetadataRegistry from OSGI. I submitted a PR with the fix (it's a one liner).

openhab/openhab-js#25

@digitaldan
Copy link
Contributor

digitaldan commented Dec 7, 2021

So I was not correct with my last comment, JSSCripting looks loaded in the UI, but nashorn is not, also Nashorn is still executing UI rules regardless. So will investigate later tonight .

@digitaldan
Copy link
Contributor

@jpg0 does this look like the core is using what the actual script engine reports on mime types it supports VS what we advertise in the GraalJSScriptEngineFactory ? So our custom mime type is not being used?

@digitaldan
Copy link
Contributor

Yep, if i change that to use the openHAB ScriptEngineFactory instead, it works!

image

@digitaldan
Copy link
Contributor

digitaldan commented Dec 8, 2021

See #2595 for a fix for 👆

@jpg0
Copy link
Contributor

jpg0 commented Dec 8, 2021

@digitaldan great, thanks!

@digitaldan
Copy link
Contributor

@rkoshak i think the changes in

Will solve the startup problem with scripts that you are seeing as GraalVM scripts in the UI will be paired to the new mime type.

@rkoshak
Copy link
Author

rkoshak commented Dec 9, 2021

This is just a little note to mention that I've made all of my rule templates posted to the Marketplace compatible with both Nashorn and JSScripting. Eventually I'll remove support for Nashorn so I can take advantage of the Helper Library in JSScripting but in the mean time it should help to have them work with both during the transition time.

In general it's mostly just a case of adding if(typeof(require) === "function") Object.assign(this, require('@runtime')); to the top of the scripts.

However, checking types works differently between the two. In Nashorn I can use if(event.itemState.class == UnDefType.class) but in JSScripting I have to use if(event.itemState.class == UnDefType). So I end up using something like:

var undefType = (typeof(require) === "function") ? UnDefType : UnDefType.class;
if(event.itemState == undefType){
  ...

One other gotcha is Nashorn is a little nicer about being able to automatically convert States and Commands to Strings when calling sendCommand/postUpdate but JSScripting flat out refuses. So there were cases I was lazy that needed to be updated to explicitly use the String.

Finally, there is the timer cancel issue that I brought up earlier that probably won't impact anyone else but me which is easy enough to fix by checking for both hasTerminated as well as isRunning.

Once the three PRs are merged and the startup stuff seems to be fixed and the docs are beefed up a bit my plan is to mess around with the edges of the helper library looking for errors (e.g. creating Items, creating Rules from UI Rules (I have a use case for this one actually in a rule template), etc.). Then I think I'll propose adding some of my rules_tools libraries to the Helper Library. I agree it would be better to make them a part of core and plan on doing that eventually. But in the mean time I think they are useful enough to greatly aid users now and would rather get them to users right away instead of making them wait until I have the time to figure out how to make them part of core. In my rules at least, I hardly have a rule that doesn't use TimerMgr, RateLimit, Gatekeeper, or Delay and using them saves dozens of lines of code per rule, sometimes more.

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/oh-3-examples-writing-and-using-javascript-libraries-in-mainui-created-rules/108526/56

@rkoshak
Copy link
Author

rkoshak commented Dec 20, 2021

I'm inclined to close this issue now with the 3.2 release. However, I just want to mention that if you forget to remove the old add-on jar file from add-ons prior to upgrading to OH 3.2, some weirdness will occur, including that it will only show the new ECMA as an option in the UI. Clearing the cache seemed to do the trick.

But with the PR that makes it so we can run Nashorn and JSScripting side-by-side I think we can call this issue fixed.

Thanks a lot for all the wonderful work! The future looks bright!

@rkoshak rkoshak closed this as completed Dec 20, 2021
@digitaldan
Copy link
Contributor

Yes, thanks everyone, it was awesome that we were able to move quickly and get this into 3.2!

@lewie
Copy link

lewie commented Dec 21, 2021

https://www.heise.de/news/Smart-Home-openHAB-3-2-erweitert-die-JavaScript-Integration-6301578.html

I would like to thank you very much for this great work!!!!
🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄
🎄 And wish you a Merry Christmas.🎄
🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄🎄

@jpg0
Copy link
Contributor

jpg0 commented Dec 21, 2021

Special thanks to you @digitaldan; things moved very quickly with your help!

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/ecmascript-edition-11-error-no-globals-like-items-populated/130532/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests