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

Error event is emitted due to normal connection close from RabbitMQ side #110

Closed
xaka opened this issue Oct 31, 2014 · 10 comments
Closed

Error event is emitted due to normal connection close from RabbitMQ side #110

xaka opened this issue Oct 31, 2014 · 10 comments
Assignees

Comments

@xaka
Copy link

xaka commented Oct 31, 2014

Hi there,

I was verifying how service behaves when RabbitMQ gets restarted, etc. and I've got the following:

Error: Connection closed: 320 (CONNECTION-FORCED) with message "CONNECTION_FORCED - broker forced connection closure with reason 'shutdown'"
    at Object.accept (../node_modules/amqplib/lib/connection.js:89:32)
    at Connection.mainAccept [as accept] (../node_modules/amqplib/lib/connection.js:62:33)
    at Socket.go (../node_modules/amqplib/lib/connection.js:459:48)
    at Socket.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:426:10)
    at emitReadable (_stream_readable.js:422:5)
    at readableAddChunk (_stream_readable.js:165:9)
    at Socket.Readable.push (_stream_readable.js:127:10)
    at TCP.onread (net.js:528:21)

It was absolutely usual RabbitMQ restart procedure and service crashed because it was expecting to get close event, but not error.

In my opinion there should be no error event when library gets 320 code from RabbitMQ. Even though it's called CONNECTION-FORCED, there is nothing wrong happened.

@squaremo
Copy link
Collaborator

squaremo commented Nov 3, 2014

I see what you mean. Would your code be happy if it got a 'close' event instead? (You imply this, just making sure)

@xaka
Copy link
Author

xaka commented Nov 4, 2014

It gets close event, but too late, when everything is about to crash due to unhandled error :) The point is 320 code from RabbitMQ shouldn't be treated as an error, but as usual connection close flow IMHO.

@squaremo squaremo self-assigned this Jan 10, 2015
@vicneanschi
Copy link

Hi squaremo,

did you have a chance to fix that?

@squaremo
Copy link
Collaborator

squaremo commented Jul 2, 2015

I (think I) have fixed this in master, if you would like to give it a try @vicneanschi @xaka

@jgato
Copy link

jgato commented Aug 3, 2015

Hi there,

I would like to detect correctly this connection-forced (close connection) to enable a manual reconnection process. But I have two issues:

  • If I use the .on('error'...) the received object is an error object, but not correctly composed. I mean, only the message attribute is filled, so, if I have to distinguish different errors this will be a problem:

captura de pantalla de 2015-08-03 12 22 53

Somewhere, this error object is not been created correctly.

  • If I use the .on('close', function(msg)... ) but the invoked callback does not receive any parameter.

What should be the best option to detect this closing connection with the server?

@jgato
Copy link

jgato commented Aug 4, 2015

I think it is related to: #149

@squaremo
Copy link
Collaborator

@jgato Is this still a source of problems? My test program

var amqp = require('amqplib/callback_api');

amqp.connect(onOpen);

function onOpen(err, conn) {
  if (err !== null) {
    return console.error(err);
  }
  conn.on('error', function(err) {
    console.log(err);
  });
  conn.connection.stream.write(new Buffer('garbage'));
}

prints out a "proper" error so far as I can tell. (By the way I've added a bit to put the replyCode on the error, and to give the close event the message, as you suggested above)

@mike-lang
Copy link

Is this now expected behavior? Your API documentation seems to clearly state:

#on('error', function (err) {...})

Emitted if the connection closes for any reason other than #close being called; such reasons include:

a protocol transgression the server detected (likely a bug in this library)
a server error
a network error
the server thinks the client is dead due to a missed heartbeat
a human closed the connection with an admin tool

I was going to attach a handler to the error event in my client to attempt to re-establish its connection a fixed number of times upon closure caused by error or by action on the server. Using node 4.1.2 with amqp.node 0.4.0, talking to rabbitMQ 3.5.6, when I close the connection through the server side admin tool, on the client end the close event fires, but the error event does not.

@VMBindraban
Copy link

It seems that this is still an issue on 0.5.1.

@cressie176
Copy link
Collaborator

When I run the following script then gracefully stop the broker I get a channel close event followed by a connection close event. The connection close event still provides an error object though.

const amqp = require('amqplib');

(async () => {
  const connection = await amqp.connect();
  connection.on('error', (err) => {
    console.log('Connection Error', err);
  })
  connection.on('close', (err) => {
    console.log('Connection Closed', err);
  })

  const channel = await connection.createChannel();
  channel.consume('test', (message) => {
    console.log({ message });
  })

  channel.on('error', (err) => {
    console.log('Channel Error', err);
  })
  channel.on('close', (err) => {
    console.log('Channel Closed', err);
  })
})();

Channel Closed undefined
Connection Closed Error: Connection closed: 320 (CONNECTION-FORCED) with message "CONNECTION_FORCED - broker forced connection closure with reason 'shutdown'"
at Object.accept (/Users/steve/Development/cressie176/amqplib-110/node_modules/amqplib/lib/connection.js:91:15)
at Connection.mainAccept (/Users/steve/Development/cressie176/amqplib-110/node_modules/amqplib/lib/connection.js:64:33)
at Socket.go (/Users/steve/Development/cressie176/amqplib-110/node_modules/amqplib/lib/connection.js:478:48)
at Socket.emit (events.js:400:28)
at emitReadable_ (internal/streams/readable.js:555:12)
at processTicksAndRejections (internal/process/task_queues.js:81:21) {
code: 320
}

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

No branches or pull requests

7 participants