From 9cda6cfbd858e3e7b4c2c878c0dcf23146f704f9 Mon Sep 17 00:00:00 2001 From: Edy Silva Date: Sat, 30 Nov 2024 01:56:34 -0300 Subject: [PATCH 1/2] module: fix error reporting when commonjs requires an ES module --- lib/internal/modules/cjs/loader.js | 2 ++ .../require-esm-error-annotation/app.js | 1 + .../require-esm-error-annotation/index.cjs | 11 +++++++ .../require-esm-error-annotation/package.json | 3 ++ ...sm-with-no-experimental-require-module.mjs | 29 +++++++++++++++++++ 5 files changed, 46 insertions(+) create mode 100644 test/fixtures/es-modules/package-type-module/require-esm-error-annotation/app.js create mode 100644 test/fixtures/es-modules/package-type-module/require-esm-error-annotation/index.cjs create mode 100644 test/fixtures/es-modules/package-type-module/require-esm-error-annotation/package.json create mode 100644 test/parallel/test-require-esm-with-no-experimental-require-module.mjs diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index f99d0fc2a7a0eb..5027ecc71cd5a9 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -36,6 +36,7 @@ const { ArrayPrototypeUnshiftApply, Boolean, Error, + ErrorCaptureStackTrace, JSONParse, ObjectDefineProperty, ObjectFreeze, @@ -1674,6 +1675,7 @@ function getRequireESMError(mod, pkg, content, filename) { const usesEsm = containsModuleSyntax(content, filename); const err = new ERR_REQUIRE_ESM(filename, usesEsm, parentPath, packageJsonPath); + ErrorCaptureStackTrace(err, Module.prototype.require); // Attempt to reconstruct the parent require frame. const parentModule = Module._cache[parentPath]; if (parentModule) { diff --git a/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/app.js b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/app.js new file mode 100644 index 00000000000000..72447a44e7ab59 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/app.js @@ -0,0 +1 @@ +import nothing from 'somewhere'; diff --git a/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/index.cjs b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/index.cjs new file mode 100644 index 00000000000000..f26642639c06c6 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/index.cjs @@ -0,0 +1,11 @@ +function main() { + func1(func2(func3())) +} + +function func1() { + require('./app.js') +} +function func2() {} +function func3() {} + +main() diff --git a/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/package.json b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/package.json new file mode 100644 index 00000000000000..3dbc1ca591c055 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/require-esm-error-annotation/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/test/parallel/test-require-esm-with-no-experimental-require-module.mjs b/test/parallel/test-require-esm-with-no-experimental-require-module.mjs new file mode 100644 index 00000000000000..43177eed20fcf9 --- /dev/null +++ b/test/parallel/test-require-esm-with-no-experimental-require-module.mjs @@ -0,0 +1,29 @@ +import '../common/index.mjs'; +import { test } from 'node:test'; +import * as fixtures from '../common/fixtures.mjs'; +import { spawnSync } from 'node:child_process'; +import assert from 'node:assert'; +import { EOL } from 'node:os'; + +test('correctly reports error for a longer stack trace', () => { + // The following regex matches the error message that is expected to be thrown + // + // package-type-module/require-esm-error-annotation/longer-stack.cjs:6 + // require('./app.js') + // ^ + + const fixture = fixtures.path('es-modules/package-type-module/require-esm-error-annotation/index.cjs'); + const args = ['--no-experimental-require-module', fixture]; + + const lineNumber = 6; + const lineContent = "require('./app.js')"; + const fullMessage = `${fixture}:${lineNumber}${EOL} ${lineContent}${EOL} ^${EOL}`; + + const result = spawnSync(process.execPath, args); + + console.log(`STDERR: ${result.stderr.toString()}`); + + assert.strictEqual(result.status, 1); + assert.strictEqual(result.stdout.toString(), ''); + assert(result.stderr.toString().indexOf(fullMessage) > -1); +}); From 7a97799b370f5c5d6eea58bb486ac8dc1cc96854 Mon Sep 17 00:00:00 2001 From: Edy Silva Date: Sat, 30 Nov 2024 02:19:06 -0300 Subject: [PATCH 2/2] test: improve matching --- ...uire-esm-with-no-experimental-require-module.mjs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/parallel/test-require-esm-with-no-experimental-require-module.mjs b/test/parallel/test-require-esm-with-no-experimental-require-module.mjs index 43177eed20fcf9..9f59c81f161140 100644 --- a/test/parallel/test-require-esm-with-no-experimental-require-module.mjs +++ b/test/parallel/test-require-esm-with-no-experimental-require-module.mjs @@ -3,27 +3,22 @@ import { test } from 'node:test'; import * as fixtures from '../common/fixtures.mjs'; import { spawnSync } from 'node:child_process'; import assert from 'node:assert'; -import { EOL } from 'node:os'; test('correctly reports error for a longer stack trace', () => { // The following regex matches the error message that is expected to be thrown // - // package-type-module/require-esm-error-annotation/longer-stack.cjs:6 + // package-type-module/require-esm-error-annotation/index.cjs:6 // require('./app.js') // ^ const fixture = fixtures.path('es-modules/package-type-module/require-esm-error-annotation/index.cjs'); const args = ['--no-experimental-require-module', fixture]; - - const lineNumber = 6; - const lineContent = "require('./app.js')"; - const fullMessage = `${fixture}:${lineNumber}${EOL} ${lineContent}${EOL} ^${EOL}`; + const regex = /index\.cjs:6[\n\r]+\s{2}require\('\.\/app\.js'\)[\n\r]+\s{2}\^/; const result = spawnSync(process.execPath, args); - - console.log(`STDERR: ${result.stderr.toString()}`); + const stderr = result.stderr.toString(); assert.strictEqual(result.status, 1); assert.strictEqual(result.stdout.toString(), ''); - assert(result.stderr.toString().indexOf(fullMessage) > -1); + assert.match(stderr, regex); });