fixed no http callback provided bug

Author: William Armiros

packages/core/lib/patchers/http_p.js

@@ -142,7 +142,7 @@ function enableCapture(module, downstreamXRayEnabled, subsegmentCallback) {
       // Only need to remove our listener & re-emit if we're not listening using the errorMonitor,
       // e.g. the app is running on Node 10. Otherwise the errorMonitor will re-emit automatically.
       // See: https://github.com/aws/aws-xray-sdk-node/issues/318
-      // TODO: Remove this logic when the 'error' listener is removed once node 10 support is deprecated
+      // TODO: Remove this logic once node 12 support is deprecated
       if (!events.errorMonitor && this.listenerCount('error') <= 1) {
         this.removeListener('error', errorCapturer);
         this.emit('error', e);
@@ -179,14 +179,15 @@ function enableCapture(module, downstreamXRayEnabled, subsegmentCallback) {
         } else {
           callback(res);
         }
-        // if no callback provided and there is only SDK added response listener,
-        // we consume the response so the actual end can fire.
-      } else if (res && res.listenerCount('end') === 1) {
+        // if no callback provided by user application, then we consume the response
+        // so the 'end' event fires
+        // See: https://nodejs.org/api/http.html#http_class_http_clientrequest
+      } else {
         res.resume();
       }
     });
 
-    // Use errorMonitor if available (in Node 12+), otherwise fall back to standard error listener
+    // Use errorMonitor if available (in Node 12.17+), otherwise fall back to standard error listener
     // See: https://nodejs.org/dist/latest-v12.x/docs/api/events.html#events_eventemitter_errormonitor
     req.on(events.errorMonitor || 'error', errorCapturer);
 

packages/core/test/unit/patchers/http_p.test.js

@@ -129,7 +129,7 @@ describe('HTTP/S', function() {
     });
 
     describe('on invocation', function() {
-      var capturedHttp, fakeRequest, fakeResponse, httpClient, requestSpy, sandbox;
+      var capturedHttp, fakeRequest, fakeResponse, httpClient, requestSpy, resumeSpy, sandbox;
 
       beforeEach(function() {
         sandbox = sinon.sandbox.create();
@@ -145,8 +145,10 @@ describe('HTTP/S', function() {
         }};
         httpClient.get = httpClient.request;
 
+        resumeSpy = sandbox.spy(fakeResponse, 'resume');
         requestSpy = sandbox.spy(httpClient, 'request');
-        capturedHttp = captureHTTPs(httpClient, true); });
+        capturedHttp = captureHTTPs(httpClient, true);
+      });
 
       afterEach(function() {
         sandbox.restore();
@@ -159,6 +161,16 @@ describe('HTTP/S', function() {
         resolveManualStub.should.have.been.calledWith(options);
       });
 
+      it('should consume the response if no callback is provided by user', function() {
+        capturedHttp.request(httpOptions);  // no callback
+        resumeSpy.should.have.been.calledOnce;
+      });
+
+      it('should not consume the response if a callback is provided by user', function() {
+        capturedHttp.request(httpOptions, () => {});
+        resumeSpy.should.not.have.been.called;
+      });
+
       it('should create a new subsegment with name as hostname', function() {
         var options = {hostname: 'hostname', path: '/'};
         capturedHttp.request(options);
@@ -385,7 +397,7 @@ describe('HTTP/S', function() {
         }, 50);
       });
 
-      if (process.version.startsWith('v') && process.version >= 'v12') {
+      if (process.version.startsWith('v') && process.version >= 'v12.17') {
         it('should still re-emit if there are multiple errorMonitors attached', function() {
           fakeRequest.on(events.errorMonitor, function() {});
           fakeRequest.on(events.errorMonitor, function() {});