Merge pull request #199 from braydonf/start-error

Start/Stop Improvements

Author: pnagurny

integration/regtest-node.js

@@ -117,6 +117,12 @@ describe('Node Functionality', function() {
         });
       });
 
+      node.start(function(err) {
+        if (err) {
+          throw err;
+        }
+      });
+
 
     });
   });

lib/node.js

@@ -10,16 +10,13 @@ var _ = bitcore.deps._;
 var index = require('./');
 var log = index.log;
 var Bus = require('./bus');
-var BaseService = require('./service');
 var errors = require('./errors');
 
 function Node(config) {
   if(!(this instanceof Node)) {
     return new Node(config);
   }
 
-  var self = this;
-
   this.errors = errors; // So services can use errors without having to have bitcore-node as a dependency
   this.log = log;
   this.network = null;
@@ -38,13 +35,6 @@ function Node(config) {
 
   this._setNetwork(config);
 
-  this.start(function(err) {
-    if(err) {
-      return self.emit('error', err);
-    }
-    self.emit('ready');
-  });
-
 }
 
 util.inherits(Node, EventEmitter);
@@ -137,35 +127,53 @@ Node.prototype.getServiceOrder = function() {
   return stack;
 };
 
-Node.prototype._instantiateService = function(service) {
+Node.prototype._startService = function(serviceInfo, callback) {
   var self = this;
 
-  $.checkState(_.isObject(service.config));
-  $.checkState(!service.config.node);
+  $.checkState(_.isObject(serviceInfo.config));
+  $.checkState(!serviceInfo.config.node);
+  $.checkState(!serviceInfo.config.name);
 
-  var config = service.config;
+  log.info('Starting ' + serviceInfo.name);
+
+  var config = serviceInfo.config;
   config.node = this;
-  config.name = service.name;
-  var mod = new service.module(config);
-
-  // include in loaded services
-  this.services[service.name] = mod;
-
-  // add API methods
-  var methodData = mod.getAPIMethods();
-  methodData.forEach(function(data) {
-    var name = data[0];
-    var instance = data[1];
-    var method = data[2];
-
-    if (self[name]) {
-      throw new Error('Existing API method exists: ' + name);
-    } else {
-      self[name] = function() {
-        return method.apply(instance, arguments);
-      };
+  config.name = serviceInfo.name;
+  var service = new serviceInfo.module(config);
+
+  service.start(function(err) {
+    if (err) {
+      return callback(err);
+    }
+
+    // include in loaded services
+    self.services[serviceInfo.name] = service;
+
+    // add API methods
+    var methodData = service.getAPIMethods();
+    var methodNameConflicts = [];
+    methodData.forEach(function(data) {
+      var name = data[0];
+      var instance = data[1];
+      var method = data[2];
+
+      if (self[name]) {
+        methodNameConflicts.push(name);
+      } else {
+        self[name] = function() {
+          return method.apply(instance, arguments);
+        };
+      }
+    });
+
+    if (methodNameConflicts.length > 0) {
+      return callback(new Error('Existing API method(s) exists: ' + methodNameConflicts.join(', ')));
     }
+
+    callback();
+
   });
+
 };
 
 Node.prototype.start = function(callback) {
@@ -175,15 +183,15 @@ Node.prototype.start = function(callback) {
   async.eachSeries(
     servicesOrder,
     function(service, next) {
-      log.info('Starting ' + service.name);
-      try {
-        self._instantiateService(service);
-      } catch(err) {
+      self._startService(service, next);
+    },
+    function(err) {
+      if (err) {
         return callback(err);
       }
-      self.services[service.name].start(next);
-    },
-    callback
+      self.emit('ready');
+      callback();
+    }
   );
 };
 
@@ -198,8 +206,13 @@ Node.prototype.stop = function(callback) {
   async.eachSeries(
     services,
     function(service, next) {
-      log.info('Stopping ' + service.name);
-      self.services[service.name].stop(next);
+      if (self.services[service.name]) {
+        log.info('Stopping ' + service.name);
+        self.services[service.name].stop(next);
+      } else {
+        log.info('Stopping ' + service.name + ' (not started)');
+        setImmediate(next);
+      }
     },
     callback
   );

lib/scaffold/start.js

@@ -83,34 +83,54 @@ function registerSyncHandlers(node, delay) {
 
   function logSyncStatus() {
     log.info(
-      'Sync Status: Tip:', node.services.db.tip.hash,
+      'Database Sync Status: Tip:', node.services.db.tip.hash,
       'Height:', node.services.db.tip.__height,
       'Rate:', count/10, 'blocks per second'
     );
   }
 
-  node.on('synced', function() {
-    clearInterval(interval);
-  });
-
   node.on('ready', function() {
-    node.services.db.on('addblock', function(block) {
-      count++;
-      // Initialize logging if not already instantiated
-      if (!interval) {
-        interval = setInterval(function() {
-          logSyncStatus();
-          count = 0;
-        }, delay);
-      }
-    });
+
+    if (node.services.db) {
+      node.on('synced', function() {
+        clearInterval(interval);
+        logSyncStatus();
+      });
+      node.services.db.on('addblock', function(block) {
+        count++;
+        // Initialize logging if not already instantiated
+        if (!interval) {
+          interval = setInterval(function() {
+            logSyncStatus();
+            count = 0;
+          }, delay);
+        }
+      });
+    }
+
   });
 
   node.on('stopping', function() {
     clearInterval(interval);
   });
 }
 
+/**
+ * Will shutdown a node and then the process
+ * @param {Object} _process - The Node.js process object
+ * @param {Node} node - The Bitcore Node instance
+ */
+function cleanShutdown(_process, node) {
+  node.stop(function(err) {
+    if(err) {
+      log.error('Failed to stop services: ' + err);
+      return _process.exit(1);
+    }
+    log.info('Halted');
+    _process.exit(0);
+  });
+}
+
 /**
  * Will register event handlers to stop the node for `process` events
  * `uncaughtException` and `SIGINT`.
@@ -133,15 +153,7 @@ function registerExitHandlers(_process, node) {
       });
     }
     if (options.sigint) {
-      node.stop(function(err) {
-        if(err) {
-          log.error('Failed to stop services: ' + err);
-          return _process.exit(1);
-        }
-
-        log.info('Halted');
-        _process.exit(0);
-      });
+      cleanShutdown(_process, node);
     }
   }
 
@@ -190,6 +202,16 @@ function start(options) {
     log.error(err);
   });
 
+  node.start(function(err) {
+    if(err) {
+      log.error('Failed to start services');
+      if (err.stack) {
+        log.error(err.stack);
+      }
+      start.cleanShutdown(process, node);
+    }
+  });
+
   return node;
 
 }
@@ -235,3 +257,4 @@ module.exports.registerExitHandlers = registerExitHandlers;
 module.exports.registerSyncHandlers = registerSyncHandlers;
 module.exports.setupServices = setupServices;
 module.exports.spawnChildProcess = spawnChildProcess;
+module.exports.cleanShutdown = cleanShutdown;

lib/services/db.js

@@ -75,6 +75,7 @@ DB.prototype._setDataPath = function() {
 };
 
 DB.prototype.start = function(callback) {
+
   var self = this;
   if (!fs.existsSync(this.dataPath)) {
     mkdirp.sync(this.dataPath);

lib/services/web.js

@@ -48,7 +48,7 @@ WebService.prototype.stop = function(callback) {
     }
 
     callback();
-  })
+  });
 };
 
 WebService.prototype.setupAllRoutes = function() {
@@ -60,7 +60,7 @@ WebService.prototype.setupAllRoutes = function() {
       this.app.use('/' + this.node.services[key].getRoutePrefix(), subApp);
       this.node.services[key].setupRoutes(subApp, express);      
     } else {
-      log.info('Not setting up routes for ' + service.name);
+      log.debug('No routes defined for: ' + key);
     }
   }
 };
@@ -186,4 +186,4 @@ WebService.prototype.socketMessageHandler = function(message, socketCallback) {
   }
 };
 
-module.exports = WebService;
+module.exports = WebService;

test/node.unit.js

@@ -65,7 +65,6 @@ describe('Bitcore Node', function() {
       var TestNode = proxyquire('../lib/node', {});
       TestNode.prototype.start = sinon.spy();
       var node = new TestNode(config);
-      TestNode.prototype.start.callCount.should.equal(1);
       node._unloadedServices.length.should.equal(1);
       node._unloadedServices[0].name.should.equal('test1');
       node._unloadedServices[0].module.should.equal(TestService);
@@ -105,29 +104,6 @@ describe('Bitcore Node', function() {
       should.exist(regtest);
       node.network.should.equal(regtest);
     });
-    it('should emit error if an error occurred starting services', function(done) {
-      var config = {
-        datadir: 'testdir',
-        services: [
-          {
-            name: 'test1',
-            module: TestService
-          }
-        ],
-      };
-      var TestNode = proxyquire('../lib/node', {});
-      TestNode.prototype.start = function(callback) {
-        setImmediate(function() {
-          callback(new Error('error'));
-        });
-      };
-      var node = new TestNode(config);
-      node.once('error', function(err) {
-        should.exist(err);
-        err.message.should.equal('error');
-        done();
-      });
-    });
   });
 
   describe('#openBus', function() {
@@ -214,11 +190,12 @@ describe('Bitcore Node', function() {
     });
   });
 
-  describe('#_instantiateService', function() {
+  describe('#_startService', function() {
     it('will instantiate an instance and load api methods', function() {
       var node = new Node(baseConfig);
       function TestService() {}
       util.inherits(TestService, BaseService);
+      TestService.prototype.start = sinon.stub().callsArg(0);
       TestService.prototype.getData = function() {};
       TestService.prototype.getAPIMethods = function() {
         return [
@@ -230,9 +207,28 @@ describe('Bitcore Node', function() {
         module: TestService,
         config: {}
       };
-      node._instantiateService(service);
-      should.exist(node.services.testservice);
-      should.exist(node.getData);
+      node._startService(service, function(err) {
+        if (err) {
+          throw err;
+        }
+        TestService.prototype.start.callCount.should.equal(1);
+        should.exist(node.services.testservice);
+        should.exist(node.getData);
+      });
+    });
+    it('will give an error from start', function() {
+      var node = new Node(baseConfig);
+      function TestService() {}
+      util.inherits(TestService, BaseService);
+      TestService.prototype.start = sinon.stub().callsArgWith(0, new Error('test'));
+      var service = {
+        name: 'testservice',
+        module: TestService,
+        config: {}
+      };
+      node._startService(service, function(err) {
+        err.message.should.equal('test');
+      });
     });
   });
 
@@ -318,7 +314,7 @@ describe('Bitcore Node', function() {
 
       node.start(function(err) {
         should.exist(err);
-        err.message.should.match(/^Existing API method exists/);
+        err.message.should.match(/^Existing API method\(s\) exists\:/);
         done();
       });