Rascal doesn't reconnect when connection with the broker is dropped about rascal

[cressie176]

I worked out what's going on.

amqplib is catching the error in what it calls it's main accept loop and [re-emitting]((https://github.com/amqp-node/amqplib/blob/bbe579e467866a40ff1c0ae2428c416111340364/lib/connection.js#L429) it as a frame error. This is handled by the socket error code, which expects a socket close due to the connection having been dropped, and does nothing.

I'll change Rascal to emit the error event (and schedule the reconnection) from within a setImmediate. This will cause the error bypasses amqplib's main accept loop and bubble up to the process as expected.

I've tested locally and it is caught by the uncaughtException handler

[cressie176]

Hi @Zoddo. Have you added error handlers as described in the very important section about error handling?

[cressie176]

I added the following code to your example, and the app restarts the connection as expected

  broker.on('error', (err) => {
    console.error({ err })
  });

However I'm confused as to why the application doesn't crash without this. The error event documentation still say

If an EventEmitter does not have at least one listener registered for the 'error' event, and an 'error' event is emitted, the error is thrown, a stack trace is printed, and the Node.js process exits.

I'll continue to investigate

[Zoddo]

Oh, I have an uncaughtException handler, that's why my app didn't crash (and why I didn't explicitly register an error listener).

I believe you check if an error listener is attached to the broker instance and don't automatically reconnect if none are attached?

I confirm explicitly attaching an error listener to the broker instance fix the issue.

[cressie176]

Hi @Zoddo,

Rascal doesn't check whether an error listener is attached. The code simply emits an error then schedules the retry. I don't think that uncaughtException handlers catch error events, so it should have crashed your application.

[cressie176]

Hi @Zoddo

[email protected] emits the error events via setImmediate. I issued a major release in case there are unintended side effects, but I can't see a reason why there would be. You do need to add the explicit error handlers, when I tested using a global uncaughtException handler, it caught the errors but prevented the reconnection / resubscription code from running.

In making the change, I spotted that I had been handling channel close and connection error events incorrectly - the code was shared with the channel close and connection error, and expected an error event. Consequently it threw an exception that was being swallowed by the aforementioned main accept loop. In practice, I'm not sure this mattered though, as connection close and channel close events are only emitted without errors during a client initiated shutdown.