Print warnings for explicitly requested parsers that do not exist

Remove obsolete variable in the js parser by using already existing ones

Add stringNumbers option

Author: Ruben Bridgewater

README.md

@@ -19,6 +19,8 @@ npm install redis-parser
 ## Usage
 
 ```js
+var Parser = require('redis-parser');
+
 new Parser(options);
 ```
 
@@ -28,7 +30,8 @@ new Parser(options);
 * `returnError`: *function*; mandatory
 * `returnFatalError`: *function*; optional, defaults to the returnError function
 * `returnBuffers`: *boolean*; optional, defaults to false
-* `name`: *javascript|hiredis*; optional, defaults to hiredis and falls back to the js parser if not available
+* `name`: *javascript|hiredis*; optional, defaults to hiredis and falls back to the js parser if not available or if the stringNumbers option is choosen
+* `stringNumbers`: *boolean*; optional, defaults to false. This is only available for the javascript parser at the moment!
 
 ### Example
 
@@ -64,7 +67,9 @@ Library.prototype.streamHandler = function () {
 ```
 You do not have to use the returnFatalError function. Fatal errors will be returned in the normal error function in that case.
 
-And if you want to return buffers instead of strings, you can do this by adding the returnBuffers option.
+And if you want to return buffers instead of strings, you can do this by adding the `returnBuffers` option.
+
+If you handle big numbers, you should pass the `stringNumbers` option. That case numbers above 2^53 can be handled properly without reduced precision.
 
 ```js
 // Same functions as in the first example
@@ -76,6 +81,8 @@ var parser = new Parser({
     returnError: function(err) {
         lib.returnError(err);
     },
+    name: 'javascript', // Use the Javascript parser
+    stringNumbers: true, // Return all numbers as string instead of a js number
     returnBuffers: true // All strings are returned as buffer e.g. <Buffer 48 65 6c 6c 6f>
 });
 

changelog.md

@@ -1,3 +1,9 @@
+## v.1.2.0 - 27 Mar, 2016
+
+Features
+
+-  Added `stringNumbers` option to make sure all numbers are returned as string instead of a js number for precision
+-  The parser is from now on going to print warnings if a parser is explicitly requested that does not exist and gracefully chooses the JS parser
 
 ## v.1.1.0 - 26 Jan, 2016
 

lib/hiredis.js

@@ -2,12 +2,10 @@
 
 var hiredis = require('hiredis');
 
-function HiredisReplyParser(returnBuffers) {
+function HiredisReplyParser(options) {
     this.name = 'hiredis';
-    this.returnBuffers = returnBuffers;
-    this.reader = new hiredis.Reader({
-        return_buffers: returnBuffers
-    });
+    this.options = options;
+    this.reader = new hiredis.Reader(options);
 }
 
 HiredisReplyParser.prototype.parseData = function () {
@@ -16,11 +14,8 @@ HiredisReplyParser.prototype.parseData = function () {
     } catch (err) {
         // Protocol errors land here
         // Reset the parser. Otherwise new commands can't be processed properly
-        this.reader = new hiredis.Reader({
-            return_buffers: this.returnBuffers
-        });
+        this.reader = new hiredis.Reader(this.options);
         this.returnFatalError(err);
-        return void 0;
     }
 };
 

lib/javascript.js

@@ -1,8 +1,6 @@
 'use strict';
 
-var util = require('util');
-
-function JavascriptReplyParser(return_buffers) {
+function JavascriptReplyParser(options) {
     this.name = 'javascript';
     this.buffer = new Buffer(0);
     this.offset = 0;
@@ -12,23 +10,29 @@ function JavascriptReplyParser(return_buffers) {
     this.type = 0;
     this.protocolError = false;
     this.offsetCache = 0;
-    if (return_buffers) {
+    // If returnBuffers is active, all return values are returned as buffers besides numbers and errors
+    if (options.return_buffers) {
         this.handleReply = function (start, end) {
             return this.buffer.slice(start, end);
         };
+    } else {
+        this.handleReply = function (start, end) {
+            return this.buffer.toString('utf-8', start, end);
+        };
+    }
+    // If stringNumbers is activated the parser always returns numbers as string
+    // This is important for big numbers (number > Math.pow(2, 53)) as js numbers are 64bit floating point numbers with reduced precision
+    if (options.string_numbers) {
+        this.handleNumbers = function (start, end) {
+            return this.buffer.toString('ascii', start, end);
+        };
+    } else {
+        this.handleNumbers = function (start, end) {
+            return +this.buffer.toString('ascii', start, end);
+        };
     }
 }
 
-JavascriptReplyParser.prototype.handleReply = function (start, end) {
-    return this.buffer.toString('utf-8', start, end);
-};
-
-function IncompleteReadBuffer(message) {
-    this.name = 'IncompleteReadBuffer';
-    this.message = message;
-}
-util.inherits(IncompleteReadBuffer, Error);
-
 JavascriptReplyParser.prototype.parseResult = function (type) {
     var start = 0,
         end = 0,
@@ -48,7 +52,7 @@ JavascriptReplyParser.prototype.parseResult = function (type) {
             this.chunksSize = this.buffers[0].length;
             // Include the packetHeader delimiter
             this.bigStrSize = packetHeader + 2;
-            throw new IncompleteReadBuffer('Wait for more data.');
+            throw new Error('Wait for more data.');
         }
         // Set the offset to after the delimiter
         this.offset = end + 2;
@@ -60,7 +64,7 @@ JavascriptReplyParser.prototype.parseResult = function (type) {
         // Include the delimiter
         this.offset = end + 2;
         // Return the coerced numeric value
-        return +this.buffer.toString('ascii', start, end);
+        return this.handleNumbers(start, end);
     } else if (type === 43) { // +
         end = this.packetEndOffset();
         start = this.offset;
@@ -74,7 +78,7 @@ JavascriptReplyParser.prototype.parseResult = function (type) {
         reply = [];
         for (var i = 0; i < packetHeader; i++) {
             if (this.offset >= this.buffer.length) {
-                throw new IncompleteReadBuffer('Wait for more data.');
+                throw new Error('Wait for more data.');
             }
             reply.push(this.parseResult(this.buffer[this.offset++]));
         }
@@ -84,8 +88,6 @@ JavascriptReplyParser.prototype.parseResult = function (type) {
         start = this.offset;
         this.offset = end + 2;
         return new Error(this.buffer.toString('utf-8', start, end));
-    } else {
-        return void 0;
     }
 };
 
@@ -107,7 +109,6 @@ JavascriptReplyParser.prototype.execute = function (buffer) {
         this.buffer = Buffer.concat([this.buffer.slice(this.offset), buffer]);
     }
     this.offset = 0;
-    this.protocolError = true;
     this.run();
 };
 
@@ -118,8 +119,8 @@ JavascriptReplyParser.prototype.tryParsing = function () {
         // Catch the error (not enough data), rewind if it's an array,
         // and wait for the next packet to appear
         this.offset = this.offsetCache;
-        this.protocolError = false;
-        return void 0;
+        // Indicate that there's no protocol error by resetting the type too
+        this.type = undefined;
     }
 };
 
@@ -139,7 +140,7 @@ JavascriptReplyParser.prototype.run = function () {
         this.type = this.buffer[this.offset++];
         reply = this.tryParsing();
     }
-    if (this.type !== undefined && this.protocolError === true) {
+    if (this.type !== undefined) {
         // Reset the buffer so the parser can handle following commands properly
         this.buffer = new Buffer(0);
         this.returnFatalError(new Error('Protocol error, got ' + JSON.stringify(String.fromCharCode(this.type)) + ' as reply type byte'));
@@ -162,7 +163,7 @@ JavascriptReplyParser.prototype.packetEndOffset = function () {
         offset++;
 
         if (offset >= len) {
-            throw new IncompleteReadBuffer('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')');
+            throw new Error('Did not see LF after NL reading multi bulk count (' + offset + ' => ' + this.buffer.length + ', ' + this.offset + ')');
         }
     }
     return offset;

lib/parser.js

@@ -10,7 +10,7 @@ try {
 } catch (err) { /* ignore errors */ }
 
 function Parser (options) {
-    var parser;
+    var parser, msg;
 
     if (
         !options ||
@@ -21,18 +21,34 @@ function Parser (options) {
     }
 
     /* istanbul ignore if: hiredis should always be installed while testing */
-    if (options.name === 'hiredis' && !parsers.hiredis) {
-        console.warn('<< WARNING >> You explicitly required the hiredis parser but hiredis is not installed. The js parser is going to be returned instead.');
+    if (options.name === 'hiredis') {
+        if (!parsers.hiredis) {
+            msg = 'You explicitly required the hiredis parser but hiredis is not installed. The JS parser is going to be returned instead.';
+        } else if (options.stringNumbers) {
+            msg = 'You explicitly required the hiredis parser in combination with the stringNumbers option. Only the JS parser can handle that and is choosen instead.';
+        }
+    } else if (options.name && !parsers[options.name]) {
+        msg = 'The requested parser "' + options.name + '" is unkown and the JS parser is choosen instead.';
+    }
+
+    if (msg) {
+        console.warn(new Error(msg).stack.replace('Error: ', 'Warning: '));
+        options.name = 'javascript';
     }
 
     options.name = options.name || 'hiredis';
     options.name = options.name.toLowerCase();
-    options.returnBuffers = options.returnBuffers || false;
 
-    if (options.name === 'javascript' || !parsers.hiredis) {
-        parser = new parsers.javascript(options.returnBuffers);
+    var innerOptions = {
+        // The hiredis parser expects underscores
+        return_buffers: options.returnBuffers || false,
+        string_numbers: options.stringNumbers || false
+    };
+
+    if (options.name === 'javascript' || !parsers.hiredis || options.stringNumbers) {
+        parser = new parsers.javascript(innerOptions);
     } else {
-        parser = new parsers.hiredis(options.returnBuffers);
+        parser = new parsers.hiredis(innerOptions);
     }
 
     parser.returnError = options.returnError;

package.json

@@ -28,10 +28,11 @@
     "node": ">=0.10.0"
   },
   "devDependencies": {
-    "jshint": "^2.8.0",
-    "mocha": "^2.3.2",
+    "codeclimate-test-reporter": "^0.1.1",
+    "intercept-stdout": "^0.1.2",
     "istanbul": "^0.4.0",
-    "codeclimate-test-reporter": "^0.1.1"
+    "jshint": "^2.8.0",
+    "mocha": "^2.3.2"
   },
   "optionalDependency": {
     "hiredis": "^0.4.1"

test/parsers.spec.js

@@ -1,5 +1,6 @@
 'use strict';
 
+var intercept = require('intercept-stdout');
 var assert = require('assert');
 var Parser = require('../');
 var parsers = ['javascript', 'hiredis'];
@@ -28,10 +29,44 @@ describe('parsers', function () {
                     returnReply: returnReply,
                     returnBuffers: true
                 });
-            }, 'Please provide all return functions while initiating the parser');
+            }, function (err) {
+                assert.strictEqual(err.message, 'Please provide all return functions while initiating the parser');
+                return true;
+            });
 
         });
 
+        it('unknown parser', function () {
+            var str = '';
+            var unhookIntercept = intercept(function (data) {
+                str += data;
+                return '';
+            });
+            new Parser({
+                returnReply: returnReply,
+                returnError: returnError,
+                name: 'something_unknown'
+            });
+            unhookIntercept();
+            assert(/^Warning: The requested parser "something_unknown" is unkown and the JS parser is choosen instead\.\n +at new Parser/.test(str), str);
+        });
+
+        it('hiredis and stringNumbers', function () {
+            var str = '';
+            var unhookIntercept = intercept(function (data) {
+                str += data;
+                return '';
+            });
+            new Parser({
+                returnReply: returnReply,
+                returnError: returnError,
+                name: 'hiredis',
+                stringNumbers: true
+            });
+            unhookIntercept();
+            assert(/^Warning: You explicitly required the hiredis parser in combination with the stringNumbers option. .+.\.\n +at new Parser/.test(str), str);
+        });
+
     });
 
     parsers.forEach(function (parserName) {
@@ -401,6 +436,29 @@ describe('parsers', function () {
                 parser.execute(new Buffer('\n'));
                 assert.strictEqual(replyCount, 3);
             });
+
+            it('return numbers as strings', function () {
+                var replyCount = 0;
+                var entries = ['123', '590295810358705700002', '-99999999999999999'];
+                function checkReply(reply) {
+                    assert.strictEqual(typeof reply, 'string');
+                    assert.strictEqual(reply, entries[replyCount]);
+                    replyCount++;
+                }
+                var unhookIntercept = intercept(function () {
+                    return '';
+                });
+                var parser = new Parser({
+                    returnReply: checkReply,
+                    returnError: returnError,
+                    returnFatalError: returnFatalError,
+                    name: parserName,
+                    stringNumbers: true
+                });
+                unhookIntercept();
+                parser.execute(new Buffer(':123\r\n:590295810358705700002\r\n:-99999999999999999\r\n'));
+                assert.strictEqual(replyCount, 3);
+            });
         });
     });
 });