When using the uri config option port defaults to 0/nil if not specified

[michaellennox]

Issue

When configuring hutch using the uri option to connect to the rabbitmq server and you don't specify a port it defaults to 0/nil.

This lead to a morning of confusion as I tried to track down why my new hutch application wouldn't connect to my rabbitmq server.

Expectations

I had expected that when not specifying the port when configuring a connection with a URI it would default to 5672 in a similar manner to using Bunny directly.

2.4.1 :001 > Bunny.new('amqp://myuser:mypass@myrmqinstance.example.com/myvhost')
 => #<Bunny::Session:0x7f8f1b2d7998 myuser@myrmqinstance.example.com:5672, vhost=myvhost, addresses=[myrmqinstance.example.com:5672]>

Actual Outcome

But in actually it sets the port in the Bunny::Session instance to 0

2.4.1 :001 > Hutch::Config.set(:uri, 'amqp://myuser:mypass@myrmqinstance.example.com/myvhost')
 => "amqp://myuser:mypass@myrmqinstance.example.com/myvhost"
2.4.1 :002 > Hutch::Broker.new.open_connection
{"level":"INFO","message":"connecting to rabbitmq (amqp://myuser@myrmqinstance.example.com:/myvhost)","progname":null}
WARN -- #<Bunny::Session:0x7f8c3f04ab00 myuser@myrmqinstance.example.com:0, vhost=myvhost, addresses=[myrmqinstance.example.com:]>: Could not establish TCP connection to myrmqinstance.example.com:0: Bunny::ClientTimeout
{"level":"ERROR", "message":"amqp connection error: could not establish tcp connection to any of the configured hosts","progname":null}
{"level":"FATAL", "message":"couldn't connect to rabbitmq at amqp://myuser@myrmqinstance.example.com:/myvhost. Check your configuration, network connectivity and RabbitMQ logs.","progname":null}

Clearing out apart from the session inspect, note the port of 0 and the lack of a port in the string within addresses:

<Bunny::Session:0x7f8c3f04ab00 myuser@myrmqinstance.example.com:0, vhost=myvhost, addresses=[myrmqinstance.example.com:]>

It would appear that previously Hutch was set up to default to port 5672 (/5671 when amqps) when using the uri config option in this PR specifically this commit. This default appears to have since been removed.

Personally I'd think that defaulting to ports 5672 / 5671 when unspecified would be more in line with a user's expectations than defaulting to port 0 when unspecified.

Would you be interested in PR to restore the functionality that was implemented in #159 & a regression test to stop the default being lost again?

[michaelklishin]

Thank you for providing the examples and spending the time to narrow it down to 3473971. Port 5672 is the default per protocol spec and is used by all client libraries, so I completely agree with you.

A PR with a fix and a test case or two would be most appreciated 👍.

[michaellennox]

thanks @michaelklishin

A PR with a fix and a test case or two would be most appreciated 👍.

Will have a crack at it later this week / this weekend then and hope to submit it for review by start of next week.

[michaellennox]

Closed by #305