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

Message handlers fail after client error #251

Closed
paulgrav opened this issue Aug 8, 2012 · 9 comments
Closed

Message handlers fail after client error #251

paulgrav opened this issue Aug 8, 2012 · 9 comments
Labels

Comments

@paulgrav
Copy link

paulgrav commented Aug 8, 2012

Summary

I’m currently evaluating Redis+Socket.io and I’m coming across a situation where my
message event handlers stop firing events. I was seeing this after leaving my node.js server
running for a few hours, but now I can always reproduce by publishing a malformatted
message to my Redis server. My work around is to restart my server which is far from
ideal.

Steps to Reproduce

  1. Download this code: https://gist.github.com/3293884
  2. Setup a local Redis instance.
  3. Run the downloaded code in node.js
  4. Send “publish football.match.event foo” to the redis instance.
  5. Send “publish football.match.event "{"foo":"bar"}"” to the redis instance.
  6. Send “publish football.match.event foo” to the redis instance.

Expected Results

The client error handler (line #9) should fire 3 times so I should see this error
repeated 3 times in my logs.

error event - localhost:6379 - SyntaxError: Unexpected token o

Actual Results

I see the above mentioned error message only once. doMessageListen() is never called again.

Notes:

I have redis.debug_mode set to true. When I publish the messages I can see them in my
redis debugging. The following output appears twice in my logs.

net read localhost:6379 id 1: *3
$7
message
$20
football.match.event
$3
foo
@DTrejo
Copy link
Contributor

DTrejo commented Aug 8, 2012

So after sending a malformed command to your redis server, you expect it not to fail?

Please elaborate on what portion of this problem you believe to be an issue with node_redis, and we'll go from there.

Thank you,
D

@paulgrav
Copy link
Author

paulgrav commented Aug 9, 2012

Malformed in that I send a plaintext message instead of one containing JSON. I expect that a non-JSON message to cause JSON.parse() on line #44 to error, for client.on(‘error’) on line #9 to call it’s handler but for subsequent messages to be processed.

The message handlers on lines #40 and #34 are handlers for Redis messages which are no longer called following a single client error. This is why I think it’s a node_redis issue and not node.js or socket.io.

@paulgrav
Copy link
Author

paulgrav commented Aug 9, 2012

I’ve attempted to reduce the issue. https://gist.github.com/3302796
Using the code in that Gist for a node instances. Then sending the following messages. It’s message #3 where I expect to see output receive from sending message #1.

Message #1:

redis-cli publish football.match.event "{\"foo\": \"bar\"}"

produces this output in node.js:

XXX
{ foo: 'bar' }

Message #2

redis-cli publish football.match.event foo

produces this output in node.js:

XXX
error event - localhost:6379 - SyntaxError: Unexpected token o
connected: true

Message #3

redis-cli publish football.match.event "{\"foo\": \"bar\"}"

produces no output in node.js

@paulgrav
Copy link
Author

paulgrav commented Aug 9, 2012

I’ve done a bit of debugging. After sending message #2 the function RedisReplyParser.prototype.execute() is still being called but this.emit(“reply”… never gets called. Now, prior to sending message #3, if I stick a break point on line 72 of node_modules/redis/lib/parser/javascript.js and then manually call RedisReplyParser.prototype.reset() then everything works.

I don’t really understand exactly what’s going on in RedisReplyParser, but it looks like it’s being left in an inconsistent state where it’s unable to process subsequent messages.

@paulgrav
Copy link
Author

paulgrav commented Aug 9, 2012

I’m running node.js on a Mac and it seems that hiredis isn’t being installed/compiled correctly. So I’m using the javascript parser.

On a Solaris box that has hiredis is being used and everything works fine.

@DTrejo
Copy link
Contributor

DTrejo commented Aug 10, 2012

Happy to hear it works well on solaris :)

If you'd like to submit a failing test as well as something that fixes it, I'd be very happy.

Also, make sure you try out master before you do the above, as it may have fixed your issue. There are a lot of commits not yet on npm.

Cheers,
D

@DTrejo DTrejo closed this as completed Aug 10, 2012
@paulgrav
Copy link
Author

Why did you close the ticket?

@DTrejo DTrejo reopened this Aug 10, 2012
@DTrejo
Copy link
Contributor

DTrejo commented Aug 10, 2012

I get trigger happy sometimes, apologies.

@BridgeAR
Copy link
Contributor

This is a parser bug that is fixed in newer versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants