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 ddf1f01

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 a932fbd commit ddf1f01
Copy full SHA for ddf1f01

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
@@ -2699,6 +2699,19 @@ This error has been deprecated since `require()` now supports loading synchronou
26992699
ES modules. When `require()` encounters an ES module that contains top-level
27002700
`await`, it will throw [`ERR_REQUIRE_ASYNC_MODULE`][] instead.
27012701

2702+
<a id="ERR_REQUIRE_ESM_RACE_CONDITION"></a>
2703+
2704+
### `ERR_REQUIRE_ESM_RACE_CONDITION`
2705+
2706+
<!-- YAML
2707+
added: REPLACEME
2708+
-->
2709+
2710+
> Stability: 1 - Experimental.
2711+
2712+
An attempt was made to `require()` an [ES Module][] while another `import()` call
2713+
was already in progress to load it asynchronously.
2714+
27022715
<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
27032716

27042717
### `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
@@ -1717,6 +1717,15 @@ E('ERR_REQUIRE_ESM',
17171717
'all ES modules instead).\n';
17181718
return msg;
17191719
}, Error);
1720+
E('ERR_REQUIRE_ESM_RACE_CONDITION', (filename, parentFilename, isForAsyncLoaderHookWorker) => {
1721+
let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
1722+
raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
1723+
raceMessage += 'import()-ed via Promise.all().\n';
1724+
raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
1725+
raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
1726+
raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
1727+
return raceMessage;
1728+
}, Error);
17201729
E('ERR_SCRIPT_EXECUTION_INTERRUPTED',
17211730
'Script execution was interrupted by `SIGINT`', Error);
17221731
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');
@@ -48,6 +49,7 @@ const {
4849
kEvaluating,
4950
kEvaluationPhase,
5051
kInstantiated,
52+
kUninstantiated,
5153
kErrored,
5254
kSourcePhase,
5355
throwIfPromiseRejected,
@@ -101,24 +103,6 @@ const { translators } = require('internal/modules/esm/translators');
101103
const { defaultResolve } = require('internal/modules/esm/resolve');
102104
const { defaultLoadSync, throwUnknownModuleFormat } = require('internal/modules/esm/load');
103105

104-
/**
105-
* Generate message about potential race condition caused by requiring a cached module that has started
106-
* async linking.
107-
* @param {string} filename Filename of the module being required.
108-
* @param {string|undefined} parentFilename Filename of the module calling require().
109-
* @param {boolean} isForAsyncLoaderHookWorker Whether this is for the async loader hook worker.
110-
* @returns {string} Error message.
111-
*/
112-
function getRaceMessage(filename, parentFilename, isForAsyncLoaderHookWorker) {
113-
let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
114-
raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
115-
raceMessage += 'import()-ed via Promise.all().\n';
116-
raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
117-
raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
118-
raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
119-
return raceMessage;
120-
}
121-
122106
/**
123107
* @typedef {import('../cjs/loader.js').Module} CJSModule
124108
*/
@@ -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.
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.