Skip to content

Commit ee82212

Browse files
committed
dd-trace-api: don't proxy objects returned from callbacks
Some APIs (pretty much just trace) return whatever value is returned from a callback passed in. Without providing for this, this would trip up the check that returned objects are proxied. We don't want to proxy these objects since they come directly from the caller.
1 parent d19540d commit ee82212

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

packages/datadog-plugin-dd-trace-api/src/index.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ module.exports = class DdTraceApiPlugin extends Plugin {
4848
self = objectMap.get(self)
4949
}
5050

51+
// `trace` returns the value that's returned from the original callback
52+
// passed to it, so we need to detect that happening and bypass the check
53+
// for a proxy, since a proxy isn't needed, since the object originates
54+
// from the caller. In callbacks, we'll assign return values to this
55+
// value, and bypass the proxy check if `ret.value` is exactly this
56+
// value.
57+
let passthroughRetVal = undefined;
58+
5159
for (let i = 0; i < args.length; i++) {
5260
if (objectMap.has(args[i])) {
5361
args[i] = objectMap.get(args[i])
@@ -63,7 +71,8 @@ module.exports = class DdTraceApiPlugin extends Plugin {
6371
}
6472
}
6573
// TODO do we need to apply(this, ...) here?
66-
return orig(...fnArgs)
74+
passthroughRetVal = orig(...fnArgs)
75+
return passthroughRetVal
6776
}
6877
}
6978
}
@@ -74,7 +83,7 @@ module.exports = class DdTraceApiPlugin extends Plugin {
7483
const proxyVal = proxy()
7584
objectMap.set(proxyVal, ret.value)
7685
ret.value = proxyVal
77-
} else if (ret.value && typeof ret.value === 'object') {
86+
} else if (ret.value && typeof ret.value === 'object' && passthroughRetVal !== ret.value) {
7887
throw new TypeError(`Objects need proxies when returned via API (${name})`)
7988
}
8089
} catch (e) {

packages/datadog-plugin-dd-trace-api/test/index.spec.js

+17-7
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,19 @@ describe('Plugin', () => {
220220
describeMethod('extract', null)
221221
describeMethod('getRumData', '')
222222
describeMethod('trace')
223+
describe('trace with return value', () => {
224+
it('should return the exact same value', () => {
225+
let obj = { mustBeThis: 'value' }
226+
tracer.trace.resetHistory() // clear previous call to `trace`
227+
testChannel({
228+
name: 'trace',
229+
fn: tracer.trace,
230+
ret: obj,
231+
proxy: false,
232+
args: ['foo', {}, () => obj]
233+
})
234+
})
235+
})
223236
describeMethod('wrap')
224237
describeMethod('use', SELF)
225238
describeMethod('profilerStarted', Promise.resolve(false))
@@ -268,16 +281,13 @@ describe('Plugin', () => {
268281
})
269282
}
270283

271-
function testChannel ({ name, fn, self = dummyTracer, ret = undefined, args = [] }) {
284+
function testChannel ({ name, fn, self = dummyTracer, ret, args = [], proxy }) {
272285
testedChannels.add('datadog-api:v1:' + name)
273286
const ch = dc.channel('datadog-api:v1:' + name)
274-
const payload = {
275-
self,
276-
args,
277-
ret: {},
278-
proxy: ret && typeof ret === 'object' ? () => ret : undefined,
279-
revProxy: []
287+
if (proxy === undefined) {
288+
proxy = ret && typeof ret === 'object' ? () => ret : undefined
280289
}
290+
const payload = { self, args, ret: {}, proxy, revProxy: []}
281291
ch.publish(payload)
282292
if (payload.ret.error) {
283293
throw payload.ret.error

0 commit comments

Comments
 (0)