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

ERR_INTERNAL_ASSERTION with require(esm) #55500

Closed
nicolo-ribaudo opened this issue Oct 23, 2024 · 5 comments · Fixed by #55502
Closed

ERR_INTERNAL_ASSERTION with require(esm) #55500

nicolo-ribaudo opened this issue Oct 23, 2024 · 5 comments · Fixed by #55502
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders

Comments

@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Oct 23, 2024

Version

23.0.0

Platform

Darwin Nics-MacBook-Air.local 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:16:46 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T8112 arm64

Subsystem

No response

What steps will reproduce the bug?

EDIT: See in #55500 (comment) for a minimal reproduction

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

I don't have a minimal reproduction

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

To not tell me to open a bug report :P

What do you see instead?

(node:69972) 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:11
    throw new ERR_INTERNAL_ASSERTION(message);
          ^

Error [ERR_INTERNAL_ASSERTION]: [BABEL]: A require()-d module that is imported again must be evaluated. Status = 2
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
 (While processing: /Users/nic/Documents/dev/github.com/babel/babel/packages/babel-core/test/fixtures/async/plugin-mjs-tla-native/plugin.js)
    at assert (node:internal/assert:11:11)
    at ModuleJobSync.run (node:internal/modules/esm/module_job:358:5)
    at onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:543:42)
    at async loadMjsFromPath (file:///Users/nic/Documents/dev/github.com/babel/babel/packages/babel-core/lib/config/files/module-types.js:45:12) {
  code: 'ERR_INTERNAL_ASSERTION'
}

Additional information

I don't think I'm using Node.js internals.

cc @joyeecheung

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Oct 23, 2024

The relevant code is in packages/babel-core/lib/config/files/module-types.js.

What is going on is that:

  • in loadCodeDefault, we go in the auto .js branch
  • it calls loadCjsDefault, which requires plugin.js
  • that require call throws due to TLA
  • In the catch inside loadCodeDefault, async ??= yield* isAsync() is true to we suppress the error
  • It falls through, and calls loadMjsFromPath in the auto .mjs branch
  • loadMjsFromPath calls import_(url), which calls import(url)

However, a minimal reproduction of try/catch around require() falling back to import() does not trigger the assertion.

@nicolo-ribaudo
Copy link
Contributor Author

Minimal reproduction:

try {
  require("./file-with-tla.mjs");
} catch {
  import("./file-with-tla.mjs").then(console.log, console.error);
}

@RedYetiDev RedYetiDev added esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders confirmed-bug Issues with confirmed bugs. labels Oct 23, 2024
@RedYetiDev
Copy link
Member

// tla.js
await new Promise(resolve => resolve());
try {
    require("./tla.js");
} catch {
    import("./tla.js");
}

@RedYetiDev
Copy link
Member

Failing assertion:

assert(status === kEvaluated,

@joyeecheung
Copy link
Member

joyeecheung commented Oct 23, 2024

Thanks for reporting it, fix in #55502

Ceres6 pushed a commit to Ceres6/node that referenced this issue Oct 30, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
RafaelGSS pushed a commit that referenced this issue Nov 1, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: #55502
Fixes: #55500
Refs: #52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
joyeecheung added a commit to joyeecheung/node that referenced this issue Nov 12, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
joyeecheung added a commit to joyeecheung/node that referenced this issue Nov 12, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
joyeecheung added a commit to joyeecheung/node that referenced this issue Nov 23, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: nodejs#55502
Fixes: nodejs#55500
Refs: nodejs#52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
ruyadorno pushed a commit that referenced this issue Nov 27, 2024
When a ESM module cannot be loaded by require due to the presence
of TLA, its module status would be stopped at kInstantiated. In
this case, when it's imported again, we should allow it to be
evaluated asynchronously, as it's also a common pattern for users
to retry with dynamic import when require fails.

PR-URL: #55502
Backport-PR-URL: #55217
Fixes: #55500
Refs: #52697
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants