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.

Sign up for GitHub

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

Jump to bottom

Redis connection timeouts causing protocol errors #127

Closed
jhurliman opened this issue Aug 10, 2011 · 5 comments
Closed

Redis connection timeouts causing protocol errors #127

jhurliman opened this issue Aug 10, 2011 · 5 comments
Labels
Bug

Comments

@jhurliman
Copy link

I have an issue where node_redis is auto-reconnecting to redis and all queries hang indefinitely afterward. This is a related issue to that, where occasionally I will see one of the hung connections terminate with these kind of errors:

9 Aug 19:04:26 - error: Redis error: Error: Redis reply parser error: Error: Protocol error, got "\n" as reply type byte
    at HiredisReplyParser.execute (.../node_modules/redis/lib/parser/hiredis.js:31:37)
    at RedisClient.on_data (.../node_modules/redis/index.js:358:27)
    at Socket.<anonymous> (.../node_modules/redis/index.js:93:14)
    at Socket.emit (events.js:64:17)
    at Socket._onReadable (net.js:678:14)
    at IOWatcher.onReadable [as callback] (net.js:177:10)

9 Aug 19:14:27 - error: Redis error: Error: Redis reply parser error: Error: Protocol error, got "r" as reply type byte
    at HiredisReplyParser.execute (.../node_modules/redis/lib/parser/hiredis.js:31:37)
    at RedisClient.on_data (.../node_modules/redis/index.js:358:27)
    at Socket.<anonymous> (.../node_modules/redis/index.js:93:14)
    at Socket.emit (events.js:64:17)
    at Socket._onReadable (net.js:678:14)
    at IOWatcher.onReadable [as callback] (net.js:177:10)
@pietern
Copy link
Contributor

pietern commented Aug 11, 2011

@mranney: This can be fixed by moving the initializer for this.reply_parser to the on_connect function or the "connect" callback on the socket. This also fixes #126 since it prevents more data to be pushed to a parser in an erroneous state.

mranney added a commit that referenced this issue Aug 11, 2011
@mranney
Fix for [GH-127]
3e95c55 
Re-initialize the reply parser for every new connection.  If a connection is terminated,
the parser could be left in a bad state.  After the auto-reconnect magic kicks in, it
tries to reuse the old parser, which will not work.

This change is visible to client programs if you depend on client.reply_parser.name being
set immediately.  It will now only be set after a connection is established.

Thanks to @jhurliman for reporting and @pietern for the fix suggestion.
@mranney
Copy link
Contributor

mranney commented Aug 11, 2011

@jhurliman: can you please try this change and see if it fixes your issue?

@jhurliman
Copy link
Author

Upgraded to the new version just now, I'll let you know how it goes.

@jhurliman
Copy link
Author

Everything looks good so far, thanks!

@mranney
Copy link
Contributor

mranney commented Aug 12, 2011

Awesome, thanks guys. I'll roll a new version for npm with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@pietern @jhurliman @mranney @BridgeAR