Skip to content

Commit d59a4e6

Browse files
Qardclaude
andauthored
fix: Preserve Promise subclass return values in wrapPromise (#57)
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 3f0e14d commit d59a4e6

4 files changed

Lines changed: 86 additions & 4 deletions

File tree

lib/transforms.js

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,24 +391,46 @@ function wrapPromise (state, node) {
391391
try {
392392
let promise = __apm$traced();
393393
if (typeof promise?.then !== 'function') {
394-
__apm$ctx.result = promise;
394+
__apm$ctx.result = promise;
395395
return promise;
396396
}
397-
return promise.then(
397+
// Mirror Node.js core diagnostics_channel behaviour: for native Promise
398+
// instances, chain normally (safe since there is no subclass API to
399+
// preserve). For Promise subclasses and other thenables, side-chain the
400+
// callbacks for event publishing and return the original so that any
401+
// subclass-specific methods (e.g. APIPromise.withResponse()) remain
402+
// accessible to the caller.
403+
if (promise instanceof Promise && promise.constructor === Promise) {
404+
return promise.then(
405+
result => {
406+
__apm$ctx.result = result;
407+
${channelVariable}.asyncStart.publish(__apm$ctx);
408+
${channelVariable}.asyncEnd.publish(__apm$ctx);
409+
return result;
410+
},
411+
err => {
412+
__apm$ctx.error = err;
413+
${channelVariable}.error.publish(__apm$ctx);
414+
${channelVariable}.asyncStart.publish(__apm$ctx);
415+
${channelVariable}.asyncEnd.publish(__apm$ctx);
416+
throw err;
417+
}
418+
);
419+
}
420+
promise.then(
398421
result => {
399422
__apm$ctx.result = result;
400423
${channelVariable}.asyncStart.publish(__apm$ctx);
401424
${channelVariable}.asyncEnd.publish(__apm$ctx);
402-
return result;
403425
},
404426
err => {
405427
__apm$ctx.error = err;
406428
${channelVariable}.error.publish(__apm$ctx);
407429
${channelVariable}.asyncStart.publish(__apm$ctx);
408430
${channelVariable}.asyncEnd.publish(__apm$ctx);
409-
throw err;
410431
}
411432
);
433+
return promise;
412434
} catch (err) {
413435
__apm$ctx.error = err;
414436
${channelVariable}.error.publish(__apm$ctx);

tests/promise_subclass/mod.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
3+
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc.
4+
**/
5+
6+
/**
7+
* A Promise subclass with an extra method, similar to Anthropic SDK's APIPromise.
8+
* The instrumented wrapper must preserve the original return type so callers
9+
* can still access subclass-specific methods like `.withResponse()`.
10+
*/
11+
class ExtendedPromise extends Promise {
12+
withResponse () {
13+
return this.then(result => ({ data: result, response: { status: 200 } }))
14+
}
15+
}
16+
17+
/**
18+
* NOTE: Intentionally NOT async. Async functions always wrap their return value
19+
* in a native Promise, losing the subclass type. The real-world scenario
20+
* (e.g. Anthropic SDK's APIPromise) uses a non-async function that explicitly
21+
* constructs and returns a Promise subclass instance.
22+
*/
23+
function fetch (url) {
24+
return new ExtendedPromise(resolve => resolve(42))
25+
}
26+
27+
module.exports = { fetch }

tests/promise_subclass/test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License.
3+
* This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc.
4+
**/
5+
const { fetch } = require('./instrumented.js')
6+
const { assert, getContext } = require('../common/preamble.js')
7+
const context = getContext('orchestrion:undici:fetch_subclass');
8+
(async () => {
9+
const promise = fetch('https://example.com')
10+
// The instrumented wrapper must return the original promise, not promise.then(),
11+
// so subclass methods like withResponse() remain accessible.
12+
assert.strictEqual(typeof promise.withResponse, 'function', 'withResponse should be available on the returned promise')
13+
const { data } = await promise.withResponse()
14+
assert.strictEqual(data, 42)
15+
assert.deepStrictEqual(context, {
16+
start: true,
17+
end: true,
18+
asyncStart: 42,
19+
asyncEnd: 42
20+
})
21+
})()

tests/tests.test.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,18 @@ describe('wrap_promise_non_promise', () => {
518518
})
519519
})
520520

521+
describe('promise_subclass', () => {
522+
test('instruments async function and preserves original Promise subclass return type', () => {
523+
runTest('promise_subclass', [
524+
{
525+
channelName: 'fetch_subclass',
526+
module: { name: TEST_MODULE_NAME, versionRange: '>=0.0.1', filePath: TEST_MODULE_PATH },
527+
functionQuery: { functionName: 'fetch', kind: 'Async' }
528+
}
529+
])
530+
})
531+
})
532+
521533
describe('IIFE with class', () => {
522534
test('instruments a class within a IIFE, variable same name as class', () => {
523535
runTest('iife_nested_class', [

0 commit comments

Comments
 (0)