postwait / node-amqp

[UNMAINTAINED] node-amqp is an AMQP client for nodejs
MIT License
1.69k stars 357 forks source link

QueueDeclare error when reconnect [suggestion] #282

Open jgato opened 10 years ago

jgato commented 10 years ago

Sorry for another issue. We are making an intensive usage of node-amqp so we are finding some issues (maybe because a not correct usage). I am trying to learnt how node-amqp works, so I could collaborate and contribute.

By the moment I have realized an important issue. If you try to stablish a connection to a queue but with wrong params, for example undefined name:

connection.queue(name = undefined, options, function(queue) { })

This will produce an error, close, reconnect chain. In the reconnect process the failed queue will be called again with the "channelOpenOk" event that will try to reconnect to current queues.

this.connection._sendMethod(channel, methods.queueDeclare, { reserved1: 0 , queue: this.name , passive: !!this.options.passive , durable: !!this.options.durable , exclusive: !!this.options.exclusive , autoDelete: !!this.options.autoDelete , noWait: false , "arguments": this.options.arguments || {} });

Because of the configuration (empty name) is wrong this produce a new error, close, reconnect chain and therefore an infinite loop. An easy solutioni could be to test correct names in declaration:

Connection.prototype.queue = function (name /* options, openCallback */) { var options, callback; ... if (name == undefined ) || (name == "") console.log("Warning declaring queue with an empty name"); ... }

I dont know which error I could emmit, or how to notify a wrong declaration. If your prefer I could make pull request.

gjohnson commented 10 years ago

I would just log the error and exit.

connection.on('error', function (err) {
  connection.close();
  console.error(err.stack)
  process.exit(1);
});
jgato commented 10 years ago

Sorry @gjohnson it is not a matter of exiting the process. For example, in my case... inside the connection I have a socket.io server sending messages to clients, so, I cannot exit the process if something goes wrong. This is a bug, because a wrong call to connection.queue provoke an infinite loop.