Skip to content

Commit cbac135

Browse files
authored
diagnostics_channel: fix default callback context
traceCallback() defaults to a frozen context but writes the callback's result or error to it. Allocate a fresh context when undefined is passed and subscribers are present, matching traceSync() and tracePromise(). Set the prototype after allocation to avoid dictionary-mode properties. Assisted-by: pi Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #66481 Reviewed-By: Bryan English <bryan@bryanenglish.com>
1 parent 2e9d43c commit cbac135

2 files changed

Lines changed: 61 additions & 1 deletion

File tree

‎lib/diagnostics_channel.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,11 +647,15 @@ class TracingChannel {
647647
}
648648
}
649649

650-
traceCallback(fn, position = -1, context = kEmptyObject, thisArg, ...args) {
650+
traceCallback(fn, position = -1, context = undefined, thisArg, ...args) {
651651
if (!this.hasSubscribers) {
652652
return ReflectApply(fn, thisArg, args);
653653
}
654654

655+
if (context === undefined) {
656+
context = ObjectSetPrototypeOf({}, null);
657+
}
658+
655659
const { error } = this;
656660
const continuationWindow = this.#continuationWindow;
657661

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const dc = require('diagnostics_channel');
6+
7+
for (const expectedError of [null, new Error('test')]) {
8+
const channel = dc.tracingChannel(`test-${expectedError ? 'error' : 'result'}`);
9+
const contexts = [];
10+
const results = [{ value: 1 }, { value: 2 }];
11+
let completed = 0;
12+
13+
channel.subscribe({
14+
start: common.mustCall((context) => {
15+
assert.strictEqual(context.result, undefined);
16+
assert.strictEqual(context.error, undefined);
17+
for (const previous of contexts) {
18+
assert.notStrictEqual(context, previous);
19+
}
20+
contexts.push(context);
21+
}, 2),
22+
asyncStart: common.mustCall((context) => {
23+
assert.strictEqual(context, contexts[completed]);
24+
if (expectedError) {
25+
assert.strictEqual(context.error, expectedError);
26+
assert.strictEqual(context.result, undefined);
27+
} else {
28+
assert.strictEqual(context.result, results[completed]);
29+
assert.strictEqual(context.error, undefined);
30+
}
31+
completed++;
32+
}, 2),
33+
asyncEnd: common.mustCall(2),
34+
error: expectedError ? common.mustCall((context) => {
35+
assert.strictEqual(context, contexts[completed]);
36+
assert.strictEqual(context.error, expectedError);
37+
}, 2) : common.mustNotCall(),
38+
});
39+
40+
for (const result of results) {
41+
channel.traceCallback(common.mustCall((callback) => {
42+
setImmediate(callback, expectedError, expectedError ? undefined : result);
43+
}), undefined, undefined, undefined, common.mustCall((err, value) => {
44+
assert.strictEqual(err, expectedError);
45+
assert.strictEqual(value, expectedError ? undefined : result);
46+
}));
47+
}
48+
49+
setImmediate(common.mustCall(() => {
50+
assert.strictEqual(completed, 2);
51+
for (const [index, context] of contexts.entries()) {
52+
assert.strictEqual(context.error, expectedError || undefined);
53+
assert.strictEqual(context.result, expectedError ? undefined : results[index]);
54+
}
55+
}));
56+
}

0 commit comments

Comments
 (0)