mloughran / em-hiredis

Eventmachine redis client
MIT License
221 stars 63 forks source link

Nested callbacks not working #17

Closed dbenamy closed 12 years ago

dbenamy commented 12 years ago

Thanks for the great library!

I'm hitting a weird problem. In some cases a set() is silently failing. It's not setting the value in redis and it isn't calling the callback or errback.

This works:

get = $redis.get('foo')
set = $redis.set("bar", "2") # succeeds
get.callback { |to|
  puts('hi') # shows up

and this doesn't:

get = $redis.get('foo')
get.callback { |to|
  set = $redis.set("bar", "2") # fails without calling set.callback or set.errback
  puts('hi') # shows up

Am I doing something wrong or is this a bug?

Thanks!

Dan

mloughran commented 12 years ago

Hi Dan.

I'm not sure what's going on for you - I certainly nest callbacks all over the place and don't have any issues.

Can you expand your examples so that they're actually runnable, and include the callbacks that don't get called :)

dbenamy commented 12 years ago

I've learned a couple of things:

  1. When I said it never calls the timeout I was wrong. I let it sit a few minutes and eventually it hit the errback so I guess it timed out.
  2. I've got a recursive call which might be an improper stucture for this program. When I take out the recursion, it works.

Here's the full code:

require 'rubygems'

require 'em-hiredis'
require 'eventmachine'

def two()
  puts("In two()")
  res = $redis.get("bar")
  res.callback do
    puts("In two success callback!")
  end
  res.errback do
    puts("In two errback!")
  end
end

def one()
  puts("In one()")
  get = $redis.get("foo")
#  two() # this works
  get.callback do |to|
    two() # with this one, it sits for a while in two() after calling get() and
          # then calls errback
    puts("In get.callback")
  end
end

def process_queue()
  $redis.blpop('queue', 0) do |queue, msg_id|
    one()
    process_queue() # commenting this out makes both cases work
  end
end

EM.run do
  $redis = EM::Hiredis.connect()
  $redis.callback do
    process_queue()
  end
end
mloughran commented 12 years ago

The problem is that you're calling the two function after the clicking operation to redis. You either want to change the code so that you only issue the blocking blpop after finishing processing the item, or use two different redis connections.

Martyn

On 6 Apr 2012, at 02:29, Daniel Benamyreply@reply.github.com wrote:

I've learned a couple of things:

  1. When I said it never calls the timeout I was wrong. I let it sit a few minutes and eventually it hit the errback so I guess it timed out.
  2. I've got a recursive call which might be an improper stucture for this program. When I take out the recursion, it works.

Here's the full code:

require 'rubygems'

require 'em-hiredis'
require 'eventmachine'

def two()
 puts("In two()")
 res = $redis.get("bar")
 res.callback do
   puts("In two success callback!")
 end
 res.errback do
   puts("In two errback!")
 end
end

def one()
 puts("In one()")
 get = $redis.get("foo")
#  two() # this works
 get.callback do |to|
   two() # with this one, it sits for a while in two() after calling get() and
         # then calls errback
   puts("In get.callback")
 end
end

def process_queue()
 $redis.blpop('queue', 0) do |queue, msg_id|
   one()
   process_queue() # commenting this out makes both cases work
 end
end

EM.run do
 $redis = EM::Hiredis.connect()
 $redis.callback do
   process_queue()
 end
end

Reply to this email directly or view it on GitHub: https://github.com/mloughran/em-hiredis/issues/17#issuecomment-4989251

dbenamy commented 12 years ago

Cool. Thanks for the help!