Promise Chain Breaks w/ .all()

Question

I am fairly new promises and thought I had a handle on things but clearly I do not.

The following code is meant to grab X number of table names (getTableName()), pass those to getData() and loop over each table to get the data. I then call Promise.all() to resolve those and attempt to pass the data to the next link in the chain (createFile()) but the Promise.all() resolves after it moves on to createFile().

Is this an example of the "antipattern" coming back to bite me? If so, can you anyone suggest how I should restructure such that I can accomplish what I am setting out to do?

Thanks!

exports.getRawDataForExport = function(data) {
  return new Promise(function(resolve, reject) {

    var getTableName = function() {
      return knex('monitored_parameter')
        .where('device_id', data.device_id)
        .whereIn('internal_name', data.param)
        .select()
        .then(function(rows) {
          if(rows) {
            var runningResult = {};
            for(var i = 0; i < rows.length; i++) {
              var tbl = {"table" : 'monitored_parameter_data_' + rows[i].id, "param" : rows[i].display_name};
              runningResult.table = [];
              runningResult.table.push(tbl);
            }

            return runningResult;

          } else {
            // if no MP row(s) found we cannot proceed
            throw new Error("No history found for that parameter.");
          }
        });
    }

    var getData = function(runningResult) {
      var promises = [];

      if(data.date_start && data.date_end) {
        // grab within range

        for(var i = 0; i < runningResult.table.length; i++) {
          promises.push(
            knexHistory(runningResult.table[i].table)
              .select('created', 'data_value as value', 'unit')
              .whereBetween('created', [data.date_start, data.date_end])
            );
        }

        // *** the chain moves on to createFile() before this data is passed along
        Promise.all(promises).then(function(data) {
          console.dir('inside promises.all');
          console.dir(data);
          runningResult.data = data;
          return runningResult;
        });

      } else {
        // grab all
        for(var i = 0; i < runningResult.table.length; i++) {
          promises.push(
            knexHistory(runningResult.table[i].table)
              .select('created', 'data_value as value', 'unit')
            );
        }

        // *** the chain moves on to createFile() before this data is passed along
        Promise.all(promises).then(function(data) {
          console.dir('inside promises.all');
          console.dir(data);
          runningResult.data = data;
          return runningResult;
        });
      }
    }

    var createFile = function(runningResult) {

      var files = [],
          zipFileName = filePathExport + 'Data_Export.zip';

      for(var i = 0; i < runningResult.table.length; i++) {
        var fields = ['created', 'value', 'unit'],
            csvFileName = runningResult.param + '_export.csv',
            csvFilePath = filePathExport + runningResult.param + '_export.csv';

        var csv = json2csv({data : runningResult.data[i], fields : fields, doubleQuotes : ''});

        fs.writeFileSync(csvFilePath, csv);

        files.push(csvFilePath);
      }

      var zip = new admzip();

      for(var j = 0; j < files.length; j++) {
        var input = fs.readFileSync(files[i]);
        zip.addFile(csvFileName, input, '', 0644);
      }

      zip.writeZip(zipFileName);

      return zipFileName;

    }

    getTableName()
      .then(getData)
      .then(createFile)
      .then(function(zipFile) {
        resolve(zipFile);
      })
      .catch(function(err) {
        resolve(err);
      });

  });
}

score 4 · Accepted Answer · edited May 23 '17 at 11:54

4

You need to return the result of Promise.all(...).then(...) (in both instances), so that getData returns a promise.

And, yes you are using the promise constructor antipattern, although it is not the cause of the problem.

Remove the return new Promise(function(resolve, reject) { wrapper, and just return like this:

return getTableName().then(getData)

... leaving out the calls to resolve

edited May 23 '17 at 11:54

Community

1
1

answered Apr 12 '17 at 20:47

trincot

317,000
35
244
286

Thank you for the quick reply, this approach is working great. – niczak Apr 12 '17 at 21:11

score 1 · Answer 2 · edited Apr 12 '17 at 21:14

You probably just want to return the chained Promise.all

var getData = function(runningResult) {
      var promises = [];

      if(data.date_start && data.date_end) {
        // grab within range

        for(var i = 0; i < runningResult.table.length; i++) {
          promises.push(
            knexHistory(runningResult.table[i].table)
              .select('created', 'data_value as value', 'unit')
              .whereBetween('created', [data.date_start, data.date_end])
            );
        }

        // *** the chain moves on to createFile() before this data is passed along
        return Promise.all(promises).then(function(data) {
          console.dir('inside promises.all');
          console.dir(data);
          runningResult.data = data;
          return runningResult;
        });

      } else {
        // grab all
        for(var i = 0; i < runningResult.table.length; i++) {
          promises.push(
            knexHistory(runningResult.table[i].table)
              .select('created', 'data_value as value', 'unit')
            );
        }

        // *** the chain moves on to createFile() before this data is passed along
        return Promise.all(promises).then(function(data) {
          console.dir('inside promises.all');
          console.dir(data);
          runningResult.data = data;
          return runningResult;
        });
      }
    }

Promise Chain Breaks w/ .all()

2 Answers2