Łukasz Korona Łukasz Korona - 4 months ago 8
Node.js Question

nodejs then() function executed before promise resolve

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.

Answer

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 *** comments:

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 composition 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.