search

noodlefrenzy / node-amqp10

amqp10 is a promise-based, AMQP 1.0 compliant node.js client
MIT License
134 stars 56 forks source link

fixes #358 - the connection:opened event should be emited after all operations ? #364

Open karlvlam opened 6 years ago

karlvlam commented 6 years ago

Creating receiver link right after 'connection:opened' event will get duplicated messages. Using process.nextTick() to create the link can avoid the issue.

Please review whether this fix is appropriate or not...

This change is Reviewable

tony-gutierrez commented 6 years ago

@noodlefrenzy Can we get some integrations of these fixes?!

amarzavery commented 6 years ago

@princjef - What do you think?

princjef commented 6 years ago

The current behavior should not be a new thing. Though I don't know the original intent of the code, my understanding of the connection:opened event is that it specifically indicates that the AMQP connection was opened successfully. To that end, the event is currently being fired in the right place.

That being said, you typically need at least a connection+session to really do anything in AMQP. That's what the connected event is for. It tells you when your client is ready to work with links, etc. There isn't a constant for it in the client, but you should be able to listen to that instead of modifying the semantics of the connection:opened event. Give that a shot and let me know if it resolves your issue.

For reference, the connected event is fired at https://github.com/noodlefrenzy/node-amqp10/blob/master/lib/amqp_client.js#L214 and https://github.com/noodlefrenzy/node-amqp10/blob/master/lib/amqp_client.js#L218.