Skip to content

Commit ddf1f01

Browse 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

12 files changed

Lines changed: 59 additions & 23 deletions

File tree

‎doc/api/errors.md‎

Lines changed: 13 additions & 0 deletions
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`

‎lib/internal/errors.js‎

Lines 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',

‎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');
@@ -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.

‎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) {

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

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)