fix http client integration always setting the error flag (#1956)

DataDog · Apr 4, 2022 · 24896ed · 24896ed
1 parent 9d31b52
commit 24896ed
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 10 deletions.
diff --git a/packages/datadog-instrumentations/src/http/client.js b/packages/datadog-instrumentations/src/http/client.js
@@ -50,6 +50,7 @@ function patch (http, methodName) {
 
       const ar = new AsyncResource('bound-anonymous-fn')
 
+      let finished = false
       let callback = args.callback
 
       if (callback) {
@@ -60,34 +61,33 @@ function patch (http, methodName) {
       const req = ar.bind(request).call(this, options, callback)
       const emit = req.emit
 
-      req.emit = function (eventName, arg) {
-        const finished = false
-        const finish = (finished, req, res) => {
-          if (!finished) {
-            finished = true
-            asyncEndClientCh.publish({ req, res })
-          }
+      const finish = (req, res) => {
+        if (!finished) {
+          finished = true
+          asyncEndClientCh.publish({ req, res })
         }
+      }
 
+      req.emit = function (eventName, arg) {
         ar.runInAsyncScope(() => {
           switch (eventName) {
             case 'response': {
               const res = arg
-              const listener = ar.bind(() => finish(finished, req, res))
+              const listener = ar.bind(() => finish(req, res))
               res.on('end', listener)
               res.on('error', listener)
               break
             }
             case 'connect':
             case 'upgrade':
-              finish(finished, req, arg)
+              finish(req, arg)
               break
             case 'error':
               errorClientCh.publish(arg)
             case 'abort': // deprecated and replaced by `close` in node 17
             case 'timeout':
             case 'close':
-              finish(finished, req)
+              finish(req)
           }
         })
 

diff --git a/packages/datadog-plugin-http/test/client.spec.js b/packages/datadog-plugin-http/test/client.spec.js
@@ -856,6 +856,38 @@ describe('Plugin', () => {
           })
         })
 
+        it('should not update the span after the request is finished', done => {
+          const app = express()
+
+          app.get('/user', (req, res) => {
+            res.status(200).send()
+          })
+
+          getPort().then(port => {
+            agent
+              .use(traces => {
+                expect(traces[0][1]).to.have.property('error', 0)
+                expect(traces[0][1].meta).to.have.property('http.status_code', '200')
+                expect(traces[0][1].meta).to.have.property('http.url', `${protocol}://localhost:${port}/user`)
+              })
+              .then(done)
+              .catch(done)
+
+            appListener = server(app, port, () => {
+              tracer.trace('test.request', (span, finish) => {
+                const req = http.request(`${protocol}://localhost:${port}/user`, res => {
+                  res.on('data', () => {})
+                  res.on('end', () => {
+                    setTimeout(finish)
+                  })
+                })
+
+                req.end()
+              })
+            })
+          })
+        })
+
         if (protocol === 'http') {
           it('should skip requests marked as noop', done => {
             const app = express()