diff --git a/lib/parser/hiredis.js b/lib/parser/hiredis.js index 9dba8c93e8..e7db7433f7 100644 --- a/lib/parser/hiredis.js +++ b/lib/parser/hiredis.js @@ -27,15 +27,20 @@ HiredisReplyParser.prototype.reset = function () { HiredisReplyParser.prototype.execute = function (data) { var reply; this.reader.feed(data); - try { - while ((reply = this.reader.get()) !== undefined) { - if (reply && reply.constructor === Error) { - this.emit("reply error", reply); - } else { - this.emit("reply", reply); - } + while (true) { + try { + reply = this.reader.get(); + } catch (err) { + this.emit("error", err); + break; + } + + if (reply === undefined) break; + + if (reply && reply.constructor === Error) { + this.emit("reply error", reply); + } else { + this.emit("reply", reply); } - } catch (err) { - this.emit("error", err); } }; diff --git a/tests/hiredis_parser.js b/tests/hiredis_parser.js new file mode 100644 index 0000000000..f1515b110b --- /dev/null +++ b/tests/hiredis_parser.js @@ -0,0 +1,38 @@ +var Parser = require('../lib/parser/hiredis').Parser; +var assert = require('assert'); + +/* +This test makes sure that exceptions thrown inside of "reply" event handlers +are not trapped and mistakenly emitted as parse errors. +*/ +(function testExecuteDoesNotCatchReplyCallbackExceptions() { + var parser = new Parser(); + var replies = [{}]; + + parser.reader = { + feed: function() {}, + get: function() { + return replies.shift(); + } + }; + + var emittedError = false; + var caughtException = false; + + parser + .on('error', function() { + emittedError = true; + }) + .on('reply', function() { + throw new Error('bad'); + }); + + try { + parser.execute(); + } catch (err) { + caughtException = true; + } + + assert.equal(caughtException, true); + assert.equal(emittedError, false); +})();