Skip to content

Commit b0e835c

Browse files
committed
diagnostics_channel: return original thenable
This makes tracePromise return the original thenable to allow custom thenable types to retain their methods rather than producing the chained result type.
1 parent 5b6091c commit b0e835c

File tree

2 files changed

+27
-4
lines changed

2 files changed

+27
-4
lines changed

lib/diagnostics_channel.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ const {
1010
ObjectDefineProperty,
1111
ObjectGetPrototypeOf,
1212
ObjectSetPrototypeOf,
13+
Promise,
14+
PromisePrototypeThen,
1315
ReflectApply,
1416
SafeFinalizationRegistry,
1517
SafeMap,
@@ -26,6 +28,7 @@ const {
2628
} = require('internal/validators');
2729

2830
const { triggerUncaughtException } = internalBinding('errors');
31+
const { isPromise } = require('internal/util/types');
2932

3033
const dc_binding = internalBinding('diagnostics_channel');
3134
const { subscribers: subscriberCounts } = dc_binding;
@@ -369,16 +372,20 @@ class TracingChannel {
369372

370373
const { start, end, asyncStart, asyncEnd, error } = this;
371374

372-
function reject(err) {
375+
function onReject(err) {
373376
context.error = err;
374377
error.publish(context);
375378
asyncStart.publish(context);
376379
// TODO: Is there a way to have asyncEnd _after_ the continuation?
377380
asyncEnd.publish(context);
381+
}
382+
383+
function onRejectWithRethrow(err) {
384+
onReject(err);
378385
throw err;
379386
}
380387

381-
function resolve(result) {
388+
function onResolve(result) {
382389
context.result = result;
383390
asyncStart.publish(context);
384391
// TODO: Is there a way to have asyncEnd _after_ the continuation?
@@ -396,7 +403,17 @@ class TracingChannel {
396403
context.result = result;
397404
return result;
398405
}
399-
return result.then(resolve, reject);
406+
// isPromise() matches sub-classes, but we need to match only direct
407+
// instances of the native Promise type to safely use PromisePrototypeThen.
408+
if (isPromise(result) && result.constructor === Promise) {
409+
return PromisePrototypeThen(result, onResolve, onRejectWithRethrow);
410+
}
411+
// For non-native thenables, subscribe to the result but return the
412+
// original thenable so the consumer can continue handling it directly.
413+
// Non-native thenables don't have unhandledRejection tracking, so
414+
// swallowing the rejection here doesn't change existing behaviour.
415+
result.then(onResolve, onReject);
416+
return result;
400417
} catch (err) {
401418
context.error = err;
402419
error.publish(context);

test/parallel/test-diagnostics-channel-tracing-channel-promise-thenable.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ class ResolvedThenable {
1212
then(resolve) {
1313
return new ResolvedThenable(resolve(this.#result));
1414
}
15+
customMethod() {
16+
return this.#result;
17+
}
1518
}
1619

1720
const channel = dc.tracingChannel('test');
@@ -49,7 +52,10 @@ const result = channel.tracePromise(common.mustCall(function(value) {
4952
}), input, thisArg, expectedResult);
5053

5154
assert(result instanceof ResolvedThenable);
52-
assert.notStrictEqual(result, innerThenable);
55+
// With branching then, the original thenable is returned directly so that
56+
// extra methods defined on it remain accessible to the caller.
57+
assert.strictEqual(result, innerThenable);
58+
assert.deepStrictEqual(result.customMethod(), expectedResult);
5359
result.then(common.mustCall((value) => {
5460
assert.deepStrictEqual(value, expectedResult);
5561
}));

0 commit comments

Comments
 (0)