Skip to content

Commit

Permalink
Enabling Redis client error logging by default
Browse files Browse the repository at this point in the history
PR: #269
  • Loading branch information
iliyanyotov authored and wavded committed Aug 16, 2019
1 parent 5469fcb commit a50fbcc
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 6 deletions.
2 changes: 1 addition & 1 deletion Readme.md
Expand Up @@ -43,7 +43,7 @@ The following additional params may be included:
- `prefix` Key prefix defaulting to "sess:"
- `unref` Set `true` to unref the Redis client. **Warning**: this is [an experimental feature](https://github.com/mranney/node_redis#clientunref).
- `serializer` An object containing `stringify` and `parse` methods compatible with Javascript's `JSON` to override the serializer used
- `logErrors` Whether or not to log client errors. (default: `false`\)
- `logErrors` Whether or not to log client errors. (default: `true`\)
- If `true`, a default logging function (`console.error`) is provided.
- If a function, it is called anytime an error occurs (useful for custom logging)
- If `false`, no logging occurs.
Expand Down
14 changes: 9 additions & 5 deletions lib/connect-redis.js
Expand Up @@ -68,6 +68,9 @@ module.exports = function (session) {

this.serializer = options.serializer || JSON;

this.logErrors = options.logErrors !== undefined ? options.logErrors : true;
delete options.logErrors;

if (options.url) {
options.socket = options.url;
}
Expand All @@ -87,15 +90,16 @@ module.exports = function (session) {
}

// logErrors
if(options.logErrors){
// if options.logErrors is function, allow it to override. else provide default logger. useful for large scale deployment
if (this.logErrors) {
// if this.logErrors is function, allow it to override. else provide default logger. useful for large scale deployment
// which may need to write to a distributed log
if(typeof options.logErrors != 'function'){
options.logErrors = function (err) {
if (typeof this.logErrors != 'function') {
this.logErrors = function (err) {
console.error('Warning: connect-redis reported a client error: ' + err);
};
}
this.client.on('error', options.logErrors);

this.client.on('error', this.logErrors);
}

if (options.pass) {
Expand Down
21 changes: 21 additions & 0 deletions test/connect-redis-test.js
Expand Up @@ -209,4 +209,25 @@ test('serializer', function (t) {
return lifecycleTest(store, t);
});

test('logErrors', function (t) {
// Default to true, thus using console.error
var store = new RedisStore();
t.equal(typeof store.logErrors, 'function');

// Disabled logging
store = new RedisStore({
logErrors: false,
});
t.equal(store.logErrors, false);

// Using custom function
var logErrors = function(error) {
console.warn('Error caught: ', error);
};
store = new RedisStore({
logErrors,
});
t.equal(store.logErrors, logErrors);
});

test('teardown', redisSrv.disconnect);
5 changes: 5 additions & 0 deletions test/redis-server.js
Expand Up @@ -8,6 +8,11 @@ exports.connect = function () {
'--port', port,
'--loglevel', 'notice',
], { stdio: 'inherit' });

redisSrv.on('error', function (error) {
console.error('Error caught spawning the server: ', error);
});

return P.delay(1500);
};

Expand Down

0 comments on commit a50fbcc

Please sign in to comment.