Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

try catch error in 'get' #37

Closed
wants to merge 8 commits into from

4 participants

Yiyu He TJ Holowaychuk dylanmcd fengmk2
Yiyu He

When I use RedisStore.get, if some error throw in the callback and forget to catch it, it will catch by ‘RedisStore.get’ and do callback(err) again!

In the old test.js, when test RedisStore.length, it will throw an error [TypeError: Object # has no method 'length'], but it will catch in RedisStore.get. I think the try catch in function get should only catch JSON.parse error.

Also I update the test.js ,remove test of RedisStore.length and RedisStore.clear.

heyiyu.pt_52624 and others added some commits
TJ Holowaychuk
Owner
tj commented

yikes yeah this is really noisy, and you don't follow the existing styling at all (return fn&&fn(err); etc) also what is multi_redis?

Yiyu He

Sorry. I used to modified this module to suits my own redis module multi_redis, so It's such a noisy. I merged the newest version and fix the code style.

dylanmcd

+1, I'll volunteer to rewrite if the noise is still holding you back from merging

fengmk2

+1

TJ Holowaychuk
Owner
tj commented

yeah ideally we squash so there's not 6 commits

Yiyu He

move this to #65

Yiyu He dead-horse closed this
Davis Ford davisford referenced this pull request in socketstream/socketstream
Closed

RedisStore issues #337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 27, 2011
  1. fix bug of get & update the test

    heyiyu.pt_52624 authored
  2. update test, remove clear & length

    heyiyu.pt_52624 authored
Commits on Jan 5, 2012
  1. fix try catch

    heyiyu.pt_52624 authored
Commits on Mar 16, 2012
  1. Yiyu He

    use multi-redis

    dead-horse authored
Commits on Sep 16, 2012
  1. Yiyu He

    reset to connect-redis

    dead-horse authored
  2. Yiyu He
Commits on Oct 17, 2012
  1. Yiyu He

    fix try catch bug of get & update the test

    heyiyu.pt_52624 authored dead-horse committed
  2. Yiyu He
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 10 deletions.
  1. +8 −7 lib/connect-redis.js
  2. +8 −3 test.js
15 lib/connect-redis.js
View
@@ -85,20 +85,21 @@ module.exports = function(connect){
* @param {Function} fn
* @api public
*/
-
RedisStore.prototype.get = function(sid, fn){
sid = this.prefix + sid;
debug('GET "%s"', sid);
this.client.get(sid, function(err, data){
if (err) return fn(err);
+ if (!data) return fn();
+ var result;
+ data = data.toString();
+ debug('GOT %s', data);
try {
- if (!data) return fn();
- data = data.toString();
- debug('GOT %s', data);
- fn(null, JSON.parse(data));
+ result = JSON.parse(data);
} catch (err) {
- fn(err);
- }
+ return fn(err);
+ }
+ return fn(null, result);
});
};
11 test.js
View
@@ -15,12 +15,12 @@ store.client.on('connect', function(){
store.set('123', { cookie: { maxAge: 2000 }, name: 'tj' }, function(err, ok){
assert.ok(!err, '#set() got an error');
assert.ok(ok, '#set() is not ok');
-
+
// #get()
store.get('123', function(err, data){
assert.ok(!err, '#get() got an error');
assert.deepEqual({ cookie: { maxAge: 2000 }, name: 'tj' }, data);
-
+
// #set null
store.set('123', { cookie: { maxAge: 2000 }, name: 'tj' }, function(){
store.destroy('123', function(){
@@ -29,6 +29,11 @@ store.client.on('connect', function(){
store_alt.client.end();
});
});
- })
+ throw new Error('Error in fn');
+ });
});
});
+
+process.once('uncaughtException', function (err) {
+ assert.ok(err.message === 'Error in fn', '#get() catch wrong error');
+});
Something went wrong with that request. Please try again.