Skip to content

Commit 3cb46cf

Browse files
committed
lib: fix assert shows diff messages in ESM and CJS
This PR addresses an issue which was caused by the design in the ESM loader. The ESM loader was modifying the file path and replacing the 'file' property with the file proto in the stack trace. This, in turn, led to unhandled exceptions when the assert module attempted to open the file to display erroneous code. The changes in this PR resolve this issue by handling the file path correctly, ensuring that the remaining message formatting code can execute as expected.
1 parent 33704c4 commit 3cb46cf

File tree

2 files changed

+65
-1
lines changed

2 files changed

+65
-1
lines changed

lib/assert.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ const CallTracker = require('internal/assert/calltracker');
7373
const {
7474
validateFunction,
7575
} = require('internal/validators');
76+
const { fileURLToPath } = require('internal/url');
7677

7778
let isDeepEqual;
7879
let isDeepStrictEqual;
@@ -296,7 +297,7 @@ function getErrMessage(message, fn) {
296297
overrideStackTrace.set(err, (_, stack) => stack);
297298
const call = err.stack[0];
298299

299-
const filename = call.getFileName();
300+
let filename = call.getFileName();
300301
const line = call.getLineNumber() - 1;
301302
let column = call.getColumnNumber() - 1;
302303
let identifier;
@@ -330,6 +331,14 @@ function getErrMessage(message, fn) {
330331
const { StringDecoder } = require('string_decoder');
331332
decoder = new StringDecoder('utf8');
332333
}
334+
335+
// ESM file prop is a file proto. Convert that to path.
336+
// This ensure opensync will not throw ENOENT for ESM files.
337+
const fileProtoPrefix = 'file://';
338+
if (StringPrototypeStartsWith(filename, fileProtoPrefix)) {
339+
filename = fileURLToPath(filename);
340+
}
341+
333342
fd = openSync(filename, 'r', 0o666);
334343
// Reset column and message.
335344
({ 0: column, 1: message } = getCode(fd, line, column));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
3+
require('../common');
4+
const tmpdir = require('../common/tmpdir');
5+
const assert = require('assert');
6+
const { writeFileSync, unlink } = require('fs');
7+
const { execSync } = require('child_process');
8+
9+
tmpdir.refresh();
10+
11+
// To ensure the assert ok throwing similar error messages for esm and cjs
12+
13+
const nodejs = `${process.execPath}`;
14+
15+
// Importing package for each extensions.
16+
const fileImports = {
17+
mjs: 'import assert from "assert";',
18+
js: 'const assert = require("assert");',
19+
};
20+
21+
const fileNames = [];
22+
23+
for (const [ext, header] of Object.entries(fileImports)) {
24+
const fileName = `test-file.${ext}`;
25+
// Store the generated filesnames in an array
26+
fileNames.push(`${tmpdir.path}/${fileName}`);
27+
28+
writeFileSync(tmpdir.resolve(fileName), `${header}\nassert.ok(0 === 2);`);
29+
}
30+
31+
const errorsMessages = [];
32+
33+
for (const fileName of fileNames) {
34+
const command = `${nodejs} ${fileName}`;
35+
assert.throws(
36+
() => {
37+
execSync(command, { stdio: 'pipe' });
38+
},
39+
(e) => {
40+
// For each error message, filter the lines which will starts with AssertionError
41+
errorsMessages.push(
42+
e.message.split('\n').find((s) => s.startsWith('AssertionError'))
43+
);
44+
return true;
45+
}
46+
);
47+
}
48+
49+
assert.deepStrictEqual(errorsMessages[0], errorsMessages[1]);
50+
51+
for (const fileName of fileNames) {
52+
unlink(fileName, () => {});
53+
}
54+
55+
tmpdir.refresh();

0 commit comments

Comments
 (0)