Skip to content

Commit fd51424

Browse files
Qardclaude
andcommitted
fix: Preserve original return value in wrapPromise to avoid losing Promise subclasses
When the instrumented function returns a Promise subclass (e.g. Anthropic SDK's APIPromise which has a .withResponse() method), the previous wrapPromise template returned `promise.then(cb)` — which calls the subclass's overridden .then() and returns a plain native Promise, losing any subclass-specific methods on the original. Fix by calling .then() as a side effect for publishing asyncStart/asyncEnd channel events, then returning the original promise unchanged. This ensures the caller receives the same type that the wrapped function originally returned, preserving any subclass API. Add a test (promise_subclass) to cover this case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b494f56 commit fd51424

4 files changed

Lines changed: 61 additions & 3 deletions

File tree

lib/transforms.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,21 +394,20 @@ function wrapPromise (state, node) {
394394
__apm$ctx.result = promise;
395395
return promise;
396396
}
397-
return promise.then(
397+
promise.then(
398398
result => {
399399
__apm$ctx.result = result;
400400
${channelVariable}.asyncStart.publish(__apm$ctx);
401401
${channelVariable}.asyncEnd.publish(__apm$ctx);
402-
return result;
403402
},
404403
err => {
405404
__apm$ctx.error = err;
406405
${channelVariable}.error.publish(__apm$ctx);
407406
${channelVariable}.asyncStart.publish(__apm$ctx);
408407
${channelVariable}.asyncEnd.publish(__apm$ctx);
409-
throw err;
410408
}
411409
);
410+
return promise;
412411
} catch (err) {
413412
__apm$ctx.error = err;
414413
${channelVariable}.error.publish(__apm$ctx);

tests/promise_subclass/mod.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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+
* NOTE: This function is intentionally NOT async. Async functions always wrap
12+
* their return value in a native Promise, losing the subclass type. The real-
13+
* world scenario (e.g. Anthropic SDK's APIPromise) uses a non-async function
14+
* that explicitly constructs and returns a Promise subclass instance.
15+
*/
16+
class ExtendedPromise extends Promise {
17+
withResponse () {
18+
return this.then(result => ({ data: result, response: { status: 200 } }))
19+
}
20+
}
21+
22+
function fetch (url) {
23+
return new ExtendedPromise(resolve => resolve(42))
24+
}
25+
26+
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)