Handle Success/Error Responses from ssh2 using Promises

Question

I'm building a node.js app which in production will act as a SSH client to many servers, some of which may be inaccessible at any given time. I'm trying to write a function which attempts to run a SSH command with each client in its config upon startup, and I'm not able to handle both successful sessions and those which end in error. I wrapped a ssh2 client in a promise. If I remove the third (trash) server and only successes result, this works fine! See the output:

STDOUT: Hello World

STDOUT: Hello World

Session closed
Session closed
Successful session: Hello World,Hello World

But if one of the connections times out, even though I handle the error, I don't get to keep any of my data. It looks like the error message overwrites all of the resolved promises

Successful session: Error: Timed out while waiting for handshake,Error: 
Timed out while waiting for handshake,Error: Timed out while waiting 
for handshake

Here's my code, forgive me if this is a bit scattered, as I've combined a few of my modules for the sake of this question. My goal is to keep the data from the successful session and gracefully handle the failure.

var Client = require('ssh2').Client;

 const labs = {
    "ny1": "192.168.1.2",
    "ny2": "192.168.1.3",
    "ny3": "1.1.1.1"
};

function checkLabs() {
    let numLabs = Object.keys(labs).length;
    let promises = [];

    for(i=0;i<numLabs;i++){
        let labName = Object.keys(labs)[i];
        promises.push(asyncSSH("echo 'Hello World'", labs[labName]));
    }

    Promise.all(promises.map(p => p.catch(e => e)))
        .then(results => console.log("Successful session: " + results))
        .catch(e => console.log("Error! " + e));
}

var sendSSH = function (command, dest, callback) {
var conn = new Client();

        conn.on('ready', function() {
            return conn.exec(command, function(err, stream) {
                if (err) throw err;
                stream.on('data', function(data) {
                    callback(null, data);
                    console.log('STDOUT: ' + data);
                }).stderr.on('data', function(data){
                    callback(err);
                    console.log('STDERR: ' + data);
                }).on('close', function(err) {
                    if(err) {
                        console.log('Session closed due to error');
                    } else {
                        console.log('Session closed');
                    }
                });
                stream.end('ls -l\nexit\n');
            });
        }).on('error', function(err){
            callback(err);
        }).connect({
            host: dest,
            port: 22,
            username: 'root',
            readyTimeout: 10000,
            privateKey: require('fs').readFileSync('link-to-my-key')
        });
};

function asyncSSH(command, dest) {
    return new Promise(function(resolve, reject){
        sendSSH(command, dest, function(err,data) {
            if (!err) {
                resolve(data);
            } else {
                reject(err);
            }
        });
    });
}

checklabs();

How can I better use this promise wrapper to handle whatever errors come from the ssh2 client? Any tips are appreciated.

Answer 1

To get best use from each connection, you can (and arguably should) promisify separately :

• the instatiation of each Client() instance
• each instance's conn.exec() method (and any other asynchronous methods as required)

This will allow each instance of Client() to be reused with different commands (though not required in this example).

You should also be sure to disconnect each socket when its job is done, by calling client_.end(). For this, a "disposer pattern" is recommended.

With those points in mind and with a few assumptions, here's what I ended up with :

var Client = require('ssh2').Client;

const labs = {
    "ny1": "192.168.1.2",
    "ny2": "192.168.1.3",
    "ny3": "1.1.1.1"
};

function checkLabs() {
    let promises = Object.keys(labs).map((key) => {
        return withConn(labs[key], (conn) => { 
            return conn.execAsync("echo 'Hello World'")
            .catch((e) => "Error: " + e.message); // catch in order to immunise the whole process against any single failure.
                                                  // and inject an error message into the success path.
        });
    });
    Promise.all(promises)
    .then(results => console.log("Successful session: " + results))
    .catch(e => console.log("Error! " + e.message)); // with individual errors caught above, you should not end up here.
}

// disposer pattern, based on https://stackoverflow.com/a/28915678/3478010
function withConn(dest, work) {
    var conn_;
    return getConnection(dest).then((conn) => {
        conn_ = conn;
        return work(conn);
    }).then(() => {
        if(conn_) {
            conn_.end(); // on success, disconnect the socket (ie dispose of conn_).
        }
    }, () => {
        if(conn_) {
            conn_.end(); // on error, disconnect the socket (ie dispose of conn_).
        }
    });
    // Note: with Bluebird promises, simplify .then(fn,fn) to .finally(fn).
}

function getConnection(dest) {
    return new Promise((resolve, reject) => {
        let conn = promisifyConnection(new Client());
        conn.on('ready', () => {
            resolve(conn);
        })
        .on('error', reject)
        .connect({
            host: dest,
            port: 22,
            username: 'root',
            readyTimeout: 10000,
            privateKey: require('fs').readFileSync('link-to-my-key')
        });
    });
}

function promisifyConnection(conn) {
    conn.execAsync = (command) => { // promisify conn.exec()
        return new Promise((resolve, reject) => {
            conn.exec(command, (err, stream) => {
                if(err) {
                    reject(err);
                } else {
                    let streamSegments = []; // array in which to accumulate streamed data
                    stream.on('close', (err) => {
                        if(err) {
                            reject(err);
                        } else {
                            resolve(streamSegments.join('')); // or whatever is necessary to combine the accumulated stream segments
                        }
                    }).on('data', (data) => {
                        streamSegments.push(data);
                    }).stderr.on('data', function(data) {
                        reject(new Error(data)); // assuming `data` to be String
                    });
                    stream.end('ls -l\nexit\n'); // not sure what this does?
                }
            });
        });
    };
    // ... promisify any further Client methods here ...
    return conn;
}

NOTES:

• the promisification of conn.exec() includes an assumption that data may be received in a series of segments (eg packets). If this assumption is not valid, then the need for the streamSegments array disappears.
• getConnection() and promisifyConnection() could be written as one function but with separate function it's easier to see what's going on.
• getConnection() and promisifyConnection() keep all the messy stuff well away from the application code.