Resolve scalability issues (#137)

* Fix issues with event emission and send throwing

This commit fixes two scalability issues I identified in lab.  First, we
will defer event emissions to ensure that the listners can register
their callbacks.  Second, we add some defensive coding around the
session child writing to handle write errors gracefully.

Tested in our lab, with 75+ concurrent telnet sessions.

fixes #135, fixes #136

* Roll PATCH version

CHANGELOG.md

@@ -1,3 +1,9 @@
+v0.6.3 - Oct 4 2016
+
+* Fix issue with exceptions being thrown by `session.send` (Ryan Milbourne)
+* Fix issue with events being emitted before listener callbacks are registered (Ryan Milbourne)
+* Build: Update dependencies (greenkeeper)
+
 v0.6.2 - May 10 2016
 
 * modify "sendEof" to invoke callback w/o waiting for child stream closure (Ryan Milbourne)

lib/spectcl.js

@@ -135,7 +135,9 @@ Spectcl.prototype.spawn = function (command, cmdParams, cmdOptions, spawnOptions
             self.fullBuffer = true
             self.cache = self.cache.slice(self.cache.length-self.options.matchMax, self.cache.length)
         }
-        self.emit('data', data)
+        process.nextTick(function(){
+            self.emit('data', data)
+        })
     }
 
     // Arguments handling
@@ -193,17 +195,20 @@ Spectcl.prototype.spawn = function (command, cmdParams, cmdOptions, spawnOptions
         debug('[spawn] child tty:\t'+self.child.stdout.ttyname)
     }
 
-    debug('[spawn] child pid:\t'+self.child.pid)
-
     self.child.on('error', function(err){
         debug('[spawn] child %d error: '+err,self.child.pid)
-        self.emit('error', err)
+        process.nextTick(function(){
+            self.emit('error', err)
+        })
     })
 
     self.child.on('exit', function(code, signal){
         debug('[spawn] child %d exit. code: %s\tsignal: %s',self.child.pid,code,signal)
-        self.emit('exit', code, signal)
+        process.nextTick(function(){
+            self.emit('exit', code, signal)
+        })
     })
+    debug('[spawn] child pid:\t'+self.child.pid)
 
     return
 }
@@ -259,7 +264,7 @@ Spectcl.prototype.expect = function(expArr, cb){
          * @returns {String|RegExp|Number} The Expectation that was found in the cache or null if no match was found.
          */
       , matchCache = function(){
-          debug('[expect] self.cache = %s',self.cache)
+          debug('[expect] [%d] cache contents: %s',self.child.pid, util.inspect(self.cache))
           for(var i=0; i<expectations.length; i++){
               var exp = expectations[i]
                 , matched
@@ -390,6 +395,7 @@ Spectcl.prototype.expect = function(expArr, cb){
     }
     debug('[expect] [%d] expectations: '+Object.keys(expectObject), self.child.pid)
 
+
     /**
      * Handles end of readable stream.
      * Remove the data listener and clear the timeout interval.
@@ -476,13 +482,35 @@ Spectcl.prototype.expect = function(expArr, cb){
  * @return {undefined}
  */
 Spectcl.prototype.send = function(data, cb){
-    debug('[send] writing %s to child stdin',data)
-    this.child.stdin.write(data, function(){
-        debug('[send] wrote to child stdin')
+    var self = this
+    debug('[send] writing %s to child stdin',util.inspect(data))
+
+    var writeErrListener = function(err){
+        debug('[send] err %s',err)
+    }
+    self.child.stdin.on('error', writeErrListener)
+
+    try {
+        self.child.stdin.write(data, function(err){
+            if(err){
+                self.child.stdin.removeListener('error',writeErrListener)
+                debug('[send] error: %s',err)
+                if(cb){
+                    return cb(err)
+                }
+            }
+            self.child.stdin.removeListener('error',writeErrListener)
+            debug('[send] wrote to child stdin')
+            if(cb){
+                return cb()
+            }
+        })
+    } catch(err) {
+        debug('[send] error: %s',err)
         if(cb){
-            return cb()
+            return cb(err)
         }
-    })
+    }
 }
 
 

package.json

@@ -1,7 +1,7 @@
 {
   "name": "spectcl",
   "description": "Spawns and interacts with child processes using spawn / expect commands",
-  "version": "0.6.2",
+  "version": "0.6.3",
   "author": "Ryan Milbourne <ryan.milbourne@viasat.com>",
   "maintainers": [
     "Greg Cochard <greg@gregcochard.com>",

test/spectcl.js

@@ -616,10 +616,10 @@ describe('spectcl', function(){
                 '>', function(){
                     assert.notEqual(session.expect_out.match, null, 'null expect_out match')
                     assert.notEqual(session.expect_out.buffer, '', 'empty expect_out buffer')
-                    session.send('process.exit()\r', function(){
+                    session.child.on('exit', function(){
                         finished()
                     })
-                    session.child.on('exit', function(){
+                    session.send('process.exit()\r', function(){
                         finished()
                     })
                 }