Node internal crash node::fs::InternalModuleStat starting from Node 22.10

[arthur-fontaine]

I use this library through import-sync, but I can't import this library anymore.

 #  node[17027]: void node::fs::InternalModuleStat(const v8::FunctionCallbackInfo<v8::Value>&) at ../src/node_file.cc:1043
  #  Assertion failed: (args.Length()) >= (2)

----- Native stack trace -----

 1: 0xfb795c node::Assert(node::AssertionInfo const&) [node]
 2: 0xfc0a74  [node]
 3: 0x1f209b8  [node]

----- JavaScript stack trace -----

1: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:2375:36
2: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11144:39
3: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11147:27
4: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11160:21
5: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11222:28
6: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11243:33
7: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11515:30
8: /workspaces/barebones-nodejs/@httptoolkit/esm/esm.js:11512:21
9: onceWrapper (node:events:622:26)
10: emit (node:events:507:28)


Aborted

I debugged it and found it comes from this exact line.

esm/src/fs/stat-fast.js

Line 70 in aa8723a

? binding.fs.internalModuleStat(toNamespacedPath(thePath))

I checked what should be the second argument (as told by the error)

https://github.com/nodejs/node/blob/9b6cea6ebe1942e24dc096b1bae53ecfacb781b2/lib/internal/modules/cjs/loader.js#L239C55-L239C72

The patch should be

+ ? binding.fs.internalModuleStat(binding.fs, toNamespacedPath(thePath))
- ? binding.fs.internalModuleStat(toNamespacedPath(thePath))

But the binding.fs parameter should ONLY be added if the version is greater or equal to 22.10. Maybe we should try to find the commit in Node to ensure that there is no others conditions.

You can find a reproduction at https://github.com/arthur-fontaine/htk-esm-internal-crash-repro. It doesn't use the @httptoolkit/esm library, but I copied its code and tried to reduce it to localize the bug. (look for the "HERE IS THE BUG" comment in esm/loader.js)

EDIT:

I think this is the commit where internalModuleStat started to require 2 parameters nodejs/node@f5d454a.

EDIT2:

The PR on Node that added the second parameter on internalModuleStat: nodejs/node#54408

EDIT3:

Checking if Node is version 22.10 or greater seems to be the only condition. Then the patch should look like

+ const nodeVersion = process.version.slice(1).split('.'); // Ex. "22.10.0" -> ["22", "10", "0"]
+ const major = parseInt(nodeVersion[0], 10);
+ const minor = parseInt(nodeVersion[1], 10);

const result = typeof thePath === "string"
-    ? binding.fs.internalModuleStat(toNamespacedPath(thePath))
+    ? (major > 22 || (major === 22 && minor >= 10))
+        ? binding.fs.internalModuleStat(binding.fs, toNamespacedPath(thePath))
+        : binding.fs.internalModuleStat(toNamespacedPath(thePath))
    : -1;