Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TypeError when handling multi exception #457

Merged
merged 1 commit into from Nov 23, 2013

Conversation

kwangsu61
Copy link
Contributor

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

When errors occurs using multi/exec, avoid "TypeError: Cannot read property 'length' of undefined"
@brycebaril
Copy link
Contributor

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

@kwangsu61
Copy link
Contributor Author

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
Copy link

bpytlik commented Aug 22, 2013

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

@brycebaril
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

bpytlik commented Oct 3, 2013

@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 redis:master Nov 23, 2013
brycebaril added a commit that referenced this pull request Nov 23, 2013
@brycebaril
Copy link
Contributor

Thanks! I went ahead and added a test.

@lamujeresponja
Copy link

I have found this error again in node redis 0.12

@brycebaril
Copy link
Contributor

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants