Adding some test for EADDRINUSE errors on startup

.gitignore

@@ -1,3 +1,5 @@
 node_modules
 *.swp
 *.swo
+
+coverage/

lib/Grape.js

@@ -6,9 +6,12 @@ var _s = require('underscore.string')
 var async = require('async')
 var Events = require('events')
 var UWS = require('uws')
+var debug = require('debug')('grenache:grape')
+
+var noop = () => {}
 
 class Grape extends Events {
-  
+
   constructor(conf) {
     super()
 
@@ -24,62 +27,72 @@ class Grape extends Events {
     this._mem = {}
   }
 
-  create_node(port, bootstrap) {
+  create_node(port, bootstrap, cb) {
     var dht = new DHT({
-      bootstrap: bootstrap 
-    })
-
-    dht.listen(port, () => {
-      console.log(port, 'now listening')
+      bootstrap: bootstrap
     })
 
     dht.on('announce', (_peer, ih) => {
       var val = this.hex2Str(ih)
-      console.log(port, 'announce', val)
+      debug(port, 'announce', val)
     })
 
     dht.on('warning', () => {
-      console.log(port, 'warning')
+      debug(port, 'warning')
     })
 
     dht.on('node', () => {
-      console.log(port, 'node')
+      debug(port, 'node')
     })
 
     dht.on('listening', () => {
-      console.log(port, 'listening')
+      debug(port, 'listening')
     })
 
     dht.on('ready', () => {
-      console.log(port, 'ready')
+      debug(port, 'ready')
     })
 
     dht.on('error', (err) => {
-      console.log(port, 'error', err)
+      debug(port, 'error', err)
     })
 
     dht.on('peer', (_peer, val, from) => {
       var ih = this.str2Hex(val)
 
       if (!this._mem[ih]) {
-        this._mem[ih] = { 
+        this._mem[ih] = {
           peers: {}
         }
       }
 
       var me = this._mem[ih]
       me._uts = new Date()
-      
+
       var peer = 'tcp://' + _peer.host + ':' + _peer.port
 
       me.peers[peer] = {
         host: peer,
         _uts: new Date()
       }
 
-      console.log(port, 'found potential peer ' + peer + (from ? ' through ' + from.address + ':' + from.port : '') + ' for hash: ' + val)
+      debug(port, 'found potential peer ' + peer + (from ? ' through ' + from.address + ':' + from.port : '') + ' for hash: ' + val)
+    })
+
+    dht.once('error', handleBootstrapError)
+
+    dht.listen(port, (err) => {
+      dht.removeListener('error', handleBootstrapError)
+      if (err) return cb(err)
+
+      cb(null, dht)
     })
 
+    function handleBootstrapError(err) {
+      cb(err)
+      cb = noop
+    }
+
     return dht
   }
 
@@ -112,7 +125,7 @@ class Grape extends Events {
 
     this[met](data, (res) => {
       cb(null, res)
-    }) 
+    })
   }
 
   handleLookup(_val, cb) {
@@ -130,14 +143,14 @@ class Grape extends Events {
           var val = [_val, this.timeslot(0 - i)].join('-')
           this.lookup(val, res => {
             data = _.union(data, res)
-            next()          
+            next()
           })
         })
       })(i)
     }
 
     async.parallel(queue, () => {
-      cb(data)     
+      cb(data)
     })
   }
 
@@ -153,15 +166,16 @@ class Grape extends Events {
   }
 
   announce(val, port, cb) {
-    this.node.announce(this.str2Hex(val), port || this.conf.dht_port, cb ? cb : () => {})
+    this.node.announce(this.str2Hex(val), port || this.conf.dht_port, cb ? cb : () => {
+    })
   }
 
   lookup(val, cb) {
     var ih = this.str2Hex(val)
 
     this.node.lookup(ih, (err) => {
       var res = []
-    
+
       if (this._mem[ih]) {
         let me = this._mem[ih]
         res = _.reduce(me.peers, (acc, v, k) => {
@@ -170,30 +184,44 @@ class Grape extends Events {
         }, [])
       }
 
-      cb(res) 
+      cb(res)
     })
   }
 
   start(cb) {
-    this.node = this.create_node(this.conf.dht_port, this.conf.dht_bootstrap)
-
-    this.transport = new UWS.Server({ port: this.conf.api_port }, (err) => {
+    this.create_node(this.conf.dht_port, this.conf.dht_bootstrap, (err, node) => {
       if (err) return cb(err)
 
-      this.transport.on('connection', socket => {
-        socket.on('message', msg => {
-          msg = JSON.parse(msg)
-          var rid = msg[0]
-          var type = msg[1]
-          var data = msg[2]
-          this.onRequest(type, data, (err, res) => {
-            console.log('Grape.reply', rid, type, err, res)
-            socket.send(JSON.stringify([rid, err || res]))
+      this.node = node
+
+      this.transport = new UWS.Server({port: this.conf.api_port}, (err) => {
+        this.transport.httpServer.removeListener('error', handleApiBootstrapError)
+
+        if (err) return cb(err)
+
+        this.transport.on('connection', socket => {
+          socket.on('message', msg => {
+            msg = JSON.parse(msg)
+            var rid = msg[0]
+            var type = msg[1]
+            var data = msg[2]
+            this.onRequest(type, data, (err, res) => {
+              console.log('Grape.reply', rid, type, err, res)
+              socket.send(JSON.stringify([rid, err || res]))
+            })
           })
         })
+
+        cb()
+        cb = noop
       })
 
-      cb()
+      // We have to do this because UWS.Server does not call the callback with an error if the port is already in use.
+      this.transport.httpServer.once('error', handleApiBootstrapError)
+      function handleApiBootstrapError(err) {
+        cb(err)
+        cb = noop
+      }
     })
   }
 
@@ -211,7 +239,12 @@ class Grape extends Events {
         // Transport.close does not accept a callback, but should since its underlying implementation accepts one
         //transport ? transport.close(cb) : cb()
 
+        if (!transport) return cb()
+
         transport.close()
+
+        // Under the hood it creates a httpServer instance then doesn't clean it up
+        transport.httpServer.close();
         cb()
       }
     ], cb)

package.json

@@ -35,7 +35,7 @@
     "mocha": "^2.2.5"
   },
   "scripts": {
-    "test": "mocha test/*",
+    "test": "istanbul test ./node_modules/.bin/_mocha -- -R spec",
     "lint": "eslint --config .eslintrc --fix index.js lib/ test/"
   },
   "license": "MIT",

test/Grape.js

@@ -31,4 +31,57 @@ describe('Grape', () => {
       })
     })
   })
+
+  it('calling stop on an unstarted Grape is fine', (done) => {
+    var grape = new Grape({
+      dht_port: 20000,
+      api_port: 20001
+    })
+
+    grape.stop(done)
+  })
+
+  describe('errors', () => {
+    it('should error when api_port is already in use', (done) => {
+      var grape1 = new Grape({
+        dht_port: 20000,
+        api_port: 20001
+      })
+
+      var grape2 = new Grape({
+        dht_port: 30000,
+        api_port: 20001 //same
+      })
+
+      grape1.start((err) => {
+        assert.ifError(err)
+        grape2.start((err) => {
+          assert.ok(err instanceof Error)
+          assert.equal('EADDRINUSE', err.code)
+          grape1.stop(done)
+        })
+      })
+    })
+
+    it('should error when dht_port is already in use', (done) => {
+      var grape1 = new Grape({
+        dht_port: 20000,
+        api_port: 20001
+      })
+
+      var grape2 = new Grape({
+        dht_port: 20000,  //same
+        api_port: 30000
+      })
+
+      grape1.start((err) => {
+        assert.ifError(err)
+        grape2.start((err) => {
+          assert.ok(err instanceof Error)
+          assert.equal('EADDRINUSE', err.code)
+          grape1.stop(done)
+        })
+      })
+    })
+  })
 })