[FIX] requestOne() argument shifting was incorrect. Callback would ge…

…t clobbered on shift. Added test. (#200)

[FIX] createResponseMux message handler always tried to invoke a callback. While not having the callback is questionable, added safety in case it was not provided.
[ADD] JSDoc adds/fixes
[FIX] Other test fixes

lib/nats.js

@@ -21,7 +21,7 @@ var net = require('net'),
 /**
  * Constants
  */
-var VERSION = '0.8.0',
+var VERSION = '0.8.2',
 
     DEFAULT_PORT = 4222,
     DEFAULT_PRE = 'nats://localhost:',
@@ -102,7 +102,14 @@ var VERSION = '0.8.0',
 
     FLUSH_THRESHOLD = 65536;
 
-
+/**
+ * @param {String} message
+ * @param {String} code
+ * @param {Error} [chainedError]
+ * @constructor
+ *
+ * @api private
+ */
 function NatsError(message, code, chainedError) {
     Error.captureStackTrace(this, this.constructor);
     this.name = this.constructor.name;
@@ -1185,7 +1192,7 @@ Client.prototype.publish = function(subject, msg, opt_reply, opt_callback) {
  * @param {String} subject
  * @param {Object} [opts]
  * @param {Function} callback
- * @return {Mixed}
+ * @return {Number}
  * @api public
  */
 Client.prototype.subscribe = function(subject, opts, callback) {
@@ -1230,7 +1237,7 @@ Client.prototype.subscribe = function(subject, opts, callback) {
  * Unsubscribing to a subscription that already yielded the specified number of messages
  * will clear any pending timeout callbacks.
  *
- * @param {Mixed} sid
+ * @param {Number} sid
  * @param {Number} [opt_max]
  * @api public
  */
@@ -1278,7 +1285,7 @@ Client.prototype.unsubscribe = function(sid, opt_max) {
  * request call, the original timeout handler associated with the multiplexed
  * request is replaced with the one provided to this function.
  *
- * @param {Mixed} sid
+ * @param {Number} sid
  * @param {Number} timeout
  * @param {Number} expected
  * @param {Function} callback
@@ -1330,7 +1337,7 @@ Client.prototype.timeout = function(sid, timeout, expected, callback) {
  * @param {String} subject
  * @param {String} [opt_msg]
  * @param {Object} [opt_options]
- * @param {Function} callback
+ * @param {Function} [callback]
  * @return {Number}
  * @api public
  */
@@ -1413,15 +1420,15 @@ Client.prototype.requestOne = function(subject, opt_msg, opt_options, timeout, c
     }
 
     if (typeof opt_msg === 'number') {
-        timeout = opt_msg;
         callback = opt_options;
-        opt_msg = EMPTY;
+        timeout = opt_msg;
         opt_options = null;
+        opt_msg = EMPTY;
     }
 
     if (typeof opt_options === 'number') {
-        timeout = opt_options;
         callback = timeout;
+        timeout = opt_options;
         opt_options = null;
     }
 
@@ -1463,7 +1470,9 @@ Client.prototype.createResponseMux = function() {
                         client.cancelMuxRequest(token);
                     }
                 }
-                conf.callback(msg);
+                if(conf.callback) {
+                    conf.callback(msg);
+                }
             }
         });
         this.respmux = {};
@@ -1495,6 +1504,11 @@ Client.prototype.initMuxRequestDetails = function(callback, expected) {
     return conf;
 };
 
+/**
+ * Returns the mux request configuration
+ * @param token
+ * @returns Object
+ */
 Client.prototype.getMuxRequestConfig = function(token) {
     // if the token is a number, we have a fake sid, find the request
     if (typeof token === 'number') {
@@ -1538,15 +1552,15 @@ Client.prototype.cancelMuxRequest = function(token) {
  */
 Client.prototype.oldRequestOne = function(subject, opt_msg, opt_options, timeout, callback) {
     if (typeof opt_msg === 'number') {
-        timeout = opt_msg;
         callback = opt_options;
-        opt_msg = EMPTY;
+        timeout = opt_msg;
         opt_options = null;
+        opt_msg = EMPTY;
     }
 
     if (typeof opt_options === 'number') {
-        timeout = opt_options;
         callback = timeout;
+        timeout = opt_options;
         opt_options = null;
     }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "nats",
-  "version": "0.8.0",
+  "version": "0.8.2",
   "description": "Node.js client for NATS, a lightweight, high-performance cloud native messaging system",
   "keywords": [
     "nats",

test/autounsub.js

@@ -168,6 +168,7 @@ describe('Max responses and Auto-unsub', function() {
             nc.publish(reply, 'I can help!');
         });
 
+        /* jshint loopfunc: true */
         // Create 5 requests
         for (var i = 0; i < 5; i++) {
             nc.request('help', null, {
@@ -188,21 +189,16 @@ describe('Max responses and Auto-unsub', function() {
     }
 
     it('should not leak subscriptions when using max', function(done) {
-        /* jshint loopfunc: true */
         var nc = NATS.connect(PORT);
         requestSubscriptions(nc, done);
     });
 
     it('oldRequest should not leak subscriptions when using max', function(done) {
-        /* jshint loopfunc: true */
         var nc = NATS.connect({port: PORT, useOldRequestStyle: true});
         requestSubscriptions(nc, done);
     });
 
     function requestGetsWantedNumberOfMessages(nc, done) {
-        /* jshint loopfunc: true */
-        var nc = NATS.connect({port: PORT, useOldRequestStyle: true});
-
         var received = 0;
 
         nc.subscribe('help', function(msg, reply) {

test/basics.js

@@ -495,4 +495,48 @@ describe('Basics', function() {
             done();
         });
     });
+
+    function paramTranspositions(nc, done) {
+        var all = false;
+        var four = false;
+        var three = true;
+        var count = 0;
+        nc.flush(function() {
+            nc.requestOne("a", NATS.EMPTY, {}, 1, function() {
+                all = true;
+                called();
+            });
+
+            nc.requestOne("b", NATS.EMPTY, 1, function() {
+                four = true;
+                called();
+            });
+
+            nc.requestOne("b", 1, function() {
+                three = true;
+                called();
+            });
+        });
+
+        function called() {
+            count++;
+            if(count === 3) {
+                all.should.be.true();
+                four.should.be.true();
+                three.should.be.true();
+                nc.close();
+                done();
+            }
+        }
+    }
+
+    it('requestOne: optional param transpositions', function (done) {
+        var nc = NATS.connect(PORT);
+        paramTranspositions(nc, done);
+    });
+
+    it('old requestOne: optional param transpositions', function (done) {
+        var nc = NATS.connect({port: PORT, useOldRequestStyle: true});
+        paramTranspositions(nc, done);
+    });
 });