From b8bb1f0c95fffcfad03c10d3600df2c1542026b7 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 27 Mar 2026 15:53:16 +0100 Subject: [PATCH] esm: add `ERR_REQUIRE_ESM_RACE_CONDITION` --- doc/api/errors.md | 11 ++++++++ lib/internal/errors.js | 9 ++++++ lib/internal/modules/esm/loader.js | 28 ++++--------------- lib/internal/modules/esm/module_job.js | 4 ++- .../test-esm-require-race-condition.js | 9 ++++++ .../node_modules/cjs-pkg/index.js | 2 ++ .../node_modules/dual-pkg/index.cjs | 2 ++ .../node_modules/dual-pkg/index.mjs | 1 + .../node_modules/dual-pkg/package.json | 7 +++++ .../node_modules/esm-pkg/index.mjs | 1 + .../node_modules/esm-pkg/package.json | 4 +++ .../import-require-cycle/race-condition.cjs | 2 ++ 12 files changed, 57 insertions(+), 23 deletions(-) create mode 100644 test/es-module/test-esm-require-race-condition.js create mode 100644 test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js create mode 100644 test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs create mode 100644 test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs create mode 100644 test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json create mode 100644 test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs create mode 100644 test/fixtures/import-require-cycle/node_modules/esm-pkg/package.json create mode 100644 test/fixtures/import-require-cycle/race-condition.cjs diff --git a/doc/api/errors.md b/doc/api/errors.md index 65ef2ce7bf5d01..b7a258ce6a2f08 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2707,6 +2707,17 @@ This error has been deprecated since `require()` now supports loading synchronou ES modules. When `require()` encounters an ES module that contains top-level `await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead. + + +### `ERR_REQUIRE_ESM_RACE_CONDITION` + + + +An attempt was made to `require()` an [ES Module][] while another `import()` call +was already in progress to load it asynchronously. + ### `ERR_SCRIPT_EXECUTION_INTERRUPTED` diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7c4728627731fe..94f073446db2a1 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1730,6 +1730,15 @@ E('ERR_REQUIRE_ESM', 'all ES modules instead).\n'; return msg; }, Error); +E('ERR_REQUIRE_ESM_RACE_CONDITION', (filename, parentFilename, isForAsyncLoaderHookWorker) => { + let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`; + raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically '; + raceMessage += 'import()-ed via Promise.all().\n'; + raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n'; + raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`; + raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`; + return raceMessage; +}, Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); E('ERR_SERVER_ALREADY_LISTEN', diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 04c374c00cfc3e..8243d895712151 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -27,6 +27,7 @@ const { ERR_REQUIRE_ASYNC_MODULE, ERR_REQUIRE_CYCLE_MODULE, ERR_REQUIRE_ESM, + ERR_REQUIRE_ESM_RACE_CONDITION, ERR_UNKNOWN_MODULE_FORMAT, } = require('internal/errors').codes; const { getOptionValue } = require('internal/options'); @@ -49,6 +50,7 @@ const { kEvaluating, kEvaluationPhase, kInstantiated, + kUninstantiated, kErrored, kSourcePhase, throwIfPromiseRejected, @@ -102,24 +104,6 @@ const { translators } = require('internal/modules/esm/translators'); const { defaultResolve } = require('internal/modules/esm/resolve'); const { defaultLoadSync, throwUnknownModuleFormat } = require('internal/modules/esm/load'); -/** - * Generate message about potential race condition caused by requiring a cached module that has started - * async linking. - * @param {string} filename Filename of the module being required. - * @param {string|undefined} parentFilename Filename of the module calling require(). - * @param {boolean} isForAsyncLoaderHookWorker Whether this is for the async loader hook worker. - * @returns {string} Error message. - */ -function getRaceMessage(filename, parentFilename, isForAsyncLoaderHookWorker) { - let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`; - raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically '; - raceMessage += 'import()-ed via Promise.all().\n'; - raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n'; - raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`; - raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`; - return raceMessage; -} - /** * @typedef {import('../cjs/loader.js').Module} CJSModule */ @@ -306,7 +290,7 @@ class ModuleLoader { const parentFilename = urlToFilename(parent?.filename); // This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666 if (!job.module) { - assert.fail(getRaceMessage(filename, parentFilename), this.isForAsyncLoaderHookWorker); + throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, this.isForAsyncLoaderHookWorker); } const status = job.module.getStatus(); debug('Module status', job, status); @@ -339,8 +323,8 @@ class ModuleLoader { throwIfPromiseRejected(job.instantiated); } if (status !== kEvaluating) { - assert.fail(`Unexpected module status ${status}. ` + - getRaceMessage(filename, parentFilename)); + assert(status === kUninstantiated, `Unexpected module status ${status}`); + throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false); } let message = `Cannot require() ES Module ${filename} in a cycle.`; if (parentFilename) { @@ -376,7 +360,7 @@ class ModuleLoader { #checkCachedJobForRequireESM(specifier, url, parentURL, job) { // This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666 if (!job.module) { - assert.fail(getRaceMessage(url, parentURL, this.isForAsyncLoaderHookWorker)); + throw new ERR_REQUIRE_ESM_RACE_CONDITION(url, parentURL, this.isForAsyncLoaderHookWorker); } // This module is being evaluated, which means it's imported in a previous link // in a cycle. diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 22032f79e90d44..11d5b0a3cb66f9 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -55,6 +55,7 @@ const { getOptionValue } = require('internal/options'); const noop = FunctionPrototype; const { ERR_REQUIRE_ASYNC_MODULE, + ERR_REQUIRE_ESM_RACE_CONDITION, } = require('internal/errors').codes; let hasPausedEntry = false; @@ -420,7 +421,8 @@ class ModuleJob extends ModuleJobBase { // always handle CJS using the CJS loader to eliminate the quirks. return { __proto__: null, module: this.module, namespace: this.module.getNamespace() }; } - assert.fail(`Unexpected module status ${status}.`); + assert(status === kUninstantiated, `Unexpected module status ${status}.`); + throw new ERR_REQUIRE_ESM_RACE_CONDITION(); } async run(isEntryPoint = false) { diff --git a/test/es-module/test-esm-require-race-condition.js b/test/es-module/test-esm-require-race-condition.js new file mode 100644 index 00000000000000..db5f38e99ba19c --- /dev/null +++ b/test/es-module/test-esm-require-race-condition.js @@ -0,0 +1,9 @@ +'use strict'; +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('node:assert'); + +assert.throws( + () => require(fixtures.path('import-require-cycle/race-condition.cjs')), + { code: 'ERR_REQUIRE_ESM_RACE_CONDITION' }, +); diff --git a/test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js b/test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js new file mode 100644 index 00000000000000..fccd8bddaf294c --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js @@ -0,0 +1,2 @@ +'use strict'; +require('dual-pkg'); diff --git a/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs b/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs new file mode 100644 index 00000000000000..76ddafd134c797 --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs @@ -0,0 +1,2 @@ +'use strict'; +require('esm-pkg'); diff --git a/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs b/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs new file mode 100644 index 00000000000000..4a86e7b4154b3b --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs @@ -0,0 +1 @@ +import "esm-pkg"; diff --git a/test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json b/test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json new file mode 100644 index 00000000000000..46e082ade7d03a --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json @@ -0,0 +1,7 @@ +{ + "name": "dual-pkg", + "exports": { + "import": "./index.mjs", + "require": "./index.cjs" + } +} diff --git a/test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs b/test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs new file mode 100644 index 00000000000000..336ce12bb9106a --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs @@ -0,0 +1 @@ +export {} diff --git a/test/fixtures/import-require-cycle/node_modules/esm-pkg/package.json b/test/fixtures/import-require-cycle/node_modules/esm-pkg/package.json new file mode 100644 index 00000000000000..2e63a3959245f6 --- /dev/null +++ b/test/fixtures/import-require-cycle/node_modules/esm-pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "esm-pkg", + "exports": "./index.mjs" +} diff --git a/test/fixtures/import-require-cycle/race-condition.cjs b/test/fixtures/import-require-cycle/race-condition.cjs new file mode 100644 index 00000000000000..d5d879b4a195e8 --- /dev/null +++ b/test/fixtures/import-require-cycle/race-condition.cjs @@ -0,0 +1,2 @@ +import("dual-pkg"); +require("cjs-pkg");