Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable heartbeats If server not using it, but… #43

Closed
wants to merge 1 commit into from
Closed

Disable heartbeats If server not using it, but… #43

wants to merge 1 commit into from

Conversation

sega-yarkin
Copy link

Disable heartbeats If server not using it, but prefer our own heartbeat value from parameters in seconds

@sega-yarkin
Copy link
Author

I'm using RabbitMQ with heartbeat=0, when the client received data.heartbeat=0 and heartbeat=true, it setInterval( ..., 0*1000 ).
But if RabbitMQ is not using heartbeats it can still respond to it.

@bakkerthehacker
Copy link
Owner

It is a bug that allows the data.heartbeat = 0 case to slip through.

However, I don't want to add numbers to the heartbeat parameter for openAMQPCommunication. If more advanced heartbeating is needed, then the client should call connection.tune-ok directly.

@sega-yarkin
Copy link
Author

@bakkerthehacker
Okay, I'm get it, thanks

@sega-yarkin
Copy link
Author

@bakkerthehacker
If I run connection.tune-ok after handle.openAMQPCommunication RabbitMQ close connection with error unexpected method in connection state running 'connection.tune_ok'. What's wrong?

@bakkerthehacker
Copy link
Owner

Currently there is no easy way to replace only one part of the openAMQPCommunication method list. To have a custom connection.tune will require calling all of the necessary initialization methods instead of openAMQPCommunication, as connection.tune is called during this process and cannot be called afterwards. These methods are described in the docs here and listed in the code here.

I am also planning on writing some advanced tutorials that don't use openAMQPCommunication to demonstrate these cases. See issue #36

@sega-yarkin
Copy link
Author

I think it will so low level, in my case I just need to control heartbeats. I did some fixes in my fork and use its, I just need to say to server that I'll use heartbeats and he needs to answer me (rabbitmq don't answer if tune-ok was with heartbeat = 0).

P.S. And look at line https://github.com/bakkerthehacker/bramqp/blob/master/lib/connectionHandle.js#L291 , maybe it should be '1:channel.close'?

@bakkerthehacker
Copy link
Owner

You are correct, thanks for finding that! I have gone through the entire library and added all the channel scoping where necessary.

@giorni
Copy link

giorni commented Sep 10, 2015

Hi @bakkerthehacker, Can you explain the reason to not accept the heartbeat parameter as a number?

@bakkerthehacker
Copy link
Owner

I have been very hesitant about modifying any of the parameters in openAMQPCommunication. It is a common function and it could easily become cluttered with features that very few clients use.

However, seeing as you both have expressed interest in modifying the heartbeats, I can look into expanding the parameter.

@bakkerthehacker
Copy link
Owner

This has been added in 54ad17d and published in 0.3.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants