Investigate module loading issue

[kipz]

Shown in below commit. Is fixed by removing circular references.

https://github.com/atomisthq/spring-team-handlers/commit/f402bd11a431ddf70a13de167150d4da56195b1f

✔ ~/scm/mackinac/spring-team-handlers/.atomist [master|✚ 1] 09:58 $ rug test -X Resolving dependencies for atomist:spring-team-handlers (0.1.113·local) completed Compiling source files of atomist:spring-team-handlers (0.1.113·local) completed Loading rugs of atomist:spring-team-handlers (0.1.113·local) failed

com.atomist.rug.RugJavaScriptException: Error during eval of: .atomist/handlers/event/fingerprint/spring/SpringRestMvcDistiller.js at com.atomist.rug.runtime.js.JavaScriptContext.evaluate(JavaScriptContext.scala:89) at com.atomist.rug.runtime.js.JavaScriptContext$$anonfun$3.apply(JavaScriptContext.scala:81) at com.atomist.rug.runtime.js.JavaScriptContext$$anonfun$3.apply(JavaScriptContext.scala:81) at scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59) at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48) at com.atomist.rug.runtime.js.JavaScriptContext.(JavaScriptContext.scala:81) at com.atomist.project.archive.ArchiveRugResolver.com$atomist$project$archive$ArchiveRugResolver$$find(ArchiveRugResolver.scala:34) at com.atomist.project.archive.ArchiveRugResolver.(ArchiveRugResolver.scala:51) at com.atomist.rug.cli.command.AbstractCompilingAndOperationLoadingCommand.lambda$createRugResolver$1(AbstractCompilingAndOperationLoadingCommand.java:69) at com.atomist.rug.cli.output.ProgressReportingOperationRunner.run(ProgressReportingOperationRunner.java:22) at com.atomist.rug.cli.command.AbstractCompilingAndOperationLoadingCommand.createRugResolver(AbstractCompilingAndOperationLoadingCommand.java:67) at com.atomist.rug.cli.command.AbstractCompilingAndOperationLoadingCommand.run(AbstractCompilingAndOperationLoadingCommand.java:156) at com.atomist.rug.cli.command.AbstractCommand.run(AbstractCommand.java:36) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at com.atomist.rug.cli.command.ReflectiveCommandRunMethodRunner.invokeCommand(ReflectiveCommandRunMethodRunner.java:17) at com.atomist.rug.cli.command.ReflectiveCommandRunner.invokeReflectiveCommand(ReflectiveCommandRunner.java:92) at com.atomist.rug.cli.command.ReflectiveCommandRunner.invokeCommand(ReflectiveCommandRunner.java:261) at com.atomist.rug.cli.command.ReflectiveCommandRunner.runCommand(ReflectiveCommandRunner.java:59) at com.atomist.rug.cli.Runner.runCommand(Runner.java:93) at com.atomist.rug.cli.Runner.run(Runner.java:40) at com.atomist.rug.cli.Main.invokeRunner(Main.java:34) at com.atomist.rug.cli.Main.main(Main.java:25) Caused by: javax.script.ScriptException: TypeError: Concat_1.Concat is not a function in .atomist/microgrammar/Utils.js at line number 20 at jdk.nashorn.api.scripting.NashornScriptEngine.throwAsScriptException(NashornScriptEngine.java:467) at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:451) at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:403) at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:399) at jdk.nashorn.api.scripting.NashornScriptEngine.eval(NashornScriptEngine.java:155) at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264) at com.atomist.rug.runtime.js.JavaScriptContext.evaluate(JavaScriptContext.scala:94) ... 24 more Caused by: .atomist/microgrammar/Utils.js:20 TypeError: Concat_1.Concat is not a function at jdk.nashorn.internal.runtime.ECMAErrors.error(ECMAErrors.java:57) at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:213) at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:185) at jdk.nashorn.internal.runtime.ECMAErrors.typeError(ECMAErrors.java:172) at jdk.nashorn.internal.runtime.Undefined.lookup(Undefined.java:102) at jdk.nashorn.internal.runtime.linker.NashornLinker.getGuardedInvocation(NashornLinker.java:106) at jdk.nashorn.internal.runtime.linker.NashornLinker.getGuardedInvocation(NashornLinker.java:98) at jdk.internal.dynalink.support.CompositeTypeBasedGuardingDynamicLinker.getGuardedInvocation(CompositeTypeBasedGuardingDynamicLinker.java:176) at jdk.internal.dynalink.support.CompositeGuardingDynamicLinker.getGuardedInvocation(CompositeGuardingDynamicLinker.java:124) at jdk.internal.dynalink.support.LinkerServicesImpl.getGuardedInvocation(LinkerServicesImpl.java:154) at jdk.internal.dynalink.DynamicLinker.relink(DynamicLinker.java:253) at jdk.nashorn.internal.scripts.Script$Recompilation$576$412A$Utils.L:1$toMatchingLogic(.atomist/microgrammar/Utils.js:20) at jdk.nashorn.internal.scripts.Script$Recompilation$1000$251A$Ops.L:1$Opt$Opt(.atomist/microgrammar/Ops.js:6) at jdk.nashorn.internal.scripts.Script$Recompilation$998$60AAAAA$SpringRestMvcDistiller$cu1$restOf.L:1(.atomist/handlers/event/fingerprint/spring/SpringRestMvcDistiller.js:48) at jdk.nashorn.internal.runtime.ScriptFunctionData.invoke(ScriptFunctionData.java:647) at jdk.nashorn.internal.runtime.ScriptFunction.invoke(ScriptFunction.java:494) at jdk.nashorn.internal.runtime.ScriptRuntime.apply(ScriptRuntime.java:393) at jdk.nashorn.api.scripting.ScriptObjectMirror.call(ScriptObjectMirror.java:117) at com.coveo.nashorn_modules.Module.compileJavaScriptModule(Module.java:334) at com.coveo.nashorn_modules.Module.compileModuleAndPutInCache(Module.java:288) at com.coveo.nashorn_modules.Module.loadModuleAsFile(Module.java:202) at com.coveo.nashorn_modules.Module.attemptToLoadFromThisFolder(Module.java:177) at com.coveo.nashorn_modules.Module.require(Module.java:116) at jdk.nashorn.internal.scripts.Script$Recompilation$653$SpringRestMvcDistiller.:program(.atomist/handlers/event/fingerprint/spring/SpringRestMvcDistiller.js:1) at jdk.nashorn.internal.runtime.ScriptFunctionData.invoke(ScriptFunctionData.java:637) at jdk.nashorn.internal.runtime.ScriptFunction.invoke(ScriptFunction.java:494) at jdk.nashorn.internal.runtime.ScriptRuntime.apply(ScriptRuntime.java:393) at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:446) ... 29 more

[kipz]

I found this:

https://github.com/atomisthq/spring-team-handlers/blob/f402bd11a431ddf70a13de167150d4da56195b1f/.atomist/handlers/event/fingerprint/spring/SpringRestMvcDistiller.ts#L48

Which is calling functions during initialisation that are in modules that have circular references. If we change that line of code to be a function that is called when data are required instead, the problem goes away.

As far as I know, this is not supported in other JS runtimes either.

From node docs: https://nodejs.org/api/modules.html#modules_cycles

When main.js loads a.js, then a.js in turn loads b.js. At that point, b.js tries to load a.js. In order to > prevent an infinite loop, an unfinished copy of the a.js exports object is returned to the b.js module. b.js then finishes loading, and its exports object is provided to the a.js module.

This is how we solved the circular loading problem in our module loader too:

https://github.com/coveo/nashorn-commonjs-modules/pull/14

[johnsonr]

With this particular failure scenario, the unit tests still passed in node and involved loading this.

However, it's possible that the code was still incorrect and yet just happened to work in this case in node.

Given the node documentation you found here, I'm inclined to close this issue.

[kipz]

If the two modules with circular references are loaded before functions on them are called, then that code would work fine I guess.

So I maybe the node tests load the modules in a different order to the rug tests?

[johnsonr]

I suppose so. So it's incorrect but just happens to work for some reason in this case.