fixup! module: allow ESM that failed to be required to be re-imported

lib/internal/modules/esm/module_job.js

@@ -22,7 +22,7 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
 
-const { ModuleWrap, kEvaluated, kInstantiated } = internalBinding('module_wrap');
+const { ModuleWrap, kInstantiated } = internalBinding('module_wrap');
 const {
   privateSymbols: {
     entry_point_module_private_symbol,
@@ -356,14 +356,19 @@ class ModuleJobSync extends ModuleJobBase {
   async run() {
     // This path is hit by a require'd module that is imported again.
     const status = this.module.getStatus();
-    if (status === kEvaluated) {
+    if (status > kInstantiated) {
+      if (this.evaluationPromise) {
+        await this.evaluationPromise;
+      }
       return { __proto__: null, module: this.module };
     } else if (status === kInstantiated) {
       // The evaluation may have been canceled because instantiateSync() detected TLA first.
       // But when it is imported again, it's fine to re-evaluate it asynchronously.
       const timeout = -1;
       const breakOnSigint = false;
-      await this.module.evaluate(timeout, breakOnSigint);
+      this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint);
+      await this.evaluationPromise;
+      this.evaluationPromise = undefined;
       return { __proto__: null, module: this.module };
     }
 

test/es-module/test-require-module-retry-import-errored.js

@@ -0,0 +1,35 @@
+// This tests that after failing to require an ESM that contains TLA,
+// retrying with import() still works, and produces consistent results.
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const { exportedReject } = require('../fixtures/es-modules/tla/export-promise.mjs');
+
+assert.throws(() => {
+  require('../fixtures/es-modules/tla/await-export-promise.mjs');
+}, {
+  code: 'ERR_REQUIRE_ASYNC_MODULE'
+});
+
+const interval = setInterval(() => {}, 1000);  // Keep the test running, because await alone doesn't.
+const err = new Error('rejected');
+
+const p1 = import('../fixtures/es-modules/tla/await-export-promise.mjs')
+  .then(common.mustNotCall(), common.mustCall((e) => {
+    assert.strictEqual(e, err);
+  }));
+
+const p2 = import('../fixtures/es-modules/tla/await-export-promise.mjs')
+  .then(common.mustNotCall(), common.mustCall((e) => {
+    assert.strictEqual(e, err);
+  }));
+
+const p3 = import('../fixtures/es-modules/tla/await-export-promise.mjs')
+  .then(common.mustNotCall(), common.mustCall((e) => {
+    assert.strictEqual(e, err);
+  }));
+
+exportedReject(err);
+
+Promise.all([p1, p2, p3]).then(common.mustCall(() => { clearInterval(interval); }));

test/es-module/test-require-module-retry-import-evaluating.js

@@ -0,0 +1,32 @@
+// This tests that after failing to require an ESM that contains TLA,
+// retrying with import() still works, and produces consistent results.
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+const { exportedResolve } = require('../fixtures/es-modules/tla/export-promise.mjs');
+
+assert.throws(() => {
+  require('../fixtures/es-modules/tla/await-export-promise.mjs');
+}, {
+  code: 'ERR_REQUIRE_ASYNC_MODULE'
+});
+
+const interval = setInterval(() => {}, 1000);  // Keep the test running, because await alone doesn't.
+const value = { hello: 'world' };
+
+const p1 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => {
+  assert.strictEqual(ns.default, value);
+}), common.mustNotCall());
+
+const p2 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => {
+  assert.strictEqual(ns.default, value);
+}), common.mustNotCall());
+
+const p3 = import('../fixtures/es-modules/tla/await-export-promise.mjs').then(common.mustCall((ns) => {
+  assert.strictEqual(ns.default, value);
+}), common.mustNotCall());
+
+exportedResolve(value);
+
+Promise.all([p1, p2, p3]).then(common.mustCall(() => { clearInterval(interval); }));

test/fixtures/es-modules/tla/await-export-promise.mjs

@@ -0,0 +1,4 @@
+import promise from './export-promise.mjs';
+let result;
+result = await promise;
+export default result;

test/fixtures/es-modules/tla/export-promise.mjs

@@ -0,0 +1,8 @@
+let exportedResolve;
+let exportedReject;
+const promise = new Promise((resolve, reject) => {
+  exportedResolve = resolve;
+  exportedReject = reject;
+});
+export default promise;
+export { exportedResolve, exportedReject };