Fix TypeError when handling multi exception #457

Merged
merged 1 commit into from Nov 23, 2013

4 participants

@kwangsu61

When errors occurs using multi/exec, avoid "TypeError: Cannot read property 'length' of undefined"

@kwangsu61 kwangsu61 Fix TypeError when handling multi exception
When errors occurs using multi/exec, avoid "TypeError: Cannot read property 'length' of undefined"
46cd932
@brycebaril
NodeRedis member

Hi @kwangsu61 -- can you provide an example, test case, or existing issue with one of those for me to provide some context?

@kwangsu61

Hello @brycebaril
for example, when Redis server is down (Redis connection to xxx failed - connect ECONNREFUSED),
if multi/exec is executed,
"TypeError: Cannot read property 'length' of undefined"(1056 line : cur.length) occurs.
Thank you

@bpytlik

I just filed #478 because I encountered this issue. I also provided a demo coffeescript program that demonstrates the behavior.

@brycebaril
NodeRedis member

Hi @bpytlik this fix needs more work. Right now this completely breaks the current test suite. My guess is instead of never splicing some sort of logic is required to see when splicing is applicable.

@brycebaril
NodeRedis member

Ahh, nope, I was wrong -- this does not break any of the current tests. Sorry for the misinformation! Will evaluate how to add a test for this.

@bpytlik

@brycebaril I didn't offer a fix, @kwangsu61 did though :) I offered a CS file that demonstrates the issue.

@brycebaril brycebaril merged commit 46cd932 into NodeRedis:master Nov 23, 2013
@brycebaril brycebaril added a commit that referenced this pull request Nov 23, 2013
@brycebaril brycebaril Adding test for #457 c1926cd
@brycebaril
NodeRedis member

Thanks! I went ahead and added a test.

@brycebaril brycebaril added a commit that referenced this pull request May 13, 2014
@brycebaril brycebaril Adding test for #457 ace71b8
@lamujeresponja

I have found this error again in node redis 0.12

@brycebaril
NodeRedis member

@lamujeresponja -- can you reproduce the error, and if so can you provide a code snippet that does? Thanks!

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