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

Author: joyeecheung

doc/api/vm.md

@@ -642,10 +642,12 @@ 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`. This is the case even if the `evaluateCallback` is an
-   asynchronous function, any asynchronous operations it performs will only have effect after
-   `evaluate()` has returned. If an `evaluateCallback` rejects asynchronously, that rejection will be
-   not reflected in the promise returned by `evaluate()`.
+   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:
 

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

@@ -26,9 +26,11 @@ common.allowGlobals(globalThis.callCount);
   const promise = 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.message, 'synchronous source text module');
+    assert.strictEqual(err, mod.error);
     // Calling evaluate() again results in the same rejection synchronously.
     const promise2 = mod.evaluate();
     assert.match(inspect(promise2), /rejected/);
@@ -115,6 +117,7 @@ common.allowGlobals(globalThis.callCount);
   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);
@@ -147,6 +150,7 @@ common.allowGlobals(globalThis.callCount);
   // 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);
 

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

@@ -22,7 +22,7 @@ process.on('unhandledRejection', common.mustCall((err) => {
 }));
 
 const mod = new vm.SyntheticModule(['a'], common.mustCall(async () => {
-  await Promise.reject(new Error('asynchronous source text module'));
+  throw new Error('asynchronous source text module');
 }));
 
 const promise = mod.evaluate();

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

@@ -66,8 +66,11 @@ const vm = require('vm');
   }));
   const promise = mod.evaluate();
   assert.match(inspect(promise), /rejected/);
+  assert(mod.error, 'Expected mod.error to be set');
+  assert.strictEqual(mod.error.message, 'synchronous synthethic module');
+
   promise.catch(common.mustCall((err) => {
-    assert.strictEqual(err.message, 'synchronous synthethic module');
+    assert.strictEqual(err, mod.error);
 
     // Calling evaluate() again results in a promise _synchronously_ rejected
     // with the same reason.