node icon indicating copy to clipboard operation
node copied to clipboard

ERR_INTERNAL_ASSERTION thrown when using --experimental-require-module and --import together

Open kylesmile opened this issue 1 year ago • 6 comments

Version

v22.7.0

Platform

Darwin Kyles-MBP-2.home 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:14:30 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

With two files test.mjs and test.cjs (both files can be empty):

node --experimental-require-module --import=./test.mjs test.cjs

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

No error should be thrown, and --experimental-require-module and --import can be used in combination. For example, to use module customization hooks when ECMAScript and CommonJS module types need to be mixed.

What do you see instead?

This error is thrown:

node:internal/assert:14
    throw new ERR_INTERNAL_ASSERTION(message);
          ^

Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

    at assert (node:internal/assert:14:11)
    at ModuleLoader.requireCommonJS (node:internal/modules/esm/translators:316:3)
    at callTranslator (node:internal/modules/esm/loader:428:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:434:30)
    at async link (node:internal/modules/esm/module_job:87:21) {
  code: 'ERR_INTERNAL_ASSERTION'
}

Additional information

No response

kylesmile avatar Aug 26 '24 21:08 kylesmile

$ touch repro.cjs  
$ touch test.mjs 
$ node --experimental-require-module --import=./test.mjs repro.cjs
(node:179265) ExperimentalWarning: Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
node:internal/assert:14
    throw new ERR_INTERNAL_ASSERTION(message);
          ^

Error [ERR_INTERNAL_ASSERTION]: This is caused by either a bug in Node.js or incorrect usage of Node.js internals.
Please open an issue with this stack trace at https://github.com/nodejs/node/issues

    at assert (node:internal/assert:14:11)
    at ModuleLoader.requireCommonJS (node:internal/modules/esm/translators:290:3)
    at callTranslator (node:internal/modules/esm/loader:436:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:442:30)
    at async ModuleJob._link (node:internal/modules/esm/module_job:106:19) {
  code: 'ERR_INTERNAL_ASSERTION'
}

Node.js v22.7.0

avivkeller avatar Aug 26 '24 21:08 avivkeller

I think part of my WIP for synchronous hooks can fix this by handling the translations more correctly (i.e. properly distinguishing require(esm) -> require(cjs) v.s. import cjs -> require(cjs) in the loader), it will need to be split out as a refactoring first though.

For example, to use module customization hooks when ECMAScript and CommonJS module types need to be mixed.

require(esm) from CJS roots won't support the asynchronous module customization hooks, just like require(cjs) from CJS roots / createRequire() doesn't support them either. The plan is to add an in-thread, synchronous variant of the customization hooks that support all types of module loading universally instead https://github.com/nodejs/loaders/pull/198 while the async variant will continue to be limited in the kind of module loading it can affect.

joyeecheung avatar Aug 27 '24 13:08 joyeecheung

require(esm) from CJS roots won't support the asynchronous module customization hooks

Good point. I would probably have run into that next, if I hadn't gotten this error.

For my case, I would also be using dynamic import from a CJS root, which should allow the async hooks if I understand correctly.

kylesmile avatar Aug 27 '24 15:08 kylesmile

It's basically this

// main.cjs
import('./a.mjs');   // module.register() hooks are triggered when loading a.mjs
require('./b.mjs');  // module.register() hooks won't be triggered when loading `b.mjs`. Need to wait for the upcoming module.registerHooks() API.

joyeecheung avatar Aug 27 '24 15:08 joyeecheung

I tested a bit and noticed actually the assertion has been bogus since https://github.com/nodejs/node/pull/52047, so opened https://github.com/nodejs/node/pull/54592 to remove it.

joyeecheung avatar Aug 27 '24 15:08 joyeecheung