reduce listeners added to emitters

fixes #4

HISTORY.md

@@ -1,3 +1,9 @@
+unreleased
+==========
+
+  * reduce listeners added to emitters
+    - avoids "event emitter leak" warnings when used multiple times on same request
+
 1.2.1 / 2014-06-08
 ==================
 

index.js

@@ -4,6 +4,12 @@
  * MIT Licensed
  */
 
+/**
+* Module dependencies.
+*/
+
+var first = require('ee-first')
+
 /**
 * Variables.
 */
@@ -32,17 +38,25 @@ module.exports = function finished(thingie, callback) {
     return thingie
   }
 
-  socket.on('error', done)
-  socket.on('close', done)
-  res.on('finish', done)
-
-  function done(err) {
-    if (err != null && !(err instanceof Error)) err = null; // suck it node
-    socket.removeListener('error', done)
-    socket.removeListener('close', done)
-    res.removeListener('finish', done)
-    callback(err)
+  var listener = res.__onFinished
+
+  // create a private single listener with queue
+  if (!listener || !listener.queue) {
+    listener = res.__onFinished = function onFinished(err) {
+      if (res.__onFinished === listener) res.__onFinished = null
+      var queue = listener.queue || []
+      while (queue.length) queue.shift()(err)
+    }
+    listener.queue = []
+
+    // finished on first event
+    first([
+      [socket, 'error', 'close'],
+      [res, 'finish'],
+    ], listener)
   }
 
+  listener.queue.push(callback)
+
   return thingie
 }

package.json

@@ -5,6 +5,9 @@
   "author": "Jonathan Ong <me@jongleberry.com> (http://jongleberry.com)",
   "license": "MIT",
   "repository": "expressjs/finished",
+  "dependencies": {
+    "ee-first": "1.0.3"
+  },
   "devDependencies": {
     "istanbul": "0.2.10",
     "mocha": "~1.20.1",
@@ -14,7 +17,7 @@
     "node": ">= 0.8.0"
   },
   "scripts": {
-    "test": "mocha --reporter dot test/",
+    "test": "mocha --reporter spec test/",
     "test-cov": "istanbul cover node_modules/mocha/bin/_mocha -- --reporter dot test/",
     "test-travis": "istanbul cover node_modules/mocha/bin/_mocha --report lcovonly -- --reporter spec test/"
   }

test/test.js

@@ -1,8 +1,10 @@
-var EventEmitter = require('events').EventEmitter
 
+var EventEmitter = require('events').EventEmitter
+var should = require('should')
+var assert = require('assert')
 var http = require('http')
+
 var onFinished = require('..')
-var should = require('should')
 
 function createThingie() {
   var ee = new EventEmitter
@@ -23,22 +25,14 @@ describe('on socket error', function () {
     called.should.be.true
   })
 
-  it('should not execute the callback if response is finished', function () {
+  it('should not execute the callback if response is finished', function (done) {
     var thingie = createThingie()
-    var called = false
-    onFinished(thingie, function () {
-      called = true
+    onFinished(thingie, function (err) {
+      assert.ifError(err)
+      done()
     })
     thingie.emit('finish')
-    try {
-      // throws if there are no listeners
-      thingie.socket.emit('error', new Error('boom'))
-      throw new Error('alksdjflaksjdf')
-    } catch (err) {
-      if (err.message !== 'boom')
-        throw new Error('wtf')
-    }
-    called.should.be.true
+    thingie.socket.emit(new Error('boom'))
   })
 })
 
@@ -116,3 +110,43 @@ describe('http', function () {
     })
   })
 })
+
+describe('event emitter leaks', function () {
+  describe('when adding a lot of listeners on the same request', function () {
+    it('should not warn and add at most 1 listener per emitter per event', function () {
+      // we just have to make sure tests pass without a bunch of logs
+      var thingie = createThingie()
+      var called = false
+
+      onFinished(thingie, function (err) {
+        called = true
+        err.message.should.equal('boom')
+      })
+
+      for (var i = 0; i < 1000; i++) {
+        onFinished(thingie, noop)
+      }
+
+      assert.equal(1, thingie.socket.listeners('error').length)
+      assert.equal(1, thingie.socket.listeners('close').length)
+      assert.equal(1, thingie.listeners('finish').length)
+
+      thingie.socket.emit('error', new Error('boom'))
+      called.should.be.true
+    })
+  })
+
+  it('should clean up after itself', function (done) {
+    var thingie = createThingie()
+    onFinished(thingie, function () {
+      assert(!thingie.socket.listeners('error').length)
+      assert(!thingie.socket.listeners('close').length)
+      assert(!thingie.listeners('finish').length)
+      done()
+    })
+
+    thingie.socket.emit('error')
+  })
+})
+
+function noop() {}