From d2455ef3651f0cb99370189a0341e859e931cdbe Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Fri, 4 Jul 2025 19:38:27 +0200
Subject: [PATCH] module: throw error when re-runing errored module jobs

Re-evaluating an errored ESM should lead to rejecting
the rejection again - this is also the case when importing
it twice. In the case of retrying with
require after import, just throw the cached error.

Drive-by: add some debug logs.
---
 lib/internal/modules/esm/loader.js              |  4 ++++
 lib/internal/modules/esm/module_job.js          | 10 ++++++++--
 .../test-import-module-retry-require-errored.js | 17 +++++++++++++++++
 ...est-require-module-retry-import-errored-2.js | 16 ++++++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)
 create mode 100644 test/es-module/test-import-module-retry-require-errored.js
 create mode 100644 test/es-module/test-require-module-retry-import-errored-2.js

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 2d45f404a68fdc..068b34b3bd0227 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -43,6 +43,7 @@ const {
   kEvaluating,
   kEvaluationPhase,
   kInstantiated,
+  kErrored,
   kSourcePhase,
   throwIfPromiseRejected,
 } = internalBinding('module_wrap');
@@ -402,6 +403,9 @@ class ModuleLoader {
         mod[kRequiredModuleSymbol] = job.module;
         const { namespace } = job.runSync(parent);
         return { wrap: job.module, namespace: namespace || job.module.getNamespace() };
+      } else if (status === kErrored) {
+        // If the module was previously imported and errored, throw the error.
+        throw job.module.getError();
       }
       // When the cached async job have already encountered a linking
       // error that gets wrapped into a rejection, but is still later
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
index a7a65e50a1607b..e4c7f45a0baed1 100644
--- a/lib/internal/modules/esm/module_job.js
+++ b/lib/internal/modules/esm/module_job.js
@@ -325,7 +325,7 @@ class ModuleJob extends ModuleJobBase {
     assert(this.module instanceof ModuleWrap);
     let status = this.module.getStatus();
 
-    debug('ModuleJob.runSync', this.module);
+    debug('ModuleJob.runSync()', status, this.module);
     // FIXME(joyeecheung): this cannot fully handle < kInstantiated. Make the linking
     // fully synchronous instead.
     if (status === kUninstantiated) {
@@ -360,6 +360,7 @@ class ModuleJob extends ModuleJobBase {
   }
 
   async run(isEntryPoint = false) {
+    debug('ModuleJob.run()', this.module);
     assert(this.phase === kEvaluationPhase);
     await this.#instantiate();
     if (isEntryPoint) {
@@ -465,7 +466,11 @@ class ModuleJobSync extends ModuleJobBase {
     assert(this.phase === kEvaluationPhase);
     // This path is hit by a require'd module that is imported again.
     const status = this.module.getStatus();
-    if (status > kInstantiated) {
+    debug('ModuleJobSync.run()', status, this.module);
+    // If the module was previously required and errored, reject from import() again.
+    if (status === kErrored) {
+      throw this.module.getError();
+    } else if (status > kInstantiated) {
       if (this.evaluationPromise) {
         await this.evaluationPromise;
       }
@@ -486,6 +491,7 @@ class ModuleJobSync extends ModuleJobBase {
   }
 
   runSync(parent) {
+    debug('ModuleJobSync.runSync()', this.module);
     assert(this.phase === kEvaluationPhase);
     // TODO(joyeecheung): add the error decoration logic from the async instantiate.
     this.module.async = this.module.instantiateSync();
diff --git a/test/es-module/test-import-module-retry-require-errored.js b/test/es-module/test-import-module-retry-require-errored.js
new file mode 100644
index 00000000000000..79afa20aa70cb5
--- /dev/null
+++ b/test/es-module/test-import-module-retry-require-errored.js
@@ -0,0 +1,17 @@
+// This tests that after failing to import an ESM that rejects,
+// retrying with require() still throws.
+
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+(async () => {
+  await assert.rejects(import('../fixtures/es-modules/throw-error.mjs'), {
+    message: 'test',
+  });
+  assert.throws(() => {
+    require('../fixtures/es-modules/throw-error.mjs');
+  }, {
+    message: 'test',
+  });
+})().catch(common.mustNotCall());
diff --git a/test/es-module/test-require-module-retry-import-errored-2.js b/test/es-module/test-require-module-retry-import-errored-2.js
new file mode 100644
index 00000000000000..3504d9932c61af
--- /dev/null
+++ b/test/es-module/test-require-module-retry-import-errored-2.js
@@ -0,0 +1,16 @@
+// This tests that after failing to require an ESM that throws,
+// retrying with import() still rejects.
+
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+assert.throws(() => {
+  require('../fixtures/es-modules/throw-error.mjs');
+}, {
+  message: 'test',
+});
+
+assert.rejects(import('../fixtures/es-modules/throw-error.mjs'), {
+  message: 'test',
+}).catch(common.mustNotCall());