[refactor] Tidy the logic for handling data back from monitor processes

[fix text] Assert the correct things in test/core/stopbypid-peaceful-test.js
[dist minor] Correct file headers in some test files
[dist minor] s/if(/if (/, s/){/) {/, and other minor whitespace

bin/monitor

@@ -33,19 +33,19 @@ function start(options) {
     // would still be running in the background unaffected.
     //
     forever.startServer(monitor);
-    
+
     //
     // Disconnect the IPC channel, letting this monitor's parent process know
     // that the child has started successfully.
     //
     process.disconnect();
-    
+
     //
     // Write the pidFile to disk
     //
     writePid(options.pidFile, monitor.child.pid);
   });
-  
+
   //
   // When the monitor restarts update the pid in the pidFile
   //
@@ -57,11 +57,11 @@ function start(options) {
   //
   // When the monitor stops or exits, remove the pid and log files
   //
-  function cleanUp(){
+  function cleanUp() {
     try {
       fs.unlinkSync(options.pidFile);
     }
-    catch(e){}
+    catch(e) {}
   }
   monitor.on('stop', cleanUp);
   monitor.on('exit', cleanUp);

lib/forever.js

@@ -166,39 +166,56 @@ function getAllPids(processes) {
 function stopOrRestart(action, event, format, target) {
   var emitter = new events.EventEmitter();
 
-  function onReceived(action, socket, next, data){
-    // if `message` exists, it's definitely an Error Object
-    var caseElse = data && data.message ? 'ok' : 'error';
-    // remove other events to avoid `listener over limit` bug.
-    socket.undata([action, caseElse], onReceived.bind(null, action, socket, next));
-    // FIX: actually, this message comes from `forever-monitor`, the process is marked as `STOPPED`.
-    //    message: Cannot stop process that is not running.
-    if(data && data.message.search('is not running')){
-      caseElse = 'error';
-    }
-    next(caseElse == 'ok' ? new Error(data.message) : null);
-    socket.end();
-  }
-
   function sendAction(proc, next) {
     var socket = new nssocket.NsSocket();
 
+    function onMessage(data) {
+      //
+      // Cleanup the socket.
+      //
+      socket.undata([action, 'ok'],    onMessage);
+      socket.undata([action, 'error'], onMessage);
+      socket.end();
+
+      //
+      // Messages are only sent back from error cases. The event
+      // calling context is available from `nssocket`.
+      //
+      var message = data && data.message,
+          type    = this.event.slice().pop();
+
+      //
+      // Remark (Tjatse): This message comes from `forever-monitor`, the process is marked
+      // as `STOPPED`: message: Cannot stop process that is not running.
+      //
+      // Remark (indexzero): We should probably warn instead of emitting an error in `forever-monitor`,
+      // OR handle that error in `bin/worker` for better RPC.
+      //
+      return type === 'error' && /is not running/.test(message)
+        ? next(new Error(message))
+        : next(null, data);
+    }
+
     socket.connect(proc.socket, function (err) {
       if (err) {
         next(err);
       }
 
-      socket.dataOnce([action, 'ok'], onReceived.bind(null, action, socket, next));
-      socket.dataOnce([action, 'error'], onReceived.bind(null, action, socket, next));
-
+      socket.dataOnce([action, 'ok'],    onMessage);
+      socket.dataOnce([action, 'error'], onMessage);
       socket.send([action]);
     });
 
+    //
+    // Remark (indexzero): This is a race condition, but unlikely to ever hit since
+    // if the socket errors we will never get any data in the first place...
+    //
     socket.on('error', function (err) {
       next(err);
     });
   }
 
+
   getAllProcesses(function (err, processes) {
     if (err) {
       return process.nextTick(function () {
@@ -207,11 +224,11 @@ function stopOrRestart(action, event, format, target) {
     }
 
     var procs = processes;
-
     if (target !== undefined && target !== null) {
-      if(isNaN(target)){
+      if (isNaN(target)) {
         procs = forever.findByScript(target, processes);
       }
+
       procs = procs
         || forever.findById(target, processes)
         || forever.findByIndex(target, processes)
@@ -225,6 +242,9 @@ function stopOrRestart(action, event, format, target) {
           emitter.emit('error', err);
         }
 
+        //
+        // Remark (indexzero): we should do something with the results.
+        //
         emitter.emit(event, forever.format(format, procs));
       });
     }

lib/forever/worker.js

@@ -66,20 +66,23 @@ Worker.prototype.start = function (callback) {
     });
 
     socket.data(['stop'], function () {
-      function onStop(err){
+      function onStop(err) {
         var args = [];
-        if(err && err instanceof Error){
-          args.push(['stop', 'error'], {message: err.message, stack: err.stack});
+        if (err && err instanceof Error) {
+          args.push(['stop', 'error'], { message: err.message, stack: err.stack });
           self.monitor.removeListener('stop', onStop);
-        }else{
+        }
+        else {
           args.push(['stop', 'ok']);
           self.monitor.removeListener('error', onStop);
         }
+
         socket.send.apply(socket, args);
         if (self.exitOnStop) {
           process.exit();
         }
       }
+
       self.monitor.once('stop', onStop);
       self.monitor.once('error', onStop);
 

test/core/daemonic-inheritance-test.js

@@ -47,16 +47,16 @@ vows.describe('forever/core/startDaemon').addBatch({
     }
   }
 }).addBatch({
-    "When the tests are over" : {
-      "stop all forever processes" : {
-        topic: function () {
-          forever.load({root:myRoot});
-          forever.stopAll().on('stopAll', this.callback.bind(null, null));
-        },
-        "should stop the correct number of procs": function (err, procs) {
-          assert.isArray(procs);
-          assert.lengthOf(procs, 1);
-        }
+  "When the tests are over" : {
+    "stop all forever processes" : {
+      topic: function () {
+        forever.load({root:myRoot});
+        forever.stopAll().on('stopAll', this.callback.bind(null, null));
+      },
+      "should stop the correct number of procs": function (err, procs) {
+        assert.isArray(procs);
+        assert.lengthOf(procs, 1);
       }
     }
-  }).export(module);
+  }
+}).export(module);

test/core/stopall-peaceful-test.js

@@ -1,5 +1,5 @@
 /*
- * start-stop-relative.js: start or stop forever using relative paths, the script path could be start with './', '../' ...
+ * stopall-peaceful-test.js: tests if `forever start` followed by `forever stopall` works.
  *
  * (C) 2010 Charlie Robbins & the Contributors
  * MIT LICENCE
@@ -46,7 +46,7 @@ vows.describe('forever/core/stopall-peaceful').addBatch({
     "try to stop all" : {
       topic: function () {
         var that = this;
-        forever.list(false, function(err, procs){
+        forever.list(false, function(err, procs) {
           assert.isNull(err);
           assert.isArray(procs);
           assert.equal(procs.length, 1);
@@ -56,10 +56,10 @@ vows.describe('forever/core/stopall-peaceful').addBatch({
           var cmd = runCmd('stopall', []);
           cmd.stdout.on('data', onData);
           //listen on the `data` event.
-          function onData(data){
+          function onData(data) {
             // check whether pid exists or not.
             var line = data.toString().replace (/[\n\r\t\s]+/g, ' ');
-            if(line && line.search(new RegExp(pid)) > 0){
+            if (line && line.search(new RegExp(pid)) > 0) {
               that.callback(null, true);
               cmd.stdout.removeListener('data', onData);
             }

test/core/stopbypid-peaceful-test.js

@@ -1,5 +1,5 @@
 /*
- * start-stop-relative.js: start or stop forever using relative paths, the script path could be start with './', '../' ...
+ * stopbypid-peaceful-test.js: tests if `forever start` followed by `forever stop <pid>` works.
  *
  * (C) 2010 Charlie Robbins & the Contributors
  * MIT LICENCE
@@ -27,17 +27,17 @@ vows.describe('forever/core/stopbypid-peaceful').addBatch({
     "to run script with 100% exit" : {
       topic: function () {
         runCmd('start', [
-          './test/fixtures/cluster-fork-mode.js'
+          './test/fixtures/log-on-interval.js'
         ]);
         setTimeout(function (that) {
           forever.list(false, that.callback);
         }, 2000, this);
       },
-      "the script should be marked as `STOPPED`": function (err, procs) {
+      "the script should be running": function (err, procs) {
         assert.isNull(err);
         assert.isArray(procs);
         assert.equal(procs.length, 1);
-        assert.ok(!procs[0].running);
+        assert.ok(procs[0].running);
       }
     }
   }
@@ -46,7 +46,7 @@ vows.describe('forever/core/stopbypid-peaceful').addBatch({
     "try to stop by pid" : {
       topic: function () {
         var that = this;
-        forever.list(false, function(err, procs){
+        forever.list(false, function(err, procs) {
           assert.isNull(err);
           assert.isArray(procs);
           assert.equal(procs.length, 1);
@@ -55,11 +55,12 @@ vows.describe('forever/core/stopbypid-peaceful').addBatch({
           // run command
           var cmd = runCmd('stop', [pid]);
           cmd.stdout.on('data', onData);
+          cmd.stderr.pipe(process.stderr);
           //listen on the `data` event.
-          function onData(data){
+          function onData(data) {
             // check whether pid exists or not.
             var line = data.toString().replace (/[\n\r\t\s]+/g, ' ');
-            if(line && line.search(new RegExp(pid)) > 0){
+            if (line && line.search(new RegExp(pid)) > 0) {
               that.callback(null, true);
               cmd.stdout.removeListener('data', onData);
             }