Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fix empty unsub/punsub TypeError calling toString on null #408

Merged
merged 3 commits into from Apr 27, 2013

Conversation

Projects
None yet
2 participants
Contributor

jeffbski commented Mar 26, 2013

When unsubscribe or punsubscribe is called and it has
no subscriptions, the reply[1] is a null which causes
TypeError: Cannot call method 'toString' of null

Failing tests are provided along with fix.

Fix is to check if reply[1] is null before calling toString otherwise
just pass null.

Stack trace for exception:

TypeError: Cannot call method 'toString' of null
    at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65)
    at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300
    at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22
    at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27)
    at Socket.<anonymous> (/Users/barczewskij/projects/node_redis/index.js:82:14)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:396:14)

jeffbski added some commits Mar 26, 2013

failing tests for empty unsub and punsub
When unsubscribe or punsubscribe is called
and there is nothing to unsubscribe from, the reply[1]
argument is a null which causes a TypeError
Cannot call method 'toString' of null

```
TypeError: Cannot call method 'toString' of null
    at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65)
    at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14)
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300:10)
    at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22)
    at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27)
    at Socket.<anonymous> (/Users/barczewskij/projects/node_redis/index.js:82:14)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:396:14)
```
fix empty unsub/punsub TypeError
When unsubscribe or punsubscribe is called and it has
no subscriptions, the reply[1] is a null which causes
`TypeError: Cannot call method 'toString' of null`

Check if reply[1] is null before calling toString otherwise
just pass null.
Owner

brycebaril commented Mar 27, 2013

Hi @jeffbski thanks for your submission!

For me (0.8.22) merging from master the last two of your four tests hang when I run node test -- any ideas?

Contributor

jeffbski commented Mar 27, 2013

Are you running the latest redis stable version 2.6.11?

The reason I became aware of the problem was due to changes from 2.5.11 to 2.6.11.

Owner

brycebaril commented Mar 27, 2013

Looks like I'm running 2.6.9 where I tested it.

If this is redis-version specific we'll need a guard on the test at least. (see other tests that use server_version_at_least())

limit cbtests to 2.6.11 and above
Test hangs on older versions of Redis
Contributor

jeffbski commented Mar 27, 2013

ok, see if this is acceptable.

@brycebaril brycebaril merged commit 383bafd into NodeRedis:master Apr 27, 2013

Owner

brycebaril commented Apr 27, 2013

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment