ruby-amqp / hutch

A system for processing messages from RabbitMQ.
https://gocardless.com/blog/hutch-inter-service-communication-with-rabbitmq/
MIT License
857 stars 137 forks source link

Hutch.connect erases Hutch::Config #256

Open niuage opened 8 years ago

niuage commented 8 years ago

Hey,

We updated Hutch from 0.21 to 0.22.1, and we now have the following issue:

> Hutch::Config.get(:mq_host)
# => xyz.rmq.cloudamqp.com

> Hutch.connected?
# => nil

> Hutch.connect
#2016-10-10T22:14:44Z 3 INFO -- connecting to rabbitmq (amqp://guest@127.0.0.1:5672/)
# [...] Could not establish TCP connection to 127.0.0.1:5672 [...]

> Hutch::Config.get(:mq_host)
# => "127.0.0.1"

Notice how the Hutch::Config value is correct the first time, then after calling .connect, it's different, and is back to the default value?

Looking at the code...

def self.connect(options = {}, config = Hutch::Config)
    return if connected?

    @broker = Hutch::Broker.new(config)
    @broker.connect(options)
end

... it should be using the Hutch::Config class as default, and I checked the value of the mq_host attribute just before.

Interestingly, after all this mess, if I reset the values in the config class, with Hutch::Config.set..., and then call Hutch.connect, it works. Could this be a thread/fork issue? We're on heroku, just fyi.

Any idea about what's going on? Thanks.

zuuno commented 8 years ago

Investigating, it appears to be related to this commit:

https://github.com/gocardless/hutch/commit/6bd0cc8055ae17404c553da2bcecc687d57d4ee5

logger called from the broker resets the connection settings

niuage commented 8 years ago

Good find, thanks for looking into it.

olleolleolle commented 8 years ago

@niuage Are you able to try to try to reproduce with current master? Here's a commits list between that tag (which is latest release) and HEAD: https://github.com/gocardless/hutch/compare/23ad748e6a968cbfa511d31dd4e0cda19bffd8dc...master

The last line of config.rb is "please initialize with the defaults unless something is set". https://github.com/gocardless/hutch/blob/master/lib/hutch/config.rb#L270 In the released version, that line isn't there.

Is that line Hutch::Config.initialize in lib/hutch/logging.rb now superfluous?

olleolleolle commented 8 years ago

@niuage Please re-test with 0.23.1.

olleolleolle commented 7 years ago

@niuage Have you had the time to look into this?

niuage commented 7 years ago

I haven't yet, sorry. I'll try today possibly.