Skip to content

Commit 8dd31df

Browse files
committed
module: fix require(esm) error annotation with tracing subscriber
reconstructErrorStack() took the first stack frame as the caller, but with a module.require tracing subscriber that frame is TracingChannel.traceSync, so the arrow pointed at a nonexistent line of the parent file. Pick the first frame that belongs to the parent. Fixes: #55350
1 parent 14ed05f commit 8dd31df

2 files changed

Lines changed: 28 additions & 3 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
const {
2525
ArrayIsArray,
2626
ArrayPrototypeFilter,
27+
ArrayPrototypeFind,
2728
ArrayPrototypeIncludes,
2829
ArrayPrototypeIndexOf,
2930
ArrayPrototypeJoin,
@@ -56,6 +57,7 @@ const {
5657
StringPrototypeCharAt,
5758
StringPrototypeCharCodeAt,
5859
StringPrototypeEndsWith,
60+
StringPrototypeIncludes,
5961
StringPrototypeIndexOf,
6062
StringPrototypeRepeat,
6163
StringPrototypeSlice,
@@ -2011,11 +2013,14 @@ function loadSource(mod, filename, formatFromNode) {
20112013
}
20122014

20132015
function reconstructErrorStack(err, parentPath, parentSource) {
2014-
const errLine = StringPrototypeSplit(
2016+
const frames = StringPrototypeSplit(
20152017
StringPrototypeSlice(err.stack, StringPrototypeIndexOf(
2016-
err.stack, ' at ')), '\n', 1)[0];
2018+
err.stack, ' at ')), '\n');
2019+
// Wrappers such as TracingChannel.traceSync may precede the parent frame.
2020+
const errLine = ArrayPrototypeFind(
2021+
frames, (frame) => StringPrototypeIncludes(frame, parentPath));
20172022
const { 1: line, 2: col } =
2018-
RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine) || [];
2023+
RegExpPrototypeExec(/(\d+):(\d+)\)/, errLine ?? '') || [];
20192024
if (line && col) {
20202025
const srcLine = StringPrototypeSplit(parentSource, '\n', line)[line - 1];
20212026
const frame = `${parentPath}:${line}\n${srcLine}\n${StringPrototypeRepeat(' ', col - 1)}^\n`;

‎test/es-module/test-cjs-esm-warn.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,24 @@ describe('CJS ↔︎ ESM interop warnings', { concurrency: !process.env.TEST_PAR
7272
assert.strictEqual(code, 1);
7373
assert.strictEqual(signal, null);
7474
});
75+
76+
it('should annotate the parent frame when require() is traced', async () => {
77+
const { code, signal, stderr } = await spawnPromisified(execPath, [
78+
'--no-experimental-require-module',
79+
'--import',
80+
'data:text/javascript,' + encodeURIComponent(
81+
'import dc from "node:diagnostics_channel";' +
82+
'dc.subscribe("tracing:module.require:start", () => {});'),
83+
requiringEsm,
84+
]);
85+
86+
assert.ok(
87+
stderr.replace(/\r/g, '').startsWith(
88+
`${requiringEsm}:1\nrequire('./package-type-module/esm.js');\n^\n`
89+
)
90+
);
91+
92+
assert.strictEqual(code, 1);
93+
assert.strictEqual(signal, null);
94+
});
7595
});

0 commit comments

Comments
 (0)