VM Module linking unclear error message

[blikest]

Version

24.9.0

Platform

Linux x64

Subsystem

vm

What steps will reproduce the bug?

1. Instantiate a SourceTextModule importing other normal modules from filesystem.
2. An unintelligible erroris thrown when linking a seemingly normal, second-layer dependency without further imports.
   My implementation based on the docs is roughly:

 export async function modularise(resource,identifier,context={},depth=0)
{// uses --experimental-vm-modules 
 let {SourceTextModule,SyntheticModule,createContext,isContext}=await import("vm");
 let {pathToFileURL}=await import("url");
 let synthetic=!resource&&/^[a-z]/.test(identifier);
 if(!synthetic)
 identifier=await resolve("path","resolve",identifier)
,resource=await fs.read(identifier,"utf8");
 if(!isContext(context))
 context=createContext({imports:{},...context});
 let precedent=context.imports[identifier];
 if(precedent)
 return precedent;
 let module=synthetic
?await import(identifier).then(module=>new SyntheticModule(Object.keys(module),function()
{Object.entries(module).reduce((module,entry)=>
 module.setExport(...entry)||module
,this);
},{identifier,context}))
:new SourceTextModule(resource||""
,{identifier,context
 ,importModuleDynamically:specifier=>import(specifier)//compose(link,"namespace")({specifier})
 ,initializeImportMeta:meta=>Object.assign(meta,{url:pathToFileURL(identifier)})
 });
 merge(context.imports,module,[identifier]);
 let links=await module.moduleRequests?.reduce(record(({specifier})=>
 modularise(null,specifier,context,depth+1)),[]);
 await module.linkRequests?.(links);
 module.instantiate?.();
 if(!depth)
 await module.evaluate();
 return module;
};

logged import graph:

first.js (unlinked) ➡️ middle.js (unlinked) ➡️ third.js (linked)
                                                   ⬆️
                                            ➡️ neighbor. js (unlinked)

Error received when instantiating middle.js:

[ERR_VM_MODULE_LINK_FAILURE] request for './third.js' is from a module not been linked
  at module.instantiate?.()

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

consistently with the given module graph

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

Error message in more correct English, since third.js is in fact in linked status, and middle.js is only not because that's the one being instantiated.
Or is it a problem that middle.js remains in unlinked stage after linkRequests?...

What do you see instead?

The error message should at least be more clear. "from a module not yet linked" perhaps?

Additional information

No response

[blikest]

After finding the cause of the error, I would add that I found a mismatch with the documentation,
because it says in multiple places that SourceTextModule
may use either .link or .linkRequests,
and especially at this point implies that .linkRequests is available even if the module has no dependencies (moduleRequests):

node/doc/api/vm.md

Line 1005 in 367bcce

If the module has no dependencies, the `modules` array can be empty.

The cause of my error was in fact that third.js had no dependencies,
and because of this the .linkRequests method was not actually present on its SourceTextModule.

Currently we have to conditionally call .link if moduleRequests is empty.

Not sure if this is an issue with the docs or the implementation.

Thank you for the quick fix to the error message!

edit: i just discovered that neighbor.js has no linkRequests method either despite having moduleRequests.length===1. There might be a reason for this?
output from module.evaluate().catch(fail=>console.log(module.linkRequests,module))

undefined
SourceTextModule
#dependencySpecifiers: undefined
#error: Symbol(kNoError)
#moduleRequests: Array(1)
#statusOverride: undefined
Symbol(kContext): {imports: {…}, Reflect: Reflect, URL: ƒ, Object: ƒ, Blob: ƒ, …}
Symbol(kWrap): ModuleWrap {sourceURL: undefined, sourceMapURL: undefined, url: '/Blik_2023_search.js'}
context: (...)
dependencySpecifiers: (...)
error: (...)
identifier: "/Blik_2023_search.js"
moduleRequests: Array(1)
namespace: (...)
status: 'unlinked'
[[Prototype]]: Module
 constructor: class SourceTextModule
 createCachedData: ƒ createCachedData()
 dependencySpecifiers: (...)
 error: (...)
 moduleRequests: (...)
 status: (...)
 Symbol(kLink): async [kLink](linker) { this.#statusOverride = 'linking'; // Iterates the module requests and links with the linker. // Modules should be aligned with the moduleRequests array in order. const modulePromises = Array(this.#moduleRequests.length); // Iterates with index to avoid calling into userspace with `Symbol.iterator`. for (let idx = 0; idx < this.#moduleRequests.length; idx++) { const { specifier, attributes } = this.#moduleRequests[idx]; const linkerResult = linker(specifier, this, { attributes, assert: attributes, }); const modulePromise = PromisePrototypeThen( PromiseResolve(linkerResult), async (module) => {…}
 context: (...)
 identifier: (...)
 namespace: (...)
 get dependencySpecifiers: dependencySpecifiers() { this.#dependencySpecifiers ??= ObjectFreeze( ArrayPrototypeMap(this.#moduleRequests, (request) => {…}
 get error: ƒ error()
 get moduleRequests: ƒ moduleRequests()
 get status: ƒ status()
 [[Prototype]]: Object
[[Prototype]]: Object

[legendecas]

linkRequests is only available on a SourceTextModule after Node.js v24.8.0, as an alternative to the method link. There is no need for a SyntheticModule to be linked.

Can you provide a minimum reproduction code that the method linkRequests is missing on SourceTextModule?

[blikest]

sorry, this was my dummy mistake, i switched computers in the meantime and forgot that was behind at v24.4.1. At least I got backwards-compatibility in check. No issue with the documentation. Thanks for the confirmation about SyntheticModules.

Only uncertainty based on the docs remaining is if the original .link method was calling the linker function sequentially (even if asynchronous) or in parallel. Because the latter seems to not work with .moduleRequests.map(linker) (where linker is async), only a sequential reduce, understandably throwing errors about unlinked states when they re-import a same module (pending linkage from the first occurrence) otherwise.

I figure parallel invocation (of async linkers) would have a very limited use case, if any, for import graphs that are acyclical, but it doesn't seem obvious from how the docs describe the original .link implementation:

node/doc/api/vm.md

Lines 705 to 709 in 367bcce

	The actual [HostResolveImportedModule][] implementation used during module
	linking is one that returns the modules linked during linking. Since at
	that point all modules would have been fully linked already, the
	[HostResolveImportedModule][] implementation is fully synchronous per
	specification.

It may follow from "returns the modules linked during linking" that "at that point they would have been fully linked", but what asynchronousity really makes me wonder (since if they are called sequentially, asynchronousity seems to come down to a JS-specific circumstance to resolve top-level awaits, not necessarily a divergence from the spec) is if this resolution can be done in parallel, or that's out of question.

The section on linkRequests similarly only says It's up to the creator of the SourceTextModule to determine if the resolution of the dependencies is synchronous or asynchronous, but not if that can have any other standard reason than top-level awaits (like any room to actually parallelize).

Anyway this was my last thought, it might just be a trivial fact that parallel invocation is out of question except for the negligable edge case of acyclical graphs.

Thank you for the support!

[legendecas]

Linkers have nothing to do with TLAs. That's the main reason that the replacement API .linkRequests(modules) does not ask for a promise. The caller can await the linker to be finished, and call .linkRequest(modules) when the promise resolves:

await module.link(linker);
// roughly equivalent to
const promises = module.moduleRequests.map(linker);
const moduleRequests = await Promise.all(promises);
module.linkRequests(moduleRequests);
module.instantiate();

This allows the linker to be fully synchronous.

[blikest]

i mean moduleRequests.map(linker) will call linker in parallel for each module, which causes errors for me with acyclical graphs because modules they import in common will only be queued to resolve in one of them, the others get it from cache to match the Module instance - and may finish before it finishes linking in the first one.

To really make the moduleRequests resolve synchronously (meaning also sequentially) you in fact need

 const moduleRequests=await module.moduleRequests.reduce((promise,next)=>
 promise.then(modules=>linker(next).then(module=>
 [...modules,module]))
,Promise.resolve([]));