Dangling connections on socket timeout

[ArenSH]

Hi,

There is an issue with socket timeout: on some network conditions we may get dangling connections.

Assume we had some problem in network such as rabbitMQ server can see client but client cannot get packages from server. Our client tried to recreate connection that closed due to heartbeat error and after 2 attempts connection was reestablished. But rabbitMQ server keeps resending packages for our previous attempts so we now have three connections: one that client use, and two dangling for previous attempts. And that connection have no error listener so any error will crash our client app.

Basically, any condition that cause tcp to resend "opening" packages will cause dangling connections.

Steps to reproduce:

1. Setup rabbitMQ server in docker docker run -d rabbitmq:latest
2. get ip of container docker inspect -f '{{ .NetworkSettings.IPAddress }} <container_id>
3. Emulate network error to get socket timeout sudo iptables -I INPUT -s <container_ip> -j DROP
4. Run test script below
5. Wait for several connection attempts, then recover network sudo iptables -D INPUT -s <container_ip> -j DROP
6. Now we have multiple connections to server and only one of them are available to us.

This lead to various unhadled errors (Heartbeat timeout, ECONNRESET, etc.)

This issue can be fixed by calling connection.toClosed on socket timeout, but i'm not shure if its correct way (lib/connect.js):

sock.setTimeout(timeout, function() {
    var error = new Error('connect ETIMEDOUT');
    if (c) c.toClosed(error);
    openCallback(error);
});

Please, take a look at this issue and let me know if pull request is required.

Test script:

'use strict';

const amqp = require('amqplib');

function openConnection() {
    amqp.connect('amqp://<container_ip>:<container_port>?heartbeat=5', {timeout: 7000})
        .then(function(con) {
            console.log('Open connection');
            con.on('close', openConnection);
            con.on('error', function(err) { console.log(err.message); });
        })
        .catch(function(err) {
            console.log(err.message);
            openConnection();
        });
    }

openConnection();

[squaremo]

This is a subtle issue, thank you for the detailed report.

This issue can be fixed by calling connection.toClosed on socket timeout, but i'm not shure if its correct way (lib/connect.js):

toClosed goes through the whole AMQP closing handshake, which I don't think is required, since we have not competed the opening handshake at that point. I think closing the TCP socket (sock.end() and sock.destroy()) should be enough.

[ArenSH]

@squaremo Could you please tell when will this fix land in npm package?