Skip to content

Commit b8bb1f0

Browse files
committed
esm: add ERR_REQUIRE_ESM_RACE_CONDITION
1 parent 53bcd11 commit b8bb1f0

File tree

12 files changed

+57
-23
lines changed

12 files changed

+57
-23
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2707,6 +2707,17 @@ 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+
An attempt was made to `require()` an [ES Module][] while another `import()` call
2719+
was already in progress to load it asynchronously.
2720+
27102721
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
27112722

27122723
### `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');
@@ -49,6 +50,7 @@ const {
4950
kEvaluating,
5051
kEvaluationPhase,
5152
kInstantiated,
53+
kUninstantiated,
5254
kErrored,
5355
kSourcePhase,
5456
throwIfPromiseRejected,
@@ -102,24 +104,6 @@ const { translators } = require('internal/modules/esm/translators');
102104
const { defaultResolve } = require('internal/modules/esm/resolve');
103105
const { defaultLoadSync, throwUnknownModuleFormat } = require('internal/modules/esm/load');
104106

105-
/**
106-
* Generate message about potential race condition caused by requiring a cached module that has started
107-
* async linking.
108-
* @param {string} filename Filename of the module being required.
109-
* @param {string|undefined} parentFilename Filename of the module calling require().
110-
* @param {boolean} isForAsyncLoaderHookWorker Whether this is for the async loader hook worker.
111-
* @returns {string} Error message.
112-
*/
113-
function getRaceMessage(filename, parentFilename, isForAsyncLoaderHookWorker) {
114-
let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
115-
raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
116-
raceMessage += 'import()-ed via Promise.all().\n';
117-
raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
118-
raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
119-
raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
120-
return raceMessage;
121-
}
122-
123107
/**
124108
* @typedef {import('../cjs/loader.js').Module} CJSModule
125109
*/
@@ -306,7 +290,7 @@ class ModuleLoader {
306290
const parentFilename = urlToFilename(parent?.filename);
307291
// This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
308292
if (!job.module) {
309-
assert.fail(getRaceMessage(filename, parentFilename), this.isForAsyncLoaderHookWorker);
293+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, this.isForAsyncLoaderHookWorker);
310294
}
311295
const status = job.module.getStatus();
312296
debug('Module status', job, status);
@@ -339,8 +323,8 @@ class ModuleLoader {
339323
throwIfPromiseRejected(job.instantiated);
340324
}
341325
if (status !== kEvaluating) {
342-
assert.fail(`Unexpected module status ${status}. ` +
343-
getRaceMessage(filename, parentFilename));
326+
assert(status === kUninstantiated, `Unexpected module status ${status}`);
327+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(filename, parentFilename, false);
344328
}
345329
let message = `Cannot require() ES Module ${filename} in a cycle.`;
346330
if (parentFilename) {
@@ -376,7 +360,7 @@ class ModuleLoader {
376360
#checkCachedJobForRequireESM(specifier, url, parentURL, job) {
377361
// This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
378362
if (!job.module) {
379-
assert.fail(getRaceMessage(url, parentURL, this.isForAsyncLoaderHookWorker));
363+
throw new ERR_REQUIRE_ESM_RACE_CONDITION(url, parentURL, this.isForAsyncLoaderHookWorker);
380364
}
381365
// This module is being evaluated, which means it's imported in a previous link
382366
// 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)