Merge pull request #31 from thekemkid/loop

Fix loop keeping event loop alive

lib/myhttp.js

@@ -42,14 +42,20 @@ function Client (opts) {
 
   // cer = current expected response
   this.cer = 0
+  this.destroyed = false
 
-  this.timeoutTicker = retimer(() => {
+  const handleTimeout = () => {
     // all pipelined requests have timed out here
     this.resData.forEach(() => this.emit('timeout'))
     this.cer = 0
     this._destroyConnection()
     this._connect()
-  }, this.timeout)
+
+    // timeout has already occured, need to set a new timeoutTicker
+    this.timeoutTicker = retimer(handleTimeout, this.timeout)
+  }
+
+  this.timeoutTicker = retimer(handleTimeout, this.timeout)
 
   this.parser[HTTPParser.kOnHeadersComplete] = (opts) => {
     this.resData[this.cer].headers = opts
@@ -65,6 +71,7 @@ function Client (opts) {
 
     this.cer = this.cer === opts.pipelining - 1 ? 0 : this.cer++
     this._doRequest(this.cer)
+
     this.timeoutTicker.reschedule(this.timeout)
   }
 
@@ -95,7 +102,7 @@ Client.prototype._connect = function () {
 
   this.conn.on('error', () => {
     this.emit('connError')
-    this._connect()
+    if (!this.destroyed) this._connect()
   })
 
   this.conn.on('data', (chunk) => {
@@ -104,7 +111,7 @@ Client.prototype._connect = function () {
   })
 
   this.conn.on('end', () => {
-    this._connect()
+    if (!this.destroyed) this._connect()
   })
 
   for (let i = 0; i < this.opts.pipelining; i++) {
@@ -126,8 +133,9 @@ Client.prototype._destroyConnection = function () {
 }
 
 Client.prototype.destroy = function () {
-  this._destroyConnection()
+  this.destroyed = true
   this.timeoutTicker.clear()
+  this._destroyConnection()
 }
 
 Client.prototype.setHeaders = function (newHeaders) {

lib/run.js

@@ -141,6 +141,7 @@ function run (opts, cb) {
         'non2xx': statusCodes[0] + statusCodes[2] + statusCodes[3] + statusCodes[4]
       }
       tracker.emit('done', result)
+
       cb(null, result)
     }
   }, 1000)

test/myhttp.test.js

@@ -305,7 +305,19 @@ test('client should emit a timeout when no response is received', (t) => {
     t.ok(1, 'timeout should have happened')
   })
 
-  setTimeout(() => {
-    client.destroy()
-  }, 1500)
+  setTimeout(() => client.destroy(), 1500)
+})
+
+test('client should emit 2 timeouts when no responses are received', (t) => {
+  t.plan(2)
+
+  const opts = timeoutServer.address()
+  opts.timeout = 1
+  const client = new Client(opts)
+
+  client.on('timeout', () => {
+    t.ok(1, 'timeout should have happened')
+  })
+
+  setTimeout(() => client.destroy(), 2500)
 })