Trying to understand Promise()

Question

So I have forked a Javascript project and trying to extend it a bit and at the same time learn from it. My Javascript skills are really newbish but I thought it would be fun. I am however really struggling with all the promises to the point that I have some many promises and thenables that I really don't understand how it is situated anymore. I fail to get the end result and are unable to see why.

So I have this to start from (I disabled one function to keep it simple):

export const calculateMovingAverage = (event, context, callback) =>
  Promise.all([
    // ma.calculateMovingAverage('Kraken', 'AskPrice'),
    ma.calculateMovingAverage('Coinbase', 'SellPrice'),
  ])
    .then((tx) => {
      console.log('tx', tx);
    }).catch((err) => {
      console.error('err', err);
      callback({ statusCode: 500, body: { message: 'Something went wrong' } });
    });

Which thus calls ma.calculateMovingAverage():

calculateMovingAverage(exchange, metric) {
    const self = this;
    const args = {
      minutes: 10,
      period: 60,
      currency: `${self.cryptoCurrency}`,
      metricname: `${metric}`,
      localCurrency: `${self.localCurrency}`,
      namespace: `${exchange}`,
    };
    var promisedland = new Promise((resolve, reject) => {
      self.cloudwatch.getMetricStatistics(args, (err, data) => {
        if (err) { return reject(err); }

        if (!Array.isArray(data.Datapoints) || !data.Datapoints.length) { return reject("No Datapoints received from CloudWatch!")}

        data.Datapoints.forEach(function(item) {
          self.ma.push(item.Timestamp, item.Average);
        });

        resolve(ma.movingAverage());
      })
    })
    promisedland.then((results) => {
      return new Promise((resolve, reject) => {
        const body = {
          value: results,
          metricName: `${metric} @ 180 MovingAverage`,
          namespace: `${exchange}`
        };
        self.cloudwatch.putAverageMetricData(body, function(err, result) {
          if (err) { return reject(err); }
          resolve(result);
        });
      }
    )
    }).catch(function(err) {
      return reject(err);
    });
  }

Now as you can see inside calculateMovingAverage() I try to call two AWS methods. getMetricStatistics and putAverageMetricData.

The first one, the getMetricStatistics functions works beautifully as I get back the Datapoints nicely from AWS.

The function itself:

  getMetricStatistics(args) {
    return this.cloudwatch.getMetricStatistics({
      EndTime: moment().subtract(180, 'minutes').utc().format(),
      Dimensions: [
        {
          Name: 'CryptoCurrency',
          Value: args.currency
        },
        {
          Name: 'LocalCurrency',
          Value: args.localCurrency
        },
        {
          Name: 'Stage',
          Value: process.env.STAGE || 'dev'
        }
      ],
      MetricName: args.metricname,
      Period: Number(args.period),
      StartTime: moment().subtract(190, 'minutes').utc().format(),
      Statistics: ['Average'],
      Unit: 'Count',
      Namespace: args.namespace || 'Coinboss',
    }).promise();
  }

Next I want to pass on the response through the MovingAverage module and would like to put the outcome of MA into CloudWatch Metrics via the putAverageMetricData function:

putAverageMetricData(args) {
    return this.cloudwatch.putMetricData({
      MetricData: [
        {
          MetricName: args.metricName,
          Timestamp: moment().utc().format(),
          Unit: 'Count',
          Value: Number(args.value),
        },
      ],
      Namespace: args.namespace || 'Coinboss',
    }).promise()
    .then(function(result) {
      console.log('putAverageMetricData', result);
    });
  }

This is where I get lost. I looks like the data never arrives at the putAverageMetricData function. The console output only shows me the console.log('tx', tx); with the following:

2017-07-15T19:37:43.670Z 118ff4f0-6995-11e7-8ae7-dd68094efbd6 tx [ undefined ]

Ok, so I was not returning the calculateMovingAverage() then. It solved the undefined error. I still don't get logging from the putAverageMetricData() function which leaves me to think I am still missing something.

I hope someone can point me into the right direction

Answer 1

Instead of giving you a direct answer to how to fix this code I will give you the Promises 101 course and I think you will be able to understand what is going on at a higher level.

Callback Functions

JavaScript is (usually) "single threaded" meaning only one line of code can be executed at a time. Sometimes, we need to do things that take really long like making a request to our server. To handle this, javascript uses the callback function. A callback function is when you pass a function into another function as an argument. The most basic example is the settimout function.

setTimeout(function() {
  // settimout takes a callback function
}, 1000);

Now, the magic of the callback is it won't be executed until all the other "not callback" code or "synchronous" code

setTimeout(function(error, goodStuff) {
  console.log("WHAAT WHY AM I SECOND?") //this gets printed second
}, 1000);

console.log("YAY! I GET TO GO FIRST!!!!") // this is printed first

This is known as the javascript event loop. Here is a little picture I made to give you an idea of what it is:

The Javascript Event Loop

As you can see there is the call stack and the asynchronous queue. All of your code that is not a callback or asynchronous will go to the call stack. The synchronous trump functions are going to cut the line and be run first. The callback functions will go to the message queue or event loop and wait for all the synchronous functions to finish. Once the synchronous function's call stack completes, the callbacks will start executing. But eventually, javascript programmers ran into a little problem.

Callback Hell and Promises

Javascript started to get more and more complicated and eventually, the callbacks started taking callbacks. The more they became nested, the harder they became to read. Look at this eye sore:

fs.readdir(source, function (err, files) {
  if (err) {
    console.log('Error finding files: ' + err)
  } else {
    files.forEach(function (filename, fileIndex) {
      console.log(filename)
      gm(source + filename).size(function (err, values) {
        if (err) {
          console.log('Error identifying file size: ' + err)
        } else {
          console.log(filename + ' : ' + values)
          aspect = (values.width / values.height)
          widths.forEach(function (width, widthIndex) {
            height = Math.round(width / aspect)
            console.log('resizing ' + filename + 'to ' + height + 'x' + height)
            this.resize(width, height).write(dest + 'w' + width + '_' + filename, function(err) {
              if (err) console.log('Error writing file: ' + err)
            })
          }.bind(this))
        }
      })
    })
  }
})

So to make stuff like this easier to read the promise was born! It took code like the callback hell above and make it read more line by line.

myfirstPromise().then((resultFromFirstPromise) => {
    return mySecondPromise();
  }).then((resultFromSecondPromise) => {
    return myThirdPromise();
  }).then((resultFromThirdPromise) => {
    //do whatever with the result of the third promise
  }).catch((someError) => {
    //if any of the promises throw an error it will go here!
  })

So applying these concepts to your code this is what we want to do:

  getMetricStatistics(options)
    .then(metricStatistics => {
      // do what you need to do with those stats

    return putAverageMetricData(metricStatistics);
    })
    .then((putMetricDataResult => {
      //do what you need to do with the metric data result
    }))
    .catch(err => {
      //do error stuff
    })

Answer 2

Your getMetricStatistics and putAverageMetricData methods already return promises, so avoid the Promise constructor antipattern in calculateMovingAverage! And don't forget to return a promise in the end from there:

calculateMovingAverage(exchange, metric) {
  const args = {
    minutes: 10,
    period: 60,
    currency: this.cryptoCurrency,
    metricname: metric,
    localCurrency: this.localCurrency,
    namespace: exchange,
  };
  return this.cloudwatch.getMetricStatistics(args).then(data => {
    if (!Array.isArray(data.Datapoints) || !data.Datapoints.length)
      throw new "No Datapoints received from CloudWatch!";

    for (const item of data.Datapoints) {
      this.ma.push(item.Timestamp, item.Average);
    }
    return this.ma.movingAverage();
  }).then(results => {
    const body = {
      value: results,
      metricName: `${metric} @ 180 MovingAverage`,
      namespace: exchange
    };
    return this.cloudwatch.putAverageMetricData(body);
  });
}

Answer 3

In calculateMovingAverage you have to return the promise you're creating, otherwise Promise.all can't tell when the promises are resolved.

return promisedland.then((results) => {
  ...
}).catch(...);