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

NullPointerException loading NPM module #587

Open
overfullstack opened this issue Apr 10, 2022 · 10 comments
Open

NullPointerException loading NPM module #587

overfullstack opened this issue Apr 10, 2022 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@overfullstack
Copy link

Hi, I am trying to load n NPM module on a Stock JDK (openjdk_17.0.1_17.30.16_x64) with graal dependencies, following this documentation

val graalVersion = "22.0.0.2"
implementation("org.graalvm.sdk:graal-sdk:$graalVersion")
implementation("org.graalvm.js:js:$graalVersion")
fun main() {
  val options: MutableMap<String, String> = HashMap()
  options["js.commonjs-require"] = "true"
  options["js.commonjs-require-cwd"] = "graal-js"
  //options["js.commonjs-global-properties"] = "./globals.js"
  options["js.commonjs-core-modules-replacements"] =
    "buffer:buffer/, path:path-browserify, crypto:crypto-js, uuid:uuid-browser"
  val cx: Context = Context.newBuilder("js")
    .allowExperimentalOptions(true)
    .allowIO(true)
    .options(options)
    .build()
  cx.eval("js", "const newman = require('newman');")
}

Github link for a runnable program

I get this exception (Complete log attached), I could infer this is due to the uuid-browser module. This is the javascript line where I get this exception.
if ((i & 0x03) === 0) r = Math.random() * 0x100000000;
what am I missing or is this a bug in graaljs?

Exception in thread "main" org.graalvm.polyglot.PolyglotException: java.lang.NullPointerException: Cannot invoke "java.util.SplittableRandom.nextDouble
[log.log](https://github.com/oracle/graaljs/files/8458779/log.log)
()" because the return value of "com.oracle.truffle.js.runtime.JSRealm.getRandom()" is null
	at com.oracle.truffle.js.builtins.math.RandomNode.random(RandomNode.java:56)
	at com.oracle.truffle.js.builtins.math.RandomNodeGen.execute(RandomNodeGen.java:28)
	at com.oracle.truffle.js.nodes.function.FunctionRootNode.executeInRealm(FunctionRootNode.java:143)
	at com.oracle.truffle.js.runtime.JavaScriptRealmBoundaryRootNode.execute(JavaScriptRealmBoundaryRootNode.java:92)
@iamstolis
Copy link
Member

Thank you for the report. Yes, this is a bug in graal-js. The experimental CommonJS Modules support attempts to load the replacement modules eagerly and too early (in the middle of the initialization of the context) - before the SplittableRandom (that is used for Math.random() implementation) is initialized.

@iamstolis iamstolis added the bug Something isn't working label Apr 10, 2022
@iamstolis
Copy link
Member

@eleinadani while this particular issue can be fixed easily by moving initTimeOffsetAndRandom() in JSRealm.initialize(), I find it unfortunate and error-prone that these modules are loaded that early (i.e. in the middle of the context initialization). Can't we load these modules later? Do they even have to be loaded eagerly?

@overfullstack
Copy link
Author

overfullstack commented Apr 10, 2022

@iamstolis Thanks for responding. I don't have such requirement to load them eagerly, is there a work around I can follow to get this resolved?

@iamstolis
Copy link
Member

is there a work around I can follow to get this resolved?

Your test-case would not work even without the mentioned bug. js.commonjs-core-modules-replacements option (as its name suggests) works for Node.js core modules only and uuid is not Node.js core module (unlike buffer, path or crypto). It is (another) bug that the value of this option is not verified properly.

@overfullstack
Copy link
Author

@iamstolis There is another problem if I remove the uuid:uuid-browser mapping. crypto is not being replaced appropriately. Here is the exception I get without uuid replacement:

Exception in thread "main" TypeError: Cannot load CommonJS module: 'crypto'
	at <js> :anonymous(graal-js/node_modules/uuid/dist/rng.js:8:202-218)
	at <js> :anonymous(graal-js/node_modules/uuid/dist/v1.js:8:202-220)
	at <js> :anonymous(graal-js/node_modules/uuid/dist/index.js:61:1225-1242)
	at <js> :anonymous(graal-js/node_modules/newman/node_modules/postman-collection/lib/collection/property.js:2:107-121)
	at <js> :anonymous(graal-js/node_modules/newman/node_modules/postman-collection/lib/collection/certificate.js:2:111-131)
	at org.graalvm.polyglot.Context.eval(Context.java:425)
	at NewmanKt.main(Newman.kt:15)
	at NewmanKt.main(Newman.kt)

The line in rng.js which throws this exception is this: var _crypto = _interopRequireDefault(require("crypto"));

@overfullstack
Copy link
Author

@iamstolis I also tried using this replacement crypto:crypto-browserify, but I get this exception at the line module.exports = require('crypto').randomBytes.
I see the same issue with crypto replacement not being proper.

Exception in thread "main" TypeError: Cannot load CommonJS module: 'crypto'
	at <js> :anonymous(graal-js/node_modules/randombytes/index.js:1:78-94)
	at <js> :anonymous(graal-js/node_modules/crypto-browserify/index.js:3:154-175)
	at org.graalvm.polyglot.Context.eval(Context.java:425)
	at NewmanKt.main(Newman.kt:15)
	at NewmanKt.main(Newman.kt)

@overfullstack
Copy link
Author

overfullstack commented Apr 12, 2022

@eleinadani Please do let me know if you need me to create a separate issue regarding the crypto replacement bug (If you see it's a bug). Also, may I please ask if there is any tentative timeline for the fix. Sorry if this feels like I am rushing, but I love this feature and wish to use this solution in our upcoming hackathon on April 20th as a POC for a project. Thanks!

@eleinadani
Copy link
Contributor

@overfullstack we are working on a fix for the NPE that you have reported here, and will merge it in the upcoming days. Once that is merged we will have a look at crypto but cannot guarantee that any fix will be merged before April 20th.

@eleinadani
Copy link
Contributor

eleinadani commented Apr 14, 2022

We just merged a fix for the NPE in this commit. Please let us know if the fix works for you.

I also had a look at what it would take to use 'crypto-browserify' in Graal.js with commonjs-require. It turns out that the module is quite old (last commit 2018) and it internally depends on some browser-specific APIs. For example, it imports a module named 'randombytes' which either uses Node.js 'crypto' (by default) or the Browser's builtin crypto.getRandomValues (when bundled). Given the usage of such APIs, it looks like the module cannot run on Graal.js out of the box (even with commonjs-require), and some patching would be needed. For example, I guess that providing a mockup implementation of crypto.getRandomValues could be enough to run 'randombytes' (but haven't tried).

Not all APIs of 'crypto-browserify' depend on built-in Browser or Node APIs. For example, the 'createHmac()' function can be used in Graal.js with the following configuration (on the latest Graal.js):

// Using the bin/js launcher, but the same options will work with context.newBuilder()...context.eval()...
$GRAALVM/bin/js --jvm 
	--experimental-options 
	--js.global-property=true
	--js.commonjs-require=true
	--js.commonjs-require-cwd=.
	--js.commonjs-core-modules-replacements=
		crypto:crypto-browserify,
		buffer:buffer/,
		stream:stream-browserify,
		events:events/,
		util:util/,
		create-hmac:create-hmac/browser.js

	-e "global.process = {env:{}}; var x = require('crypto').createHmac('sha256', 'foo').digest('hex'); console.log(x);"

683716d9d7f82eed174c6caebe086ee93376c79d7c61dd670ea00f7f8d6eb0a8

Note that 'create-hmac:create-hmac/browser.js' is used to instruct Graal.js that require('create-hmac') should load the browser.js file (instead of the default index.js). 'create-hmac' is a dependency of 'crypto-browserify' that unfortunately by default uses Node.js' modules, but provides a Browser-ready version in browser.js, which is used by package bundlers when creating a "browserified" version. Also note that I had to provide a number of other module replacements (and npm install them beforehand), and that I had to provide a basic global mockup for the process object.

@EternityGH
Copy link

@overfullstack
Have you fixed this issue yet? Can you help me? I'm facing a similar problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants