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.
Share Improve this question asked Jul 15, 2016 at 13:14 Łukasz KoronaŁukasz Korona 631 silver badge5 bronze badges2 Answers
Reset to default 5It 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();
}
});
});
}