Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3d5cf17

Browse filesBrowse files
aduh95aduh95
committed
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 9e19f55 commit 3d5cf17
Copy full SHA for 3d5cf17

12 files changed

+59-23Lines changed: 59 additions & 23 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+13Lines changed: 13 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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`
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+9Lines 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',
Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+6-22Lines 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.
Collapse file

‎lib/internal/modules/esm/module_job.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js
+3-1Lines 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) {
Collapse file

‎test/es-module/test-esm-require-race-condition.js‎

Copy file name to clipboard
+9Lines 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+
);
Collapse file

‎test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js‎

Copy file name to clipboardExpand all lines: test/fixtures/import-require-cycle/node_modules/cjs-pkg/index.js
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs‎

Copy file name to clipboardExpand all lines: test/fixtures/import-require-cycle/node_modules/dual-pkg/index.cjs
+2Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs‎

Copy file name to clipboardExpand all lines: test/fixtures/import-require-cycle/node_modules/dual-pkg/index.mjs
+1Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json‎

Copy file name to clipboardExpand all lines: test/fixtures/import-require-cycle/node_modules/dual-pkg/package.json
+7Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

‎test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs‎

Copy file name to clipboardExpand all lines: test/fixtures/import-require-cycle/node_modules/esm-pkg/index.mjs
+1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.