I have a problem with making Promise working as expected. I need to do following thing:

I get file names from stdout, split them into line and copy them. When copy operation is finished i want to start other operations and here is my problem.

I've created a copy function inside Promise, in case of error i reject it immediately, if thereare no errors i resolve it after copy in loop is finished but for some reason the function inside then() gets executed before copy operation is done

var lines  = stdout.split(/\r?\n/);

copyUpdatedFiles(lines).then(
    function() {
       console.log('this one should be executed after copy operation');
    }
);

function copyUpdatedFiles(lines) {
    return new Promise(function(resolve, reject) {
        for (var i = 0; i < linesLength; i++) {
            fs.copy(lines[i], target, function(err) {
                if (err) {
                    reject();
                }
            });
        }
        resolve();
    });
}

Please help cause i'm clearly missing something.

I have a problem with making Promise working as expected. I need to do following thing:

I get file names from stdout, split them into line and copy them. When copy operation is finished i want to start other operations and here is my problem.

I've created a copy function inside Promise, in case of error i reject it immediately, if thereare no errors i resolve it after copy in loop is finished but for some reason the function inside then() gets executed before copy operation is done

var lines  = stdout.split(/\r?\n/);

copyUpdatedFiles(lines).then(
    function() {
       console.log('this one should be executed after copy operation');
    }
);

function copyUpdatedFiles(lines) {
    return new Promise(function(resolve, reject) {
        for (var i = 0; i < linesLength; i++) {
            fs.copy(lines[i], target, function(err) {
                if (err) {
                    reject();
                }
            });
        }
        resolve();
    });
}

Please help cause i'm clearly missing something.

• javascript
• node.js
• promise

Share Improve this question Follow asked Jul 15, 2016 at 13:14 Łukasz KoronaŁukasz Korona 6311 silver badge55 bronze badges

Add a ment  | 

2 Answers 2

Sorted by: Reset to default 5

It gets resolved as soon as you call resolve, which you're doing after starting the copies but before they finish. You have to wait for the last callback before you resolve. That means keeping track of how many you've see, see the *** ments:

function copyUpdatedFiles(lines) {
    return new Promise(function(resolve, reject) {
        var callbacks = 0;                              // ***
        for (var i = 0; i < linesLength; i++) {
            fs.copy(lines[i], target, function(err) {
                if (err) {
                    reject();
                } else {                                // ***
                    if (++callbacks == lines.length) {  // ***
                        resolve();                      // ***
                    }                                   // ***
                }                                       // ***
            });
        }
    });
}

---

Alternately, there are a couple of libraries out there that promise-ify NodeJS-style callbacks so you can use standard promise position techniques like Promise.all. If you were using one of those, you'd just do something this:

function copyUpdatedFiles(lines) {
    return Promise.all(
        // CONCEPTUAL, semantics will depend on the promise wrapper lib
        lines.map(line => thePromiseWrapper(fs.copy, line, target))
    );
}

---

Side note: Your loop condition refers to a variable linesLength that isn't defined anywhere in your code. It should be lines.length.

You don't wait for the copy to success before resolving the promise, after the for, all fs.copy has been put in the call stack, but they didn't plete.

You can either use a counter inside the callback of fs.copy and call resolve once every callback has been called, or use async.

var async = require('async');
var lines  = stdout.split(/\r?\n/);

copyUpdatedFiles(lines).then(
    function() {
       console.log('this one should be executed after copy operation');
    }
);

function copyUpdatedFiles(lines) {
    return new Promise(function(resolve, reject) {
        async.map(lines, (line, callback) => {
            fs.copy(line, target, (err) => {
                callback(err);
            });
        },
        (err) => {
            if(err) {
                reject();
            } else {
                resolve();
            }
        });
    });
}