0

I have this function for deleting specific jars from a remote server:

deleteJars: function(appDir, version, callback) {
    fs.readFile('/file/location', function(err, data) {

        if(err) throw err;
        var array = data.toString().split("\n");
        for(i in array) {
            if (array[i].indexOf('worker') > -1){

                var ip = array[i].split(" ");
                var ssh = new SSH2Utils();
                var server = {
                  host: ip[0],
                  username: username,
                  password: password
                };
                var myfiles = ssh.exec(server, 'rm ' + appDir + '/' + version + '/jars/myjar*.jar', function(err,stdout,stderr, server, conn, response){
                    if(err) console.log('No jars to delete');
                    conn.end();
                    callback(response);
                });
            }
        }
    });
  }

It gets called in my application with this:

runningService.deleteJars(appDir, version, function() {

});

Immediately after this I have a smilar call to a copyJars finction which copies new jar files to the same location and after that a job is run which uses trhe jars. My problem is that sometimes the copy is done before the delete so the new jars are copied to the folder and immediately deleted with the old ones. Have I done something wrong with my delete function that allows the application to continue to the next step before completing the delete?

pac
  • 491
  • 1
  • 7
  • 30
  • Yes - this is expected. Node is async. So, you will have to deal with it in async way. – Faizuddin Mohammed Mar 05 '18 at 15:45
  • Any advice on how to do that? – pac Mar 05 '18 at 15:45
  • Possible duplicate of [How to return value from an asynchronous callback function?](https://stackoverflow.com/questions/6847697/how-to-return-value-from-an-asynchronous-callback-function). Things discussed in that post will help you here, although this isn't an exact duplicate. – Carcigenicate Mar 05 '18 at 15:47

2 Answers2

1
{
    deleteJars: function(appDir, version, callback) {
        fs.readFile('/file/location', function (err, data) {

            if (err) throw err;
            var array = data.toString().split("\n");
            const promises = [];

            for (i in array) {
                if (array[i].indexOf('worker') > -1) {

                    var ip = array[i].split(" ");
                    var ssh = new SSH2Utils();
                    var server = {
                        host: ip[0],
                        username: username,
                        password: password
                    };
                    promises.push(
                        new Promise((resolve, reject) => {
                            ssh.exec(server, 'rm ' + appDir + '/' + version + '/jars/myjar*.jar', function (err, stdout, stderr, server, conn, response) {
                                if (err) reject('No jars to delete');
                                conn.end();
                                resolve(response);
                            })
                        })
                    )
                }
            }

            Promise.all(promises).then((results) => { // results will be in order
                // use the results array
                callback(null, results);
            }).catch(err => {
                console.log(err);
            })
        });
    }
}

With callbacks, maintaining order might be a little tough - but with Promises - we have a simple API called Promise.all that you can use like above. To continue using callbacks, you can look into libraries like async that have ways to deal with this.

Faizuddin Mohammed
  • 4,118
  • 5
  • 27
  • 51
1

Without seeing how you have this worked out with your copyJars function, this seems like a classic synchronous vs asynchronous issue.

You are using an asynchronous function, fs.readFile, with a callback . If I understand the basis of how Node.js works correctly, it hands the operation of finding and opening the file up and reading the contents to the OS, when that is done, the OS returns to node and says "here it is" and then node executes the callback with the file data. This means that while the OS is off finding the files you want to delete, node will continue executing code which seems to be your copyJars function. Depending on how quickly the OS returns to your node process with the required information, this may not happen in the order you expected.

Some solutions may be:

  1. You can use fs.readFileSync. This will execute synchronously and halt execution of other code while this is being performed. deleteJars sounds like it's not a one time thing, so this may not be the most efficient solution.

  2. You can implement promises or look into async/await.

Austin
  • 114
  • 7