vm: make vm.Module.evaluate() conditionally synchronous

- Make sure that the vm.Module.evaluate() method is conditionally synchronous based on the specification. Previously, the returned promise is unconditionally pending (even for synthetic modules and source text modules without top-level await) instead of immediately fulfilled, making it harder to debug as it deviates from the specified behavior. - Clarify the synchronicity of this method in the documentation - Add more tests for the synchronicity of this method. PR-URL: #60205 Refs: #59656 Refs: #37648 Reviewed-By: Chengzhong Wu <[email protected]>

Author: joyeecheung

doc/api/vm.md

@@ -618,19 +618,47 @@ in the ECMAScript specification.
  work after that. **Default:**`false`.
 * Returns:{Promise} Fulfills with `undefined` upon success.
 
-Evaluate the module.
+Evaluate the module and its depenendencies. Corresponds to the [Evaluate() concrete method][] field of
+[Cyclic Module Record][]s in the ECMAScript specification.
 
-This must be called after the module has been linked; otherwise it will reject.
-It could be called also when the module has already been evaluated, in which
-case it will either do nothing if the initial evaluation ended in success
-(`module.status` is `'evaluated'`) or it will re-throw the exception that the
-initial evaluation resulted in (`module.status` is `'errored'`).
+If the module is a `vm.SourceTextModule`, `evaluate()` must be called after the module has been instantiated;
+otherwise `evaluate()` will return a rejected promise.
 
-This method cannot be called while the module is being evaluated
-(`module.status` is `'evaluating'`).
+For a `vm.SourceTextModule`, the promise returned by `evaluate()` may be fulfilled either
+synchronously or asynchronously:
 
-Corresponds to the [Evaluate() concrete method][] field of [Cyclic Module
-Record][]s in the ECMAScript specification.
+1. If the `vm.SourceTextModule` has no top-level `await` in itself or any of its dependencies, the promise will be
+ fulfilled _synchronously_ after the module and all its dependencies have been evaluated.
+1. If the evaluation succeeds, the promise will be _synchronously_ resolved to `undefined`.
+2. If the evaluation results in an exception, the promise will be _synchronously_ rejected with the exception
+ that causes the evaluation to fail, which is the same as `module.error`.
+2. If the `vm.SourceTextModule` has top-level `await` in itself or any of its dependencies, the promise will be
+ fulfilled _asynchronously_ after the module and all its dependencies have been evaluated.
+1. If the evaluation succeeds, the promise will be _asynchronously_ resolved to `undefined`.
+2. If the evaluation results in an exception, the promise will be _asynchronously_ rejected with the exception
+ that causes the evaluation to fail.
+
+If the module is a `vm.SyntheticModule`, `evaluate()` always returns a promise that fulfills synchronously, see
+the specification of [Evaluate() of a Synthetic Module Record][]:
+
+1. If the `evaluateCallback` passed to its constructor throws an exception synchronously, `evaluate()` returns
+ a promise that will be synchronously rejected with that exception.
+2. If the `evaluateCallback` does not throw an exception, `evaluate()` returns a promise that will be
+ synchronously resolved to `undefined`.
+
+The `evaluateCallback` of a `vm.SyntheticModule` is executed synchronously within the `evaluate()` call, and its
+return value is discarded. This means if `evaluateCallback` is an asynchronous function, the promise returned by
+`evaluate()` will not reflect its asynchronous behavior, and any rejections from an asynchronous
+`evaluateCallback` will be lost.
+
+`evaluate()` could also be called again after the module has already been evaluated, in which case:
+
+1. If the initial evaluation ended in success (`module.status` is `'evaluated'`), it will do nothing
+ and return a promise that resolves to `undefined`.
+2. If the initial evaluation resulted in an exception (`module.status` is `'errored'`), it will re-reject
+ the exception that the initial evaluation resulted in.
+
+This method cannot be called while the module is being evaluated (`module.status` is `'evaluating'`).
 
 ### `module.identifier`
 
@@ -2221,6 +2249,7 @@ const{Script, SyntheticModule } = require('node:vm');
 [Cyclic Module Record]: https://tc39.es/ecma262/#sec-cyclic-module-records
 [ECMAScript Module Loader]: esm.md#modules-ecmascript-modules
 [Evaluate() concrete method]: https://tc39.es/ecma262/#sec-moduleevaluation
+[Evaluate() of a Synthetic Module Record]: https://tc39.es/ecma262/#sec-smr-Evaluate
 [FinishLoadingImportedModule]: https://tc39.es/ecma262/#sec-FinishLoadingImportedModule
 [GetModuleNamespace]: https://tc39.es/ecma262/#sec-getmodulenamespace
 [HostLoadImportedModule]: https://tc39.es/ecma262/#sec-HostLoadImportedModule

lib/internal/vm/module.js

@@ -13,6 +13,7 @@ const{
  ObjectPrototypeHasOwnProperty,
  ObjectSetPrototypeOf,
  PromisePrototypeThen,
+ PromiseReject,
  PromiseResolve,
  ReflectApply,
  SafePromiseAllReturnArrayLike,
@@ -208,27 +209,31 @@ class Module{
 this[kWrap].instantiate();
 }
 
-asyncevaluate(options=kEmptyObject){
-validateThisInternalField(this,kWrap,'Module');
-validateObject(options,'options');
-
-lettimeout=options.timeout;
-if(timeout===undefined){
-timeout=-1;
-}else{
-validateUint32(timeout,'options.timeout',true);
-}
-const{ breakOnSigint =false}=options;
-validateBoolean(breakOnSigint,'options.breakOnSigint');
-conststatus=this[kWrap].getStatus();
-if(status!==kInstantiated&&
-status!==kEvaluated&&
-status!==kErrored){
-thrownewERR_VM_MODULE_STATUS(
-'must be one of linked, evaluated, or errored',
-);
+evaluate(options=kEmptyObject){
+try{
+validateThisInternalField(this,kWrap,'Module');
+validateObject(options,'options');
+
+lettimeout=options.timeout;
+if(timeout===undefined){
+timeout=-1;
+}else{
+validateUint32(timeout,'options.timeout',true);
+}
+const{ breakOnSigint =false}=options;
+validateBoolean(breakOnSigint,'options.breakOnSigint');
+conststatus=this[kWrap].getStatus();
+if(status!==kInstantiated&&
+status!==kEvaluated&&
+status!==kErrored){
+thrownewERR_VM_MODULE_STATUS(
+'must be one of linked, evaluated, or errored',
+);
+}
+returnthis[kWrap].evaluate(timeout,breakOnSigint);
+}catch(e){
+returnPromiseReject(e);
 }
-awaitthis[kWrap].evaluate(timeout,breakOnSigint);
 }
 
 [customInspectSymbol](depth,options){

test/parallel/test-vm-module-evaluate-source-text-module.js

@@ -0,0 +1,167 @@
+// Flags: --experimental-vm-modules
+'use strict';
+
+// This tests the result of evaluating a vm.SourceTextModule.
+constcommon=require('../common');
+
+constassert=require('assert');
+// To make testing easier we just use the public inspect API. If the output format
+// changes, update this test accordingly.
+const{ inspect }=require('util');
+constvm=require('vm');
+
+globalThis.callCount={};
+common.allowGlobals(globalThis.callCount);
+
+// Synchronous error during evaluation results in a synchronously rejected promise.
+{
+globalThis.callCount.syncError=0;
+constmod=newvm.SourceTextModule(`
+ globalThis.callCount.syncError++;
+ throw new Error("synchronous source text module");
+ export const a = 1;
+ `);
+mod.linkRequests([]);
+mod.instantiate();
+constpromise=mod.evaluate();
+assert.strictEqual(globalThis.callCount.syncError,1);
+assert.match(inspect(promise),/rejected/);
+assert(mod.error,'Expected mod.error to be set');
+assert.strictEqual(mod.error.message,'synchronous source text module');
+
+promise.catch(common.mustCall((err)=>{
+assert.strictEqual(err,mod.error);
+// Calling evaluate() again results in the same rejection synchronously.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/rejected/);
+promise2.catch(common.mustCall((err2)=>{
+assert.strictEqual(err,err2);
+// The module is only evaluated once.
+assert.strictEqual(globalThis.callCount.syncError,1);
+}));
+}));
+}
+
+// Successful evaluation of a module without top-level await results in a
+// promise synchronously resolved to undefined.
+{
+globalThis.callCount.syncNamedExports=0;
+constmod=newvm.SourceTextModule(`
+ globalThis.callCount.syncNamedExports++;
+ export const a = 1, b = 2;
+ `);
+mod.linkRequests([]);
+mod.instantiate();
+constpromise=mod.evaluate();
+assert.match(inspect(promise),/Promise{undefined}/);
+assert.strictEqual(mod.namespace.a,1);
+assert.strictEqual(mod.namespace.b,2);
+assert.strictEqual(globalThis.callCount.syncNamedExports,1);
+promise.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+
+// Calling evaluate() again results in the same resolved promise synchronously.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/Promise{undefined}/);
+assert.strictEqual(mod.namespace.a,1);
+assert.strictEqual(mod.namespace.b,2);
+promise2.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+// The module is only evaluated once.
+assert.strictEqual(globalThis.callCount.syncNamedExports,1);
+}));
+}));
+}
+
+{
+globalThis.callCount.syncDefaultExports=0;
+// Modules with either named and default exports have the same behaviors.
+constmod=newvm.SourceTextModule(`
+ globalThis.callCount.syncDefaultExports++;
+ export default 42;
+ `);
+mod.linkRequests([]);
+mod.instantiate();
+constpromise=mod.evaluate();
+assert.match(inspect(promise),/Promise{undefined}/);
+assert.strictEqual(mod.namespace.default,42);
+assert.strictEqual(globalThis.callCount.syncDefaultExports,1);
+
+promise.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+
+// Calling evaluate() again results in the same resolved promise synchronously.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/Promise{undefined}/);
+assert.strictEqual(mod.namespace.default,42);
+promise2.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+// The module is only evaluated once.
+assert.strictEqual(globalThis.callCount.syncDefaultExports,1);
+}));
+}));
+}
+
+// Successful evaluation of a module with top-level await results in a promise
+// that is fulfilled asynchronously with undefined.
+{
+globalThis.callCount.asyncEvaluation=0;
+constmod=newvm.SourceTextModule(`
+ globalThis.callCount.asyncEvaluation++;
+ await Promise.resolve();
+ export const a = 1;
+ `);
+mod.linkRequests([]);
+mod.instantiate();
+constpromise=mod.evaluate();
+assert.match(inspect(promise),/<pending>/);
+// Accessing the namespace before the promise is fulfilled throws ReferenceError.
+assert.throws(()=>mod.namespace.a,{name: 'ReferenceError'});
+assert.strictEqual(globalThis.callCount.asyncEvaluation,1);
+promise.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+assert.strictEqual(globalThis.callCount.asyncEvaluation,1);
+
+// Calling evaluate() again results in a promise synchronously resolved to undefined.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/Promise{undefined}/);
+assert.strictEqual(mod.namespace.a,1);
+promise2.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+// The module is only evaluated once.
+assert.strictEqual(globalThis.callCount.asyncEvaluation,1);
+}));
+}));
+}
+
+// Rejection of a top-level await promise results in a promise that is
+// rejected asynchronously with the same reason.
+{
+globalThis.callCount.asyncRejection=0;
+constmod=newvm.SourceTextModule(`
+ globalThis.callCount.asyncRejection++;
+ await Promise.reject(new Error("asynchronous source text module"));
+ export const a = 1;
+ `);
+mod.linkRequests([]);
+mod.instantiate();
+constpromise=mod.evaluate();
+assert.match(inspect(promise),/<pending>/);
+// Accessing the namespace before the promise is fulfilled throws ReferenceError.
+assert.throws(()=>mod.namespace.a,{name: 'ReferenceError'});
+promise.catch(common.mustCall((err)=>{
+assert.strictEqual(err,mod.error);
+assert.strictEqual(err.message,'asynchronous source text module');
+assert.strictEqual(globalThis.callCount.asyncRejection,1);
+
+// Calling evaluate() again results in a promise synchronously rejected
+// with the same reason.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/rejected/);
+promise2.catch(common.mustCall((err2)=>{
+assert.strictEqual(err,err2);
+// The module is only evaluated once.
+assert.strictEqual(globalThis.callCount.asyncRejection,1);
+}));
+}));
+}

test/parallel/test-vm-module-evaluate-synthethic-module-rejection.js

@@ -0,0 +1,42 @@
+// Flags: --experimental-vm-modules
+'use strict';
+
+// This tests the result of evaluating a vm.SyntheticModule with an async rejection
+// in the evaluation step.
+
+constcommon=require('../common');
+
+constassert=require('assert');
+// To make testing easier we just use the public inspect API. If the output format
+// changes, update this test accordingly.
+const{ inspect }=require('util');
+constvm=require('vm');
+
+// The promise _synchronously_ resolves to undefined, because for a synthethic module,
+// the evaluation operation can only either resolve or reject immediately.
+// In this case, the asynchronously rejected promise can't be handled from the outside,
+// so we'll catch it with the isolate-level unhandledRejection handler.
+// See https://tc39.es/ecma262/#sec-smr-Evaluate
+process.on('unhandledRejection',common.mustCall((err)=>{
+assert.strictEqual(err.message,'asynchronous source text module');
+}));
+
+constmod=newvm.SyntheticModule(['a'],common.mustCall(async()=>{
+thrownewError('asynchronous source text module');
+}));
+
+constpromise=mod.evaluate();
+assert.match(inspect(promise),/Promise{undefined}/);
+// Accessing the uninitialized export of a synthetic module returns undefined.
+assert.strictEqual(mod.namespace.a,undefined);
+
+promise.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+}));
+
+// Calling evaluate() again results in a promise _synchronously_ resolved to undefined again.
+constpromise2=mod.evaluate();
+assert.match(inspect(promise2),/Promise{undefined}/);
+promise2.then(common.mustCall((value)=>{
+assert.strictEqual(value,undefined);
+}));