Repository: aws/aws-sdk-js (js)

Issue:
node: timers leaked on HTTP connection errors
If the `connectTimeout` http option has been supplied, timers are used to manage the connection timeout. The timers are cleared when a connection is established, however, the timers are *not* cleared if an error happens. 

The end result is that timers are leaked for every attempt to make an HTTP connection that does not succeed.

https://github.com/aws/aws-sdk-js/blob/master/lib/http/node.js#L60

## Reproduction

1. Create the following application:
    ```
    const AWS = require('aws-sdk')

    const dbOptions = {
      region: 'us-east-1',
      accessKeyId: 'accessKeyId',
      secretAccessKey: 'secretAccessKey',
      endpoint: new AWS.Endpoint('http://localhost:12345'),
      httpOptions: {
        connectTimeout: 10000
      }
    }
    const dynamo = new AWS.DynamoDB(dbOptions)

    console.log('about to start query')

    dynamo.listTables({}).promise()
      .then(() => {
        console.log('query complete')

        console.time('waiting for leaked timers')

        process.on('exit', () => {
          console.timeEnd('waiting for leaked timers')
        })
      })
      .catch((err) => {
        console.log('error executing query', err)
      })
    ```
2. Run the node application
3. A couple seconds after starting the application, fire up local dynamo:

    `docker run -it --rm -p 12345:8000 --name dynamo amazon/dynamodb-local`

### Expected

`waiting for leaked timers: ` reports < 1 millisecond.

### Unexpected

Application hangs for multiple seconds after seeing "query complete".

`waiting for leaked timers: ` reports multiple seconds (approximately `10 - <time to start dynamo>` seconds)

Pull request:
Cleans up connectTimeout timers when request errors
<!--
Thank you for your pull request. Please provide a description below.
-->

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] `npm run test` passes
- [x] changelog is added
- [x] run `bundle exec rake docs:api` and inspect `doc/latest/index.html` if documentation is changed

Fixes #2322 


Patch:
diff --git a/.changes/next-release/bugfix-Core-28a9029d.json b/.changes/next-release/bugfix-Core-28a9029d.json
new file mode 100644
index 000000000..d9af22d8b
--- /dev/null
+++ b/.changes/next-release/bugfix-Core-28a9029d.json
@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "Core",
+  "description": "Fixes issue where connectTimeout timers would not be cleared if a request errored out. Only affects node.js."
+}
\ No newline at end of file
diff --git a/lib/http/node.js b/lib/http/node.js
index a7f142aee..39d222c05 100644
--- a/lib/http/node.js
+++ b/lib/http/node.js
@@ -55,9 +55,10 @@ AWS.NodeHttpClient = AWS.util.inherit({
 
     // connection timeout support
     if (httpOptions.connectTimeout) {
+      var connectTimeoutId;
       stream.on('socket', function(socket) {
         if (socket.connecting) {
-          var timeoutId = setTimeout(function connectTimeout() {
+          connectTimeoutId = setTimeout(function connectTimeout() {
             if (stream.didCallback) return; stream.didCallback = true;
 
             stream.abort();
@@ -66,11 +67,10 @@ AWS.NodeHttpClient = AWS.util.inherit({
               {code: 'TimeoutError'}
             ));
           }, httpOptions.connectTimeout);
-          socket.on('connect', (function(tId) {
-            return function () {
-              clearTimeout(tId);
-            };
-          })(timeoutId));
+          socket.on('connect', function() {
+            clearTimeout(connectTimeoutId);
+            connectTimeoutId = null;
+          });
         }
       });
     }
@@ -85,6 +85,10 @@ AWS.NodeHttpClient = AWS.util.inherit({
     });
 
     stream.on('error', function() {
+      if (connectTimeoutId) {
+        clearTimeout(connectTimeoutId);
+        connectTimeoutId = null;
+      }
       if (stream.didCallback) return; stream.didCallback = true;
       errCallback.apply(stream, arguments);
     });


Test patch:
diff --git a/test/node_http_client.spec.js b/test/node_http_client.spec.js
index 4bc86cdea..0d8158305 100644
--- a/test/node_http_client.spec.js
+++ b/test/node_http_client.spec.js
@@ -155,7 +155,8 @@
             global.clearTimeout = oldClearTimeout;
             return httpModule.request = oldRequest;
           });
-          return it('clears timeouts once the connection has been established', function() {
+
+          it('clears timeouts once the connection has been established', function() {
             var mockSocket, req;
             req = new AWS.HttpRequest('http://10.255.255.255');
             http.handleRequest(req, {
@@ -169,8 +170,26 @@
             expect(setTimeoutSpy.calls.length).to.equal(1);
             mockSocket.emit('connect');
             expect(clearTimeoutSpy.calls.length).to.equal(1);
-            return expect(clearTimeoutSpy.calls[0]["arguments"][0]).to.equal(timeoutId);
+            expect(clearTimeoutSpy.calls[0]["arguments"][0]).to.equal(timeoutId);
           });
+
+          it('clears timeouts if an error is encountered', function() {
+            var mockSocket = new EventEmitter();
+            var req = new AWS.HttpRequest('http://10.255.255.255');
+
+            http.handleRequest(req, {
+              connectTimeout: 120000
+            }, null, function() {
+              return {};
+            });
+
+            mockSocket.connecting = true;
+            mockClientRequest.emit('socket', mockSocket);
+            expect(setTimeoutSpy.calls.length).to.equal(1);
+            mockClientRequest.emit('error', new Error('Something happened!'));
+            expect(clearTimeoutSpy.calls.length).to.equal(1);
+            expect(clearTimeoutSpy.calls[0]["arguments"][0]).to.equal(timeoutId);
+          })
         });
       });
     });