remove socket error handler after request is done

`npm-registry-client` uses `request`, which in turn uses an HTTP agent
for reusing sockets, so the `error` handlers registered on it in
`npm-registry-client` just piled up and kept being attached over the
entire lifetime of the socket.

This patch seeks to fix this by removing the listener from the socket
once the callback is invoked, as keeping it around after that would
just be a memory leak.

Author: addaleax

lib/request.js

@@ -92,7 +92,16 @@ function regRequest (uri, params, cb_) {
 }
 
 function makeRequest (uri, params, cb_) {
-  var cb = once(cb_)
+  var socket
+  var cb = once(function (er, parsed, raw, response) {
+    if (socket) {
+      // The socket might be returned to a pool for re-use, so don’t keep
+      // the 'error' listener from here attached.
+      socket.removeListener('error', cb)
+    }
+
+    return cb_(er, parsed, raw, response)
+  })
 
   var parsed = url.parse(uri)
   var headers = {}
@@ -149,8 +158,13 @@ function makeRequest (uri, params, cb_) {
   var req = request(opts, params.streaming ? undefined : decodeResponseBody(done))
 
   req.on('error', cb)
+
+  // This should not be necessary, as the HTTP implementation in Node
+  // passes errors occurring on the socket to the request itself. Being overly
+  // cautious comes at a low cost, though.
   req.on('socket', function (s) {
-    s.on('error', cb)
+    socket = s
+    socket.on('error', cb)
   })
 
   if (params.streaming) {

test/lib/common.js

@@ -10,6 +10,14 @@ if (!global.setImmediate || !require('timers').setImmediate) {
   }
 }
 
+// See https://github.com/npm/npm-registry-client/pull/142 for background.
+// Note: `process.on('warning')` only works with Node >= 6.
+process.on('warning', function (warning) {
+  if (/Possible EventEmitter memory leak detected/.test(warning.message)) {
+    throw new Error('There should not be any EventEmitter memory leaks')
+  }
+})
+
 module.exports = {
   port: server.port,
   registry: REGISTRY,