Skip to content

Commit 0d5cbdb

Browse files
committed
module: fix wrong error annotation for require of ESM
The error annotation (arrow message) for ERR_REQUIRE_ESM was pointing to internal frames (e.g. TracingChannel.traceSync in node:diagnostics_channel) instead of the user's require() call. Update reconstructErrorStack() to use a 3-tier frame selection: 1. Find the frame matching the parent module path 2. Fall back to the first non-internal user-land frame 3. Last resort: use the first 'at' frame Fixes: #55350 PR-URL: #62685
1 parent d080801 commit 0d5cbdb

File tree

2 files changed

+95
-3
lines changed

2 files changed

+95
-3
lines changed

lib/internal/modules/cjs/loader.js

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1872,9 +1872,49 @@ function loadSource(mod, filename, formatFromNode) {
18721872
}
18731873

18741874
function reconstructErrorStack(err, parentPath, parentSource) {
1875-
const errLine = StringPrototypeSplit(
1876-
StringPrototypeSlice(err.stack, StringPrototypeIndexOf(
1877-
err.stack, ' at ')), '\n', 1)[0];
1875+
// Find the stack frame that matches the parent module path.
1876+
// We cannot simply use the first frame because internal wrappers
1877+
// like TracingChannel.traceSync may appear before the user's frame.
1878+
const stackLines = StringPrototypeSplit(err.stack, '\n');
1879+
let errLine;
1880+
for (let i = 0; i < stackLines.length; i++) {
1881+
if (StringPrototypeIndexOf(stackLines[i], parentPath) !== -1) {
1882+
errLine = stackLines[i];
1883+
break;
1884+
}
1885+
}
1886+
// Fallback: if no frame matched the parent path, prefer a user-land
1887+
// frame (skip node:internal/* and other node:-scheme frames) so the
1888+
// annotation points at user code, not internal wrappers.
1889+
if (errLine === undefined) {
1890+
for (let i = 0; i < stackLines.length; i++) {
1891+
const line = stackLines[i];
1892+
if (StringPrototypeIndexOf(line, ' at ') === -1) {
1893+
continue;
1894+
}
1895+
if (StringPrototypeIndexOf(line, 'node:internal/') !== -1) {
1896+
continue;
1897+
}
1898+
if (StringPrototypeIndexOf(line, '(node:') !== -1) {
1899+
continue;
1900+
}
1901+
errLine = line;
1902+
break;
1903+
}
1904+
}
1905+
// Last resort: if all frames are internal, use the first 'at' frame
1906+
// so the user still gets some error annotation rather than none.
1907+
if (errLine === undefined) {
1908+
for (let i = 0; i < stackLines.length; i++) {
1909+
if (StringPrototypeIndexOf(stackLines[i], ' at ') !== -1) {
1910+
errLine = stackLines[i];
1911+
break;
1912+
}
1913+
}
1914+
}
1915+
if (errLine === undefined) {
1916+
return;
1917+
}
18781918
const { 1: line, 2: col } =
18791919
RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine) || [];
18801920
if (line && col) {
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
'use strict';
2+
3+
// This test verifies that when a CommonJS module requires an ES module,
4+
// the error annotation (arrow message) points to the user's require()
5+
// call in their source file, not to an internal frame such as
6+
// TracingChannel.traceSync in node:diagnostics_channel.
7+
// Regression test for https://github.com/nodejs/node/issues/55350.
8+
9+
const { spawnPromisified } = require('../common');
10+
const fixtures = require('../common/fixtures.js');
11+
const assert = require('node:assert');
12+
const path = require('node:path');
13+
const { execPath } = require('node:process');
14+
const { describe, it } = require('node:test');
15+
16+
const requiringEsm = path.resolve(
17+
fixtures.path('/es-modules/cjs-esm-esm.js')
18+
);
19+
20+
describe('ERR_REQUIRE_ESM error annotation', { concurrency: !process.env.TEST_PARALLEL }, () => {
21+
it('should point to the user require() call, not internal frames', async () => {
22+
const { code, stderr } = await spawnPromisified(execPath, [
23+
'--no-experimental-require-module', requiringEsm,
24+
]);
25+
26+
assert.strictEqual(code, 1);
27+
28+
const stderrStr = stderr.replaceAll('\r', '');
29+
30+
// The error annotation should reference the user's file, not
31+
// node:diagnostics_channel or any other internal module.
32+
assert.ok(
33+
stderrStr.includes(requiringEsm),
34+
`Expected error output to reference ${requiringEsm}, got:\n${stderrStr}`
35+
);
36+
37+
// The error annotation line should contain the path to the requiring
38+
// file. Do not assume it is the very first stderr line — internal
39+
// throw-location lines may precede the arrow annotation.
40+
const lines = stderrStr.split('\n');
41+
const fileAnnotationLine = lines.find((l) => l.includes(requiringEsm));
42+
assert.ok(
43+
fileAnnotationLine,
44+
`Expected an annotation line referencing the requiring file, got:\n` +
45+
lines.slice(0, 10).join('\n')
46+
);
47+
assert.ok(
48+
!fileAnnotationLine.includes('diagnostics_channel'),
49+
`Annotation line should not reference diagnostics_channel, got: "${fileAnnotationLine}"`
50+
);
51+
});
52+
});

0 commit comments

Comments
 (0)