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

Fix of reconnect() method + 'passive' queues. #170

Closed

Conversation

sbellone
Copy link

Hello,
I am using node-amqp through a proxy and I figured out two issues. The issues are due to the timeout of the proxy, which closes the connection.

The first issue is about the reconnect() method: When the proxy closes the connection, I receive a 'close' event. I wanted to reconnect manually using the reconnect() method (otherwise I need to wait until an error occurs to fall in the backoff() method), but in this method, the queues' subscriptions are not marked as closed, unlike in the backoff() method.
Thus, the queues will never resubscribe once we are reconnected.

The second issue is about the passive queues. Before sending a message, I want to declare a 'passive' queue, in order to check if it exists and if it has consumers.
But by doing that, we have two issues:

  • the passive queue will override the reference of the active queue in the Connection.queues array. Then, if we reconnect, the active queue will not be able to resubscribe as amqp does not have any reference on its consumerTagListeners anymore.
  • this will open a new channel for each passive queue, which will never be closed. So here I close it immediately after having received the declare-ok response.

The attached patch has resolved these issues in my env, but my amqp knowledge is not huge so I'd like you to have a look and tell me if it seems good to you.

Thanks.

- When doing a manual reconnect(), queues don't resubscribe.
- When creating a passive queue, it overrides the reference to the existing active queue. Thus, when we reconnect(), the active queue never resubscribe.
@jgato
Copy link

jgato commented Jan 27, 2014

Is this path merged? I am having this same issue. With exclusive queue, after re-connecting for any error the queue is not re-subscribed.

@sbellone
Copy link
Author

Unfortunately I never had feedback for this, but it was working very fine in our env.
It's a shame because the patch is very little and it fixes (at least for me) critical issues.

@jgato
Copy link

jgato commented Jan 27, 2014

I will make more depth tests, but the first attempt is not solving my issue :(

My scenarios is:

  • One connection to the server
  • One connection to a queue
  • First client using the queue to subscribe exclusive
  • Second client using the queue to subscribe exclusive
  • Queue Exclusive error, so... connection error, connection close
  • Reconnect does not re-subscribe

I guess the issue is this line:

 case methods.channelClose:
      this.state = "closed";
      this.closeOK();
      this.connection.queueClosed(this.name);
     ...

Because of the error the queue is closed, I guess it would not re-connect.

@sbellone
Copy link
Author

Ok, so this is not the same scenario, on my side I was getting a proper 'close' event due to a proxy timeout.

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.

None yet

3 participants