Richlewis
Richlewis

Reputation: 15374

Chaining methods using promises

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

Answers (1)

T.J. Crowder
T.J. Crowder

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:

  1. It doesn't wait for the file to be written; all it does is set up the pipe, without waiting for the process to complete
  2. It doesn't provide any way for the caller to know when the process is complete

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:

  1. It's missing { and } around the else logic

  2. 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)

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

Related Questions