stompest
stompest copied to clipboard
When connect's host param is left as None the server hostname should be used instead of ''
Right now for STOMP's host header to work at all the host= must be manually set when calling connect. If you don't then stompest will always leave it empty.
Instead of leaving it empty when unset the hostname of the fallback server being connected to should be used.
ie: When you use tcp://localhost:61613
in config and leave host as None then host: localhost
should be used.
ie: When you use fallback:(tcp://a.stomp.local:61613,tcp://b.stomp.local:61613)
, leave host as None, and stompest connected to the second server then host: b.stomp.local
should be used.
That's a good suggestion. Would you mind to provide a patch for that, or do you prefer me to do that?
Right now I'm just working on a side project in my spare time that happens to use STOMP and tracking issues I run into along the way.
If I get into the project seriously with more time to play with I'll write the stuff in myself. But in the meantime if you feel like implementing it go ahead.
I just meditated over this issue, and these questions came up:
- What do you mean by a "working"
host
header? For ActiveMQ, you don't need that header at all to get a connection to "work", and for Apollo it should be a virtual (that is, logical) host rather that a physical address, so I right now don't see the rationale of putting a physical address in that header. The broker will know its physical address anyway. I'd rather expect a virtual host to remain the same upon failover to another broker instance. - What if you used another transport? Say, a non-IP transport. What would you fill in for the
host
then?
To me it looks like one shouldn't intermingle the physical transport and the logical STOMP session, and the code seems to confirm that, because it is not so easy to implement the desired behavior for both clients (sync and async) without mixing transport and STOMP session concerns (breaking SRP) and writing redundant code in both clients (breaking DRY).
@dantman: What's your opinion on this?
Apollo's default (and/or example) configs setup a virtual host named (localhost
, 207.0.0.1
, etc...). When configured like that Apollo will generally reject requests with an empty host
header if you use STOMP 1.1+. By working I'm talking about what you need to do to get something that's non-empty. Because host
doesn't carry over anywhere from config leaving you stuck in an awkward flow where you have to explicitly carry some extra custom config into other parts of your code just to set the host
header.
virtual hosts in STOMP are basically carried over from HTTP. They work the same way. Filing the host
header with whatever address or hostname you used to connect with is the expected behaviour. The purpose is basically so that if you point the dns for foo.tld
and bar.tld
to the same address the host
header will respectively contain foo.tld
and bar.tld
and the server will be able to differentiate between the two. It's practically never an explicit name. There is nothing really strange about two different servers even for fallover getting different hosts. Virtual host configuring is unique to each server. Using the same virtual host name on each server is not a requirement for connecting them to the same messaging group.
The STOMP spec is actually quite clear on this behaviour "It is recommended clients set this to the host name that the socket was established against, or to any name of their choosing." which would basically in our case would translate to defaulting to the hostname of the connected server but providing a config that lets you set a fixed host for all connections.
On a non-IP transport. IF you were directly connecting through it you'd probably leave that blank. host
is inherently an IP transport oriented header. You don't have the same kind of limitations you do in IP with say a unix socket. In that case if there were any header at all it should be explicitly set. (Although for that circumstance we should still add some extra config to allow that to be specified in config instead of forcing it to be passed to connect).
In the Apollo integration tests, I set the host
header to the logical host name mybroker
, on RabbitMQ, it is /
. Both aren't really socket addresses.
But I think you're right: In their connect()
methods, sync
and async
client should store the "name" of the host they connected against on the wire level and substitute a missing host
argument (None
). But the transport level should decide what that "name" is. It is very important to keep both clients transport agnostic.
In the async
client, we should set the host
attribute on the StompProtocol
instance and let the Stomp.connect()
method substitute the host
header here. The definition of frame
has to move further down; there is no reason for it to be so far up anyway.
The StompFrameTransport
of the sync
client already has this host
attribute, so the substitution in Stomp.connect()
can take place in exactly the same way.