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. PR-URL: #58957 Backport-PR-URL: #59504Fixes: #58945 Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Refs: #52697

Author: joyeecheung

lib/internal/modules/esm/loader.js

@@ -41,6 +41,7 @@ const{
  kEvaluated,
  kEvaluating,
  kInstantiated,
+ kErrored,
  throwIfPromiseRejected,
 }=internalBinding('module_wrap');
 const{
@@ -350,6 +351,9 @@ class ModuleLoader{
 mod[kRequiredModuleSymbol]=job.module;
 const{ namespace }=job.runSync(parent);
 return{wrap: job.module,namespace: namespace||job.module.getNamespace()};
+}elseif(status===kErrored){
+// If the module was previously imported and errored, throw the error.
+throwjob.module.getError();
 }
 // When the cached async job have already encountered a linking
 // error that gets wrapped into a rejection, but is still later

lib/internal/modules/esm/module_job.js

@@ -281,7 +281,7 @@ class ModuleJob extends ModuleJobBase{
 assert(this.moduleinstanceofModuleWrap);
 letstatus=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){
@@ -316,6 +316,7 @@ class ModuleJob extends ModuleJobBase{
 }
 
 asyncrun(){
+debug('ModuleJob.run()',this.module);
 awaitthis.instantiate();
 consttimeout=-1;
 constbreakOnSigint=false;
@@ -392,7 +393,11 @@ class ModuleJobSync extends ModuleJobBase{
 asyncrun(){
 // This path is hit by a require'd module that is imported again.
 conststatus=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){
+throwthis.module.getError();
+}elseif(status>kInstantiated){
 if(this.evaluationPromise){
 awaitthis.evaluationPromise;
 }
@@ -413,6 +418,7 @@ class ModuleJobSync extends ModuleJobBase{
 }
 
 runSync(parent){
+debug('ModuleJobSync.runSync()',this.module);
 // TODO(joyeecheung): add the error decoration logic from the async instantiate.
 this.module.async=this.module.instantiateSync();
 // If --experimental-print-required-tla is true, proceeds to evaluation even

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';
+constcommon=require('../common');
+constassert=require('assert');
+
+(async()=>{
+awaitassert.rejects(import('../fixtures/es-modules/throw-error.mjs'),{
+message: 'test',
+});
+assert.throws(()=>{
+require('../fixtures/es-modules/throw-error.mjs');
+},{
+message: 'test',
+});
+})().catch(common.mustNotCall());

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';
+constcommon=require('../common');
+constassert=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());