connection error event behavior doesn't match documentation in amqp.node 0.4.0

[mike-lang]

The API Documentation states in the section on connections:

#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

In amqp.node 0.3.2, I see the expected results when I close a connection from my rabbitMQ server. An error event is raised with an error object of:

[Error: Connection closed: 320 (CONNECTION-FORCED) with message "CONNECTION_FORCED - Closed via management plugin"]

In amqp.node 0.4.0, no error event is raised. Only the close event is raised.

[puncha]

Yes, I also encountered this issue. The event is not triggered. This is a regression between 0.3.x to 0.4.0

[squaremo]

Yes you are right, that this changed. It was at least a deliberate change, as discussed in #110.

I can see it is still desirable to distinguish between "I closed the connection" and "an operator closed the connection" (at the minute these are distinguished only by having a different message string passed to the event). Hmm. Perhaps it should emit an error object with the close event if it was closed by an operator, and nothing if it was closed by the application? It gets complicated quickly, doesn't it.

[mike-lang]

Passing the error object that was passed to the error event on server forced closures to the close event would work for me.

I don't really understand why this was moved off of the error event. If a server restart manifests on the client as a 320 it seems most convenient to surface that on the same event as where intermittent network connectivity issues would also surface. Generally I'd think they'd be handled the same - try to reconnect a fixed number of times and if that fails, notify the user one way or another. I don't really have a stake in how this is designed though. Just my two cents.

If one way or another I can distinguish when a connection close was server initiated from the object passed to the whatever event handler is invoked it'd save me a lot of trouble. The error object that used to get passed to the error event would do the trick. I think I'd have to manually remove my reconnection event listeners every time my client code invoked close otherwise. What a pain.

[mike-lang]

Oh I see. If you emit it as an error message and the client doesn't have an error event handler, it crashes the whole process, which I guess is bad for people who are ok with their connections just closing abruptly on their own I guess?

[squaremo]

If you emit it as an error message and the client doesn't have an error event handler, it crashes the whole process, which I guess is bad for people who are ok with their connections just closing abruptly on their own I guess?

Yes, I think the idea was to make an operator-initiated close, which covers server restarts, unexceptional. You'd most likely still want to reconnect, but to do so without crashing -- an error on the connections is supposed to mean the client has done something wrong.

Passing the error object that was passed to the error event on server forced closures to the close event would work for me.

Cool. I think the above, plus a predicate on errors (say isFatal), would do the trick.

[mike-lang]

👍 Thanks!