Skip to content

Commit c60c266

Browse files
committed
test_runner: preserve AssertionError prototype names across isolation
1 parent 4e9877f commit c60c266

File tree

7 files changed

+369
-0
lines changed

7 files changed

+369
-0
lines changed
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
'use strict';
2+
3+
// test_runner-only helpers used to preserve AssertionError actual/expected
4+
// constructor names across process isolation boundaries.
5+
6+
const {
7+
ArrayIsArray,
8+
ArrayPrototype,
9+
ObjectDefineProperty,
10+
ObjectGetOwnPropertyDescriptor,
11+
ObjectGetPrototypeOf,
12+
ObjectPrototype,
13+
ObjectPrototypeToString,
14+
ObjectSetPrototypeOf,
15+
} = primordials;
16+
17+
const kAssertionErrorCode = 'ERR_ASSERTION';
18+
const kTestFailureErrorCode = 'ERR_TEST_FAILURE';
19+
const kBaseTypeArray = 'array';
20+
const kBaseTypeObject = 'object';
21+
// Internal key used on test_runner item details during transport.
22+
const kAssertionPrototypeMetadata = 'assertionPrototypeMetadata';
23+
24+
function getName(object) {
25+
const desc = ObjectGetOwnPropertyDescriptor(object, 'name');
26+
return desc?.value;
27+
}
28+
29+
function getAssertionError(error) {
30+
if (error === null || typeof error !== 'object') {
31+
return;
32+
}
33+
34+
if (error.code === kTestFailureErrorCode) {
35+
return error.cause;
36+
}
37+
38+
return error;
39+
}
40+
41+
function getAssertionPrototype(value) {
42+
if (value === null || typeof value !== 'object') {
43+
return;
44+
}
45+
46+
const prototype = ObjectGetPrototypeOf(value);
47+
if (prototype === null) {
48+
return;
49+
}
50+
51+
const constructor = ObjectGetOwnPropertyDescriptor(prototype, 'constructor')?.value;
52+
if (typeof constructor !== 'function') {
53+
return;
54+
}
55+
56+
const constructorName = getName(constructor);
57+
if (typeof constructorName !== 'string' || constructorName.length === 0) {
58+
return;
59+
}
60+
61+
// Keep the scope narrow for this regression fix: only Array/Object values
62+
// are currently restored for AssertionError actual/expected.
63+
if (ArrayIsArray(value)) {
64+
if (constructorName === 'Array') {
65+
return;
66+
}
67+
68+
return {
69+
__proto__: null,
70+
baseType: kBaseTypeArray,
71+
constructorName,
72+
};
73+
}
74+
75+
if (ObjectPrototypeToString(value) === '[object Object]') {
76+
if (constructorName === 'Object') {
77+
return;
78+
}
79+
80+
return {
81+
__proto__: null,
82+
baseType: kBaseTypeObject,
83+
constructorName,
84+
};
85+
}
86+
}
87+
88+
function createSyntheticConstructor(name) {
89+
function constructor() {}
90+
ObjectDefineProperty(constructor, 'name', {
91+
__proto__: null,
92+
value: name,
93+
configurable: true,
94+
});
95+
return constructor;
96+
}
97+
98+
function collectAssertionPrototypeMetadata(error) {
99+
const assertionError = getAssertionError(error);
100+
if (assertionError === null || typeof assertionError !== 'object' ||
101+
assertionError.code !== kAssertionErrorCode) {
102+
return;
103+
}
104+
105+
const actual = getAssertionPrototype(assertionError.actual);
106+
const expected = getAssertionPrototype(assertionError.expected);
107+
if (!actual && !expected) {
108+
return;
109+
}
110+
111+
return {
112+
__proto__: null,
113+
actual,
114+
expected,
115+
};
116+
}
117+
118+
function applyAssertionPrototypeMetadata(error, metadata) {
119+
if (metadata === undefined || metadata === null || typeof metadata !== 'object') {
120+
return;
121+
}
122+
123+
const assertionError = getAssertionError(error);
124+
if (assertionError === null || typeof assertionError !== 'object' ||
125+
assertionError.code !== kAssertionErrorCode) {
126+
return;
127+
}
128+
129+
for (const key of ['actual', 'expected']) {
130+
const meta = metadata[key];
131+
const value = assertionError[key];
132+
const constructorName = meta?.constructorName;
133+
134+
if (meta === undefined || meta === null || typeof meta !== 'object' ||
135+
value === null || typeof value !== 'object' ||
136+
typeof constructorName !== 'string') {
137+
continue;
138+
}
139+
140+
if (meta.baseType === kBaseTypeArray && !ArrayIsArray(value)) {
141+
continue;
142+
}
143+
144+
if (meta.baseType === kBaseTypeObject &&
145+
ObjectPrototypeToString(value) !== '[object Object]') {
146+
continue;
147+
}
148+
149+
if (meta.baseType !== kBaseTypeArray && meta.baseType !== kBaseTypeObject) {
150+
continue;
151+
}
152+
153+
const currentPrototype = ObjectGetPrototypeOf(value);
154+
const currentConstructor = currentPrototype === null ? undefined :
155+
ObjectGetOwnPropertyDescriptor(currentPrototype, 'constructor')?.value;
156+
if (typeof currentConstructor === 'function' &&
157+
getName(currentConstructor) === constructorName) {
158+
continue;
159+
}
160+
161+
const basePrototype = meta.baseType === kBaseTypeArray ?
162+
ArrayPrototype :
163+
ObjectPrototype;
164+
165+
try {
166+
const constructor = createSyntheticConstructor(constructorName);
167+
const syntheticPrototype = { __proto__: basePrototype };
168+
ObjectDefineProperty(syntheticPrototype, 'constructor', {
169+
__proto__: null,
170+
value: constructor,
171+
writable: true,
172+
enumerable: false,
173+
configurable: true,
174+
});
175+
constructor.prototype = syntheticPrototype;
176+
ObjectSetPrototypeOf(value, syntheticPrototype);
177+
} catch {
178+
// Best-effort only. If prototype restoration fails, keep the
179+
// deserialized value as-is and continue reporting.
180+
}
181+
}
182+
}
183+
184+
module.exports = {
185+
applyAssertionPrototypeMetadata,
186+
collectAssertionPrototypeMetadata,
187+
kAssertionPrototypeMetadata,
188+
};

lib/internal/test_runner/reporter/v8-serializer.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ const {
66
const { DefaultSerializer } = require('v8');
77
const { Buffer } = require('buffer');
88
const { serializeError } = require('internal/error_serdes');
9+
const {
10+
collectAssertionPrototypeMetadata,
11+
kAssertionPrototypeMetadata,
12+
} = require('internal/test_runner/assertion_error_prototype');
913

1014

1115
module.exports = async function* v8Reporter(source) {
@@ -15,7 +19,15 @@ module.exports = async function* v8Reporter(source) {
1519

1620
for await (const item of source) {
1721
const originalError = item.data.details?.error;
22+
let assertionPrototypeMetadata;
1823
if (originalError) {
24+
assertionPrototypeMetadata = collectAssertionPrototypeMetadata(originalError);
25+
if (assertionPrototypeMetadata !== undefined) {
26+
// test_runner-only metadata used by the parent process to restore
27+
// AssertionError actual/expected constructor names.
28+
item.data.details[kAssertionPrototypeMetadata] = assertionPrototypeMetadata;
29+
}
30+
1931
// Error is overridden with a serialized version, so that it can be
2032
// deserialized in the parent process.
2133
// Error is restored after serialization.
@@ -29,6 +41,9 @@ module.exports = async function* v8Reporter(source) {
2941

3042
if (originalError) {
3143
item.data.details.error = originalError;
44+
if (assertionPrototypeMetadata !== undefined) {
45+
delete item.data.details[kAssertionPrototypeMetadata];
46+
}
3247
}
3348

3449
const serializedMessage = serializer.releaseBuffer();

lib/internal/test_runner/runner.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ const { DefaultDeserializer, DefaultSerializer } = require('v8');
3838
const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options');
3939
const { Interface } = require('internal/readline/interface');
4040
const { deserializeError } = require('internal/error_serdes');
41+
const {
42+
applyAssertionPrototypeMetadata,
43+
kAssertionPrototypeMetadata,
44+
} = require('internal/test_runner/assertion_error_prototype');
4145
const { Buffer } = require('buffer');
4246
const { FilesWatcher } = require('internal/watch_mode/files_watcher');
4347
const console = require('internal/console/global');
@@ -253,6 +257,15 @@ class FileTest extends Test {
253257
}
254258
if (item.data.details?.error) {
255259
item.data.details.error = deserializeError(item.data.details.error);
260+
applyAssertionPrototypeMetadata(
261+
item.data.details.error,
262+
item.data.details[kAssertionPrototypeMetadata],
263+
);
264+
}
265+
// Metadata is test_runner-internal and must not leak to downstream
266+
// reporters regardless of whether restoration ran.
267+
if (item.data.details?.[kAssertionPrototypeMetadata] !== undefined) {
268+
delete item.data.details[kAssertionPrototypeMetadata];
256269
}
257270
if (item.type === 'test:pass' || item.type === 'test:fail') {
258271
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
3+
const assert = require('node:assert');
4+
const { test } = require('node:test');
5+
6+
class ExtendedArray extends Array {}
7+
8+
test('assertion error preserves prototype name', () => {
9+
assert.deepStrictEqual(new ExtendedArray('hello'), ['hello']);
10+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use strict';
2+
3+
// Regression test for https://github.com/nodejs/node/issues/50397:
4+
// ensure --test preserves AssertionError actual type across isolation modes.
5+
6+
require('../common');
7+
const { spawnSyncAndAssert } = require('../common/child_process');
8+
const assert = require('node:assert');
9+
const fixtures = require('../common/fixtures');
10+
11+
for (const isolation of ['none', 'process']) {
12+
const args = [
13+
'--test',
14+
'--test-reporter=spec',
15+
`--test-isolation=${isolation}`,
16+
fixtures.path('test-runner/issue-50397/prototype-mismatch.js'),
17+
];
18+
spawnSyncAndAssert(process.execPath, args, {
19+
status: 1,
20+
signal: null,
21+
stderr: '',
22+
stdout(output) {
23+
// Spec reporter output varies between inspect forms; accept both while
24+
// still requiring the restored constructor name.
25+
assert.match(output, /actual:\s+(?:\[ExtendedArray\]|ExtendedArray\(1\)\s+\[\s*'hello'\s*\])/);
26+
assert.doesNotMatch(output, /actual:\s+\[Array\]/);
27+
},
28+
});
29+
}

test/parallel/test-runner-v8-deserializer.mjs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { DefaultSerializer } from 'node:v8';
88
import serializer from 'internal/test_runner/reporter/v8-serializer';
99
import runner from 'internal/test_runner/runner';
1010

11+
class ExtendedArray extends Array {}
12+
1113
async function toArray(chunks) {
1214
const arr = [];
1315
for await (const i of chunks) arr.push(i);
@@ -77,6 +79,26 @@ describe('v8 deserializer', common.mustCall(() => {
7779
]);
7880
});
7981

82+
it('should restore assertion metadata without leaking internal transport fields', async () => {
83+
let assertionError;
84+
try {
85+
assert.deepStrictEqual(new ExtendedArray('hello'), ['hello']);
86+
} catch (error) {
87+
assertionError = error;
88+
}
89+
assert(assertionError);
90+
91+
const [chunk] = await toArray(serializer([{
92+
type: 'test:diagnostic',
93+
data: { nesting: 0, details: { error: assertionError }, message: 'diagnostic' },
94+
}]));
95+
96+
const reported = await collectReported([chunk]);
97+
const detailError = reported[0].data.details.error;
98+
assert.strictEqual(detailError.actual.constructor.name, 'ExtendedArray');
99+
assert.strictEqual(Object.hasOwn(reported[0].data.details, 'assertionPrototypeMetadata'), false);
100+
});
101+
80102
const headerPosition = headerLength * 2 + 4;
81103
for (let i = 0; i < headerPosition + 5; i++) {
82104
const message = `should deserialize a serialized message split into two chunks {...${i},${i + 1}...}`;

0 commit comments

Comments
 (0)