Implement timeout on initNode() (#81)

This implements a timeout option for initNode. initNode will timeout with a rejection if the node is not initialized within the specified number of seconds. If the timeout is negative, then it will keep retrying forever.

Also fixed some of the Promise logic in InitNode().

Author: NickZ

src/index.js

@@ -43,13 +43,27 @@ let rosNode = null;
 let pingMasterTimeout = null;
 
 //------------------------------------------------------------------
-
-function _checkMasterHelper(timeout=100) {
-  const localHelper = (resolve) => {
+/**
+ * @private
+ * Helper function to see if the master is available and able to accept
+ * connections.
+ * @param {number} timeout time in ms between connection attempts
+ * @param {number} maxTimeout maximum time in ms to retry before timing out. 
+ * A negative number will make it retry forever. 0 will only make one attempt
+ * before timing out.
+ */
+function _checkMasterHelper(timeout=100, maxTimeout=-1) {
+  let startTime = Date.now();
+  const localHelper = (resolve,reject) => {
     pingMasterTimeout = setTimeout(() => {
-      // also check that the slave api server is set up
+      // also check that the slave api server is set up   
       if (!rosNode.slaveApiSetupComplete()) {
-        localHelper(resolve);
+        if (Date.now() - startTime >= maxTimeout && !(maxTimeout < 0) ) {
+          log.error(`Unable to register with master node [${rosNode.getRosMasterUri()}]: unable to set up slave API Server. Stopping...`);
+          reject(Error('Unable to setup slave API server.'));
+          return;
+        }
+        localHelper(resolve, reject);
         return;
       }
       rosNode.getMasterUri({ maxAttempts: 1 })
@@ -59,14 +73,20 @@ function _checkMasterHelper(timeout=100) {
         resolve();
       })
       .catch((err, resp) => {
-        log.warnThrottle(60000, `Unable to register with master node [${rosNode.getRosMasterUri()}]: master may not be running yet. Will keep trying.`);
-        localHelper(resolve);
+        if (Date.now() - startTime >= maxTimeout && !(maxTimeout < 0) ){
+          log.error(`Timed out before registering with master node [${rosNode.getRosMasterUri()}]: master may not be running yet.`);
+          reject(Error('Registration with master timed out.'));
+          return;
+        } else {
+          log.warnThrottle(60000, `Unable to register with master node [${rosNode.getRosMasterUri()}]: master may not be running yet. Will keep trying.`);
+          localHelper(resolve, reject);
+        }
       });
     }, timeout);
   };
 
   return new Promise((resolve, reject) => {
-    localHelper(resolve);
+    localHelper(resolve,reject);
   });
 }
 
@@ -94,10 +114,20 @@ function _anonymizeNodeName(nodeName) {
 
 let Rosnodejs = {
   /**
-   * Initializes a ros node for this process. Only one ros node can exist per process
+   * Initializes a ros node for this process. Only one ros node can exist per process.
    * If called a second time with the same nodeName, returns a handle to that node.
-   * @param nodeName {string} name of the node to initialize
-   * @param options {object} overrides for this node
+   * @param {string} nodeName name of the node to initialize
+   * @param {object} options  overrides for this node
+   * @param {boolean}   options.anonymous Set node to be anonymous
+   * @param {object}    options.logging logger options for this node
+   * @param {function}  options.logging.getLoggers  the function for setting which loggers
+   *                                                to be used for this node
+   * @param {function}  options.logging.setLoggerLevel  the function for setting the logger
+   *                                                    level
+   * @param {string}    options.rosMasterUri the Master URI to use for this node
+   * @param {number}    options.timeout time in ms to wait for node to be initialized
+   *                                    before timing out. A negative value will retry forever.
+   *                                    A value of '0' will try once before stopping. @default -1
    * @return {Promise} resolved when connection to master is established
    */
   initNode(nodeName, options) {
@@ -113,8 +143,8 @@ let Rosnodejs = {
         return Promise.resolve(this.getNodeHandle());
       }
       // else
-      throw new Error('Unable to initialize node [' + nodeName + '] - node ['
-                      + rosNode.getNodeName() + '] already exists');
+      return Promise.reject( Error('Unable to initialize node [' + nodeName + '] - node ['
+                      + rosNode.getNodeName() + '] already exists'));
     }
 
     let rosMasterUri = process.env.ROS_MASTER_URI;
@@ -129,14 +159,18 @@ let Rosnodejs = {
     const nodeOpts = options.node || {};
     rosNode = new RosNode(nodeName, rosMasterUri, nodeOpts);
 
-    return this._loadOnTheFlyMessages(options)
-      .then(_checkMasterHelper)
+    return new Promise((resolve,reject)=>{
+      this._loadOnTheFlyMessages(options)
+      .then(()=>{return _checkMasterHelper(100, options.timeout);})
       .then(Logging.initializeRosOptions.bind(Logging, this, options.logging))
-      .then(Time._initializeRosTime.bind(Time, this))
-      .then(() => { return this.getNodeHandle(); })
+      .then(Time._initializeRosTime.bind(Time, this, options.notime))
+      .then(() => { resolve(this.getNodeHandle()); })
       .catch((err) => {
         log.error('Error during initialization: ' + err);
+        this.shutdown();
+        reject(err);
       });
+    });
   },
 
   reset() {

src/lib/Time.js

@@ -10,7 +10,11 @@ function handleSimTimeMessage(msg) {
 const Time = {
   useSimTime: false,
 
-  _initializeRosTime(rosnodejs) {
+  _initializeRosTime(rosnodejs, notime) {
+    //Only for testing purposes!
+    if (notime) {
+      return Promise.resolve();
+    }
     const nh = rosnodejs.nh;
     return nh.getParam('/use_sim_time')
     .then((val) => {

test/Log.js

@@ -43,7 +43,8 @@ describe('Logging', () => {
     masterStub = xmlrpc.createServer({host: 'localhost', port: MASTER_PORT}, () => {
       rosnodejs.initNode('/testNode', {
         rosMasterUri: `http://localhost:${MASTER_PORT}`,
-        logging: {skipRosLogging: true}
+        logging: {skipRosLogging: true},
+        notime: true
       })
       .then(() => {
         rosnodejs.log.addStream({
@@ -308,7 +309,7 @@ describe('Logging', () => {
       .then(() => {
         rosnodejs.reset();
         return rosnodejs.initNode('/testNode', {logging: {waitOnRosOut: false, level: 'info'},
-                                  rosMasterUri: `http://localhost:${MASTER_PORT}`});
+                                  rosMasterUri: `http://localhost:${MASTER_PORT}`, notime: true});
       })
       .then(() => {
         done();

test/onTheFly.js

@@ -20,6 +20,7 @@ describe('OnTheFly', function () {
     return rosnodejs.initNode('/testNode', {
       rosMasterUri: `http://localhost:${MASTER_PORT}`,
       onTheFly: true,
+      notime: true,
       logging: {skipRosLogging: true}})
       .then(() => {
         done();

test/stress.js

@@ -174,7 +174,7 @@ describe('ClientShutdown', function() {
       console.error('Got unknown method call %s: %j', method, params);
     });
 
-    return rosnodejs.initNode('/my_node', {rosMasterUri: `http://localhost:${MASTER_PORT}`, logging: {testing: true}});
+    return rosnodejs.initNode('/my_node', {rosMasterUri: `http://localhost:${MASTER_PORT}`, logging: {testing: true}, notime:true});
   });
 
   afterEach(() => {

test/xmlrpcTest.js

@@ -25,7 +25,8 @@ describe('Protocol Test', () => {
   let masterStub;
   const initArgs = {
     rosMasterUri: `http://localhost:${MASTER_PORT}`,
-    logging: {skipRosLogging: true}
+    logging: {skipRosLogging: true},
+    notime: true
   };
   const nodeName = '/testNode';