dd-trace-api: don't proxy objects returned from callbacks (#5240)

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.

Author: bengl

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

@@ -48,6 +48,14 @@ module.exports = class DdTraceApiPlugin extends Plugin {
           self = objectMap.get(self)
         }
 
+        // `trace` returns the value that's returned from the original callback
+        // passed to it, so we need to detect that happening and bypass the check
+        // for a proxy, since a proxy isn't needed, since the object originates
+        // from the caller. In callbacks, we'll assign return values to this
+        // value, and bypass the proxy check if `ret.value` is exactly this
+        // value.
+        let passthroughRetVal
+
         for (let i = 0; i < args.length; i++) {
           if (objectMap.has(args[i])) {
             args[i] = objectMap.get(args[i])
@@ -63,7 +71,8 @@ module.exports = class DdTraceApiPlugin extends Plugin {
                 }
               }
               // TODO do we need to apply(this, ...) here?
-              return orig(...fnArgs)
+              passthroughRetVal = orig(...fnArgs)
+              return passthroughRetVal
             }
           }
         }
@@ -74,7 +83,7 @@ module.exports = class DdTraceApiPlugin extends Plugin {
             const proxyVal = proxy()
             objectMap.set(proxyVal, ret.value)
             ret.value = proxyVal
-          } else if (ret.value && typeof ret.value === 'object') {
+          } else if (ret.value && typeof ret.value === 'object' && passthroughRetVal !== ret.value) {
             throw new TypeError(`Objects need proxies when returned via API (${name})`)
           }
         } catch (e) {

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

@@ -220,6 +220,20 @@ describe('Plugin', () => {
     describeMethod('extract', null)
     describeMethod('getRumData', '')
     describeMethod('trace')
+
+    describe('trace with return value', () => {
+      it('should return the exact same value', () => {
+        const obj = { mustBeThis: 'value' }
+        tracer.trace.resetHistory() // clear previous call to `trace`
+        testChannel({
+          name: 'trace',
+          fn: tracer.trace,
+          ret: obj,
+          proxy: false,
+          args: ['foo', {}, () => obj]
+        })
+      })
+    })
     describeMethod('wrap')
     describeMethod('use', SELF)
     describeMethod('profilerStarted', Promise.resolve(false))
@@ -268,16 +282,13 @@ describe('Plugin', () => {
       })
     }
 
-    function testChannel ({ name, fn, self = dummyTracer, ret = undefined, args = [] }) {
+    function testChannel ({ name, fn, self = dummyTracer, ret, args = [], proxy }) {
       testedChannels.add('datadog-api:v1:' + name)
       const ch = dc.channel('datadog-api:v1:' + name)
-      const payload = {
-        self,
-        args,
-        ret: {},
-        proxy: ret && typeof ret === 'object' ? () => ret : undefined,
-        revProxy: []
+      if (proxy === undefined) {
+        proxy = ret && typeof ret === 'object' ? () => ret : undefined
       }
+      const payload = { self, args, ret: {}, proxy, revProxy: [] }
       ch.publish(payload)
       if (payload.ret.error) {
         throw payload.ret.error