You've already forked node-redis
mirror of
https://github.com/redis/node-redis.git
synced 2025-08-07 13:22:56 +03:00
fbc1642461
While using this module with the hiredis parser, I noticed that exceptions
thrown inside of subscribe/message/etc. callbacks would be reported as
parser error exceptions, which was confusing.
Example:
```
var client = require('./index').createClient();
client.on('subscribe', function() {
throw new Error('My Error');
});
client.subscribe('channel');
```
I would expect the above to throw 'My Error' as a simple exception, however,
when using the hiredis parser I get:
```
events.js:45
throw arguments[1]; // Unhandled 'error' event
^
Error: Redis reply parser error: Error: My Error
at RedisClient.<anonymous> (/Users/felix/code/node_redis/hiredis.js:4:9)
at RedisClient.emit (events.js:67:17)
at RedisClient.return_reply (/Users/felix/code/node_redis/index.js:457:22)
at HiredisReplyParser.<anonymous> (/Users/felix/code/node_redis/index.js:220:14)
at HiredisReplyParser.emit (events.js:64:17)
at HiredisReplyParser.execute (/Users/felix/code/node_redis/lib/parser/hiredis.js:35:22)
at RedisClient.on_data (/Users/felix/code/node_redis/index.js:365:27)
at Socket.<anonymous> (/Users/felix/code/node_redis/index.js:58:14)
...
```
The attached patch fixes this problem by changing the way the HiredisReplyParser
traps exceptions, and also provides a unit test for it.
Please let me know if you need any tweaks on the patch for landing it, I was
not quite sure where to put my test / how to structure it, since most test
cases seem to be integration tests at this point.
Also, if you would prefer an integration test for this, here you go:
``` javascript
var client = require('./index').createClient();
var assert = require('assert');
process.on('uncaughtException', function listener(err) {
process.removeListener('uncaughtException', listener);
// Drop the stack, otherwise the assert gets trapped by node
process.nextTick(function() {
assert.ok(!/parser error/.test(err.message), err.message);
client.quit();
});
});
client.on('subscribe', function() {
throw new Error('My Error');
});
client.subscribe('channel');
```
PS: If you wonder why this only affects pubsub events: This is because
RedisClient#return_reply does exception trapping differently for events than
it does for callbacks.
See: 3e95c55a03/index.js (L431)
Vs: 3e95c55a03/index.js (L447)
47 lines
1.1 KiB
JavaScript
47 lines
1.1 KiB
JavaScript
/*global Buffer require exports console setTimeout */
|
|
|
|
var events = require("events"),
|
|
util = require("../util").util,
|
|
hiredis = require("hiredis");
|
|
|
|
exports.debug_mode = false;
|
|
exports.name = "hiredis";
|
|
|
|
function HiredisReplyParser(options) {
|
|
this.name = exports.name;
|
|
this.options = options || {};
|
|
this.reset();
|
|
events.EventEmitter.call(this);
|
|
}
|
|
|
|
util.inherits(HiredisReplyParser, events.EventEmitter);
|
|
|
|
exports.Parser = HiredisReplyParser;
|
|
|
|
HiredisReplyParser.prototype.reset = function () {
|
|
this.reader = new hiredis.Reader({
|
|
return_buffers: this.options.return_buffers || false
|
|
});
|
|
};
|
|
|
|
HiredisReplyParser.prototype.execute = function (data) {
|
|
var reply;
|
|
this.reader.feed(data);
|
|
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);
|
|
}
|
|
}
|
|
};
|