Adding `HEARTBEAT` send support

[shymega]

Hi there,

I'm thinking of adding HEARBEAT send support to Stompex- I have a fairly good idea how to implement it from the STOMP specification. The question is, where should I add the code for this? Is there a specific convention for developers to use on this project?

Thanks!

[pareeohnos]

Currently most of that sort of logic lives in stompex.ex but to be perfectly honest, it's starting to get a little cluttered, so I'm thinking at some point of splitting it out into a number of other modules. But if you wanna take a crack at it, stompex.ex would be the place

[shymega]

Hi,

pareeohnos notifications@github.com writes:

Currently most of that sort of logic lives in stompex.ex but to be perfectly honest, it's starting to get a little cluttered, so I'm thinking at some point of splitting it out into a number of other modules.

Yeah, I figured that out from looking over the code just after reporting that bug (I know, the wrong way round!!).

I think it would be a good idea to split it out into different modules. Perhaps an issue should be created for that?

Another thing I thought about was how there were functions in stompex.ex, such as handle_info, which specifically says in the docstring that "This function should not be invoked directly (..snip..)"- surely they should be marked as private functions? I can make a PR to fix this, but I'm not sure about the safety of doing that.

But if you wanna take a crack at it, stompex.ex would be the place

Yeah, I'm having a bit of a crack at the moment. Currently working my way through the spec to understand. AFAICT, I don't need to implement sending, but I do need to implement the HEARBEAT header into the initial handshake..

Wish me luck!

-- Best regards, Dom Rodriguez

`Top-posting causes armed conflicts.' ~~ anon

[pareeohnos]

Yeah probably not a good idea, will get one opened in a bit. I was thinking of restructuring to also cater for the different specification versions, as there are a few differences between 1.0, 1.1 and 1.2 (although can't think of them off the top of my head)

The handle_info function needs to stay public (least I'm pretty sure it does) as it's part of the way GenServer works. Stompex starts up a GenServer instance, and all messages received by other processes that aren't send via a cast or call will be handled there, so I would've thought that by making it private, it would stop receiving the TCP message.

As for the heartbeats themselves, the spec seems to state that the header must be supplied on connection which is already possible in stompex, but the sending of the heartbeat is a little fiddly I think. If you're not sending any data to the server it's pretty straight forward, you just send it a new line. If you are sending data to the server however, then the heartbeat only needs to be sent when there isn't any other message being sent in the time frame specified. As per the spec:

- the sender MUST send new data over the network connection at least every <n> milliseconds
- if the sender has no real STOMP frame to send, it MUST send an end-of-line (EOL)
- if, inside a time window of at least <n> milliseconds, the receiver did not receive any new data, it MAY consider the connection as dead
- because of timing inaccuracies, the receiver SHOULD be tolerant and take into account an error margin

[pareeohnos]

Just checking in on this to see if you'd made any progress? I ask, as I've actually rewritten a lot Stompex due to finding a number of massive issues when we tried using it with another server, and that may have an impact on anything you've done so far.

[shymega]

Hi,

pareeohnos notifications@github.com writes:

Just checking in on this to see if you'd made any progress? I ask, as I've actually rewritten a lot Stompex due to finding a number of massive issues when we tried using it with another server, and that may have an impact on anything you've done so far.

Honestly, no. I've been busy with college assignments, so I haven't really dedicated much time to this.

Don't worry about it having any impact, I'll work around as and when.

-- Best regards, Dom Rodriguez

`Top-posting causes armed conflicts.' ~~ anon

[pareeohnos]

ok great, will get everything updated :)

[shymega]

Update on this, still busy with college assignments, will work on it later this summer.

[pareeohnos]

No problem, haven't had any request for this to be an immediately required feature so not to worry :)

[shymega]

@pareeohnos Did you get my email I sent to you?

[pareeohnos]

@shymega no? Not received anything, what's up?

[shymega]

@pareeohnos Righto. It's a lengthy email, and I didn't particularly want to discuss it on GH - have I got the right email address, then?

[pareeohnos]

@shymega ah there we are, had gone to my spam folder, will reply :)

[shymega]

@pareeohnos Cool, thanks. FWIW, I will mention it on GH at some point - I just don't want it public right now :smile:

[pareeohnos]

@shymega haha yeah no problem, whenever it's ready 😄

[shymega]

@pareeohnos Just replied to your email. (In case it goes to spam again..!)

[shymega]

@pareeohnos Do you have any IM accounts we could have a quick chat on? Having some teething issues with Stompex, I'd find it useful to hear your thoughts in real time..

I'm on the Elixir Slack, and Freenode IRC, as shymega.

Cheers.

[pareeohnos]

Sure, just sent you a message on IRC :)

[shymega]

Still in progress - on hold for a bit until the code is tidied up a bit.