Skip to content

Commit adeb14a

Browse files
authored
esm: add ERR_REQUIRE_ESM_RACE_CONDITION
PR-URL: #62462 Fixes: #62432 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me>
1 parent 2f5cbec commit adeb14a

File tree

12 files changed

+59
-23
lines changed

12 files changed

+59
-23
lines changed

β€Ždoc/api/errors.mdβ€Ž

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,6 +2707,19 @@ This error has been deprecated since `require()` now supports loading synchronou
27072707
ES modules. When `require()` encounters an ES module that contains top-level
27082708
`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
27092709

2710+
<a id="ERR_REQUIRE_ESM_RACE_CONDITION"></a>
2711+
2712+
### `ERR_REQUIRE_ESM_RACE_CONDITION`
2713+
2714+
<!-- YAML
2715+
added: REPLACEME
2716+
-->
2717+
2718+
> Stability: 1 - Experimental.
2719+
2720+
An attempt was made to `require()` an [ES Module][] while another `import()` call
2721+
was already in progress to load it asynchronously.
2722+
27102723
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
27112724

27122725
### `ERR_SCRIPT_EXECUTION_INTERRUPTED`

β€Žlib/internal/errors.jsβ€Ž

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1730,6 +1730,15 @@ E('ERR_REQUIRE_ESM',
17301730
'all ES modules instead).\n';
17311731
return msg;
17321732
}, Error);
1733+
E('ERR_REQUIRE_ESM_RACE_CONDITION', (filename, parentFilename, isForAsyncLoaderHookWorker) => {
1734+
let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
1735+
raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
1736+
raceMessage += 'import()-ed via Promise.all().\n';
1737+
raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
1738+
raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
1739+
raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
1740+
return raceMessage;
1741+
}, Error);
17331742
E('ERR_SCRIPT_EXECUTION_INTERRUPTED',
17341743
'Script execution was interrupted by `SIGINT`', Error);
17351744
E('ERR_SERVER_ALREADY_LISTEN',

β€Žlib/internal/modules/esm/loader.jsβ€Ž

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const {
2727
ERR_REQUIRE_ASYNC_MODULE,
2828
ERR_REQUIRE_CYCLE_MODULE,
2929
ERR_REQUIRE_ESM,
30+
ERR_REQUIRE_ESM_RACE_CONDITION,
3031
ERR_UNKNOWN_MODULE_FORMAT,
3132
} = require('internal/errors').codes;
3233
const { getOptionValue } = require('internal/options');
@@ -52,6 +53,7 @@ const {
5253
kEvaluating,
5354
kEvaluationPhase,
5455
kInstantiated,
56+
kUninstantiated,
5557
kErrored,
5658
kSourcePhase,
5759
throwIfPromiseRejected,
@@ -105,24 +107,6 @@ const { translators } = require('internal/modules/esm/translators');
105107
const { defaultResolve } = require('internal/modules/esm/resolve');
106108
const { defaultLoadSync, throwUnknownModuleFormat } = require('internal/modules/esm/load');
107109

108-
/**
109-
* Generate message about potential race condition caused by requiring a cached module that has started
110-
* async linking.
111-
* @param {string} filename Filename of the module being required.
112-
* @param {string|undefined} parentFilename Filename of the module calling require().
113-
* @param {boolean} isForAsyncLoaderHookWorker Whether this is for the async loader hook worker.
114-
* @returns {string} Error message.
115-
*/
116-
function getRaceMessage(filename, parentFilename, isForAsyncLoaderHookWorker) {
117-
let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
118-
raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
119-
raceMessage += 'import()-ed via Promise.all().\n';
120-
raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
121-
raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
122-
raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
123-
return raceMessage;
124-
}
125-
126110
/**
127111
* @typedef {import('../cjs/loader.js').Module} CJSModule
128112
*/
@@ -309,7 +293,7 @@ class ModuleLoader {
309293
const parentFilename = urlToFilename(parent?.filename);
310294
// This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
311295
if (!job.module) {
312-
assert.fail(getRaceMessage(filename, parentFilename), this.isForAsyncLoaderHookWorker);
296+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, this.isForAsyncLoaderHookWorker);
313297
}
314298
const status = job.module.getStatus();
315299
debug('Module status', job, status);
@@ -342,8 +326,8 @@ class ModuleLoader {
342326
throwIfPromiseRejected(job.instantiated);
343327
}
344328
if (status !== kEvaluating) {
345-
assert.fail(`Unexpected module status ${status}. ` +
346-
getRaceMessage(filename, parentFilename));
329+
assert(status === kUninstantiated, `Unexpected module status ${status}`);
330+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false);
347331
}
348332
let message = `Cannot require() ES Module ${filename} in a cycle.`;
349333
if (parentFilename) {
@@ -379,7 +363,7 @@ class ModuleLoader {
379363
#checkCachedJobForRequireESM(specifier, url, parentURL, job) {
380364
// This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
381365
if (!job.module) {
382-
assert.fail(getRaceMessage(url, parentURL, this.isForAsyncLoaderHookWorker));
366+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(url, parentURL, this.isForAsyncLoaderHookWorker);
383367
}
384368
// This module is being evaluated, which means it's imported in a previous link
385369
// in a cycle.

β€Žlib/internal/modules/esm/module_job.jsβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ const { getOptionValue } = require('internal/options');
5555
const noop = FunctionPrototype;
5656
const {
5757
ERR_REQUIRE_ASYNC_MODULE,
58+
ERR_REQUIRE_ESM_RACE_CONDITION,
5859
} = require('internal/errors').codes;
5960
let hasPausedEntry = false;
6061

@@ -420,7 +421,8 @@ class ModuleJob extends ModuleJobBase {
420421
// always handle CJS using the CJS loader to eliminate the quirks.
421422
return { __proto__: null, module: this.module, namespace: this.module.getNamespace() };
422423
}
423-
assert.fail(`Unexpected module status ${status}.`);
424+
assert(status === kUninstantiated, `Unexpected module status ${status}.`);
425+
throw new ERR_REQUIRE_ESM_RACE_CONDITION();
424426
}
425427

426428
async run(isEntryPoint = false) {
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
'use strict';
2+
require('../common');
3+
const fixtures = require('../common/fixtures');
4+
const assert = require('node:assert');
5+
6+
assert.throws(
7+
() => require(fixtures.path('import-require-cycle/race-condition.cjs')),
8+
{ code: 'ERR_REQUIRE_ESM_RACE_CONDITION' },
9+
);

β€Žtest/fixtures/import-require-cycle/node_modules/cjs-pkg/index.jsβ€Ž

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

β€Žtest/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjsβ€Ž

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

β€Žtest/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjsβ€Ž

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

β€Žtest/fixtures/import-require-cycle/node_modules/dual-pkg/package.jsonβ€Ž

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

β€Žtest/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjsβ€Ž

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
Β (0)