Reputation: 15374
I'm trying to get my head around promises, I think I can see how they work in the way that you can say do Step 1
, Step 2
and then Step 3
for example.
I have created this download function using node-fetch
(which uses native Promises)
## FileDownload.js
const fetch = require('node-fetch');
const fs = require('fs');
module.exports = function(url, target) {
fetch(url)
.then(function(res) {
var dest = fs.createWriteStream(target);
res.body.pipe(dest);
}).then(function(){
console.log(`File saved at ${target}`)
}).catch(function(err){
console.log(err)
});
}
So this all executes in order and I can see how that works.
I have another method that then converts a CSV file to JSON (again using a promise)
## CSVToJson.js
const csvjson = require('csvjson');
const fs = require('fs');
const write_file = require('../helpers/WriteToFile');
function csvToJson(csv_file, json_path) {
return new Promise(function(resolve, reject) {
fs.readFile(csv_file, function(err, data){
if (err)
reject(err);
else
var data = data.toString();
var options = {
delimiter : ',',
quote : '"'
};
const json_data = csvjson.toObject(data, options);
write_file(json_path, json_data)
resolve(data);
});
});
}
module.exports = {
csvToJson: csvToJson
}
When I call these functions one after another the second function fails as the first has not completed.
Do I need to wrap these two function calls inside another promise, even though on their own they each have promises implemented?
Please advise if I am totally misunderstanding this
Upvotes: 1
Views: 90
Reputation: 1073998
When I call these functions one after another the second function fails as the first has not completed.
There are two issues with the first:
To deal with the first issue, you have to wait for the finish
event on the destination stream (which pipe
returns). To deal with the second, you need to return a promise that won't be fulfilled until that happens. Something along these lines (see **
comments):
module.exports = function(url, target) {
// ** Return the end of the chain
return fetch(url)
.then(function(res) {
// ** Unfortunately, `pipe` is not Promise-enabled, so we have to resort
// to creating a promise here
return new Promise((resolve, reject) => {
var dest = fs.createWriteStream(target);
res.body.pipe(dest)
.on('finish', () => resolve()) // ** Resolve on success
.on('error', reject); // ** Reject on error
});
}).then(result => {
console.log(`File saved at ${target}`);
return result;
});
// ** Don't `catch` here, let the caller handle it
}
Then you can use then
and catch
on the result to proceeed to the next step:
theFunctionAbove("/some/url", "some-target")
.then(() = {
// It worked, do the next thing
})
.catch(err => {
// It failed
});
(Or async
/await
.)
Side note: I haven't code-reviewed it, but a serious issue in csvToJson
jumped out, a minor issue as well, and @Bergi has highlighted a second one:
It's missing {
and }
around the else
logic
The minor issue is that you have var data = data.toString();
but data
was a parameter of that function, so the var
is misleading (but harmless)
It doesn't properly handle errors in the part of the code in the else
part of the readFile
callback
We can fix both by doing a resolve
in the else
and performing the rest of the logic in a then
handler:
function csvToJson(csv_file, json_path) {
return new Promise(function(resolve, reject) {
fs.readFile(csv_file, function(err, data){
if (err)
reject(err);
else
resolve(data);
});
})
.then(data => {
data = data.toString();
var options = {
delimiter : ',',
quote : '"'
};
const json_data = csvjson.toObject(data, options);
write_file(json_path, json_data);
return data;
});
}
Upvotes: 2