Hello user folks and Brian,
I am doing some work using ActiveMessaging (a13g) and have a few suggested changes for stomp.rb version 1.0.2 that have come up in my testing.
Let me say right up front how much I like the stomp client code, clean and easy to use.
This is just a few things I thought I would pass along to try and make things better.
1) In the unsubscribe method, there is a bug/typo:
Currently it has:
1 def unsubscribe(name, headers = {}, subId=NIL)
2 headers[:destination] = name
3 transmit "UNSUBSCRIBE", headers
4 if @reliable
5 subId = name if subId==NIL
6 @h.delete(subId)
7 end
8 end
Line 6 refers to '@h', should be '@subscriptions' I think - @h does not appear anywhere else in the code.
2) Another minor suggestion/enhancement:
make reconnectDelay configurable on Connection creation.
Since 'socket' is called at the end of the initialize, it has to be set on creation, so a constructor would be lovely.
If not, fine, I can add this in the initialize when Connection gets extended for a13g's stmop adapter.
3) Here's another "problem", or could just be an enhancement.
When you set 'reliable' to true, the expectation is that the Connection will try to reconnect (that is how the 'socket' method seems to be written).
This is true initially. If activemq is down when you get a connection the first time, it will retry every 5 seconds until connecting.
But if activemq goes down, the receive method starts returning nil, and even if the service comes back, it never tries to reconnect, and never resubscribes.
I see in the Client that this causes a break, basically killing the client, but for a "reliable" connection, why not reuse the socket reconnect logic like on creation?
What I would recommend is to have logic in the connection that if the connection is reliable, then when receive returns a null instead of a message object, interpret this as the connection being lost (b/c the nil indicates an EOF was received). In which case, reset the connection as on creation of the connection, and on reconnection (assuming activemq comes back up) it will also resubscribe based on the @subscriptions.
I have this working in the stomp adapter in a13g that extends the Connection object like this:
def receive
super_result = super()
if super_result.nil? && @reliable
$stderr.print "connection.receive returning EOF as nil - resetting connection.\n"
@socket = nil
super_result = super()
end
return super_result
end
The trick here is to reset the @socket to nil - this forces the connection to try and reconnect when 'socket' is next called.
Perhaps similar logic could just be put into the Connection::receive method?
So far that is all I have found - otherwise, everything has worked out great.
Thanks for the work,
Andrew Kuklewicz
Senior Web Application Developer
www.prx.org
