8

I am writing a simple HTTP 'ping' function that is being periodically executed using AWS Lambda. It uses four asynchronous functions: http.get, S3.getObject, S3.putObject, and nodemailer.sendMail. Each seems to have a slightly different callback model.

After reading about promises, I spent way too much time trying to convert the following code to use Q promises and failed miserably.

For my own education and hopefully that of others, I was hoping someone could help me convert this to using promises (doesn't have to be Q):

'use strict';

var http = require('http');
var nodemailer = require('nodemailer');
var AWS = require('aws-sdk');
var s3 = new AWS.S3( { params: { Bucket: 'my-bucket' } } );

exports.handler = (event, context, callback) => {
  var lastStatus;

  var options = {
    host: event.server.host,
    port: event.server.port ? event.server.port : 80,
    path: event.server.path ? event.server.path : '',
    method: event.server.method ? event.server.method : 'HEAD',
    timeout: 5000
  };
  var transporter = nodemailer.createTransport({
    host: event.mail.host,
    port: event.mail.port ? event.mail.port : 587,
    auth: {
      user: event.mail.user,
      pass: event.mail.pass
    }
  });

  var d = new Date();
  var UTCstring = d.toUTCString();

  // email templates 
  var downMail = {
    from: event.mail.from,
    to: event.mail.to,
    subject: 'Lambda DOWN alert: SITE (' + event.server.host + ') is DOWN',
    text: 'LambdaAlert DOWN:\r\nSITE (' + event.server.host + ') is DOWN as at ' + UTCstring + '.'
  };
  var upMail = {
    from: event.mail.from,
    to: event.mail.to,
    subject: 'Lambda UP alert: SITE (' + event.server.host + ') is UP',
    text: 'LambdaAlert UP:\r\nSITE (' + event.server.host + ') is UP as at ' + UTCstring + '.'
  };

  // Run async chain to ensure that S3 calls execute in proper order
  s3.getObject( { Key: 'lastPingStatus' }, (err, data) => {
    // get last status from S3
    if (err) { lastStatus = "UP"; } else {
      lastStatus = data.Body.toString();
      console.log("Last observed status: " + lastStatus);
    }
    http_request(options, lastStatus);
  });

  function http_request(requestOptions, lastStatus) {
    var req = http.request(requestOptions, function(res) {
      if (res.statusCode == 200) {
        if (lastStatus == "DOWN") {
          console.log('Email up notice sending...');
          transporter.sendMail(upMail, function(error, info) {
            if (error) {
              console.log("ERROR: " + error);
              callback(null, "ERROR: " + error);
            } else {
              console.log('No further details available.');
              callback(null, 'Up message sent');
            }
          });
        }
        s3.putObject({ Key: 'lastPingStatus', Body: 'UP', ContentType: 'text/plain' }, (error, data) => { console.log("Saved last state as UP"); });
        callback(null, 'Website is OK.');
      }
    });
    req.on('error', function(e) {
      if (lastStatus == "UP") {
        console.log('Email down notice sending...');
        transporter.sendMail(downMail, function(error, info) {
          if (error) {
            console.log("ERROR: " + error);
            callback(null, "ERROR: " + error);
          } else {
            console.log('No further details available.');
            callback(null, 'Down message sent');
          }
        });
        s3.putObject({ Key: 'lastPingStatus', Body: 'DOWN', ContentType: 'text/plain' }, (error, data) => { console.log("Saved last state as DOWN"); });
        callback(null, 'Website is DOWN.');
      }
    });
    req.end();
  }
};

EDIT: First attempt at writing using promises:

'use strict';

var http = require('http');
var nodemailer = require('nodemailer');
var AWS = require('aws-sdk');
var s3 = new AWS.S3( { params: { Bucket: 'lambda-key-storage' } } );

exports.handler = (event, context, callback) => {
  var lastStatus;

  var options = {
    host: event.server.host,
    port: event.server.port ? event.server.port : 80,
    path: event.server.path ? event.server.path : '',
    method: event.server.method ? event.server.method : 'HEAD',
    timeout: 5000
  };
  var transporter = nodemailer.createTransport({
    host: event.mail.host,
    port: event.mail.port ? event.mail.port : 587,
    auth: {
      user: event.mail.user,
      pass: event.mail.pass
    }
  });

  var d = new Date();
  var UTCstring = d.toUTCString();

  // email templates 
  var downMail = {
    from: event.mail.from,
    to: event.mail.to,
    subject: 'Lambda DOWN alert: SITE (' + event.server.host + ') is DOWN',
    text: 'LambdaAlert DOWN:\r\nSITE (' + event.server.host + ') is DOWN as at ' + UTCstring + '.'
  };
  var upMail = {
    from: event.mail.from,
    to: event.mail.to,
    subject: 'Lambda UP alert: SITE (' + event.server.host + ') is UP',
    text: 'LambdaAlert UP:\r\nSITE (' + event.server.host + ') is UP as at ' + UTCstring + '.'
  };

  var myProm = new Promise(function(resolve, reject) {
    console.log("called 1");
    s3.getObject( { Key: 'lastPingStatus' }, (err, data) => {
      // get last status from S3
      if (err) { 
        resolve("UP"); 
      } else {
        resolve(data.Body.toString());
      }
    });
  })
  .then(function(lastStatus) {
    console.log("called 2");
    console.log("Last observed status: " + lastStatus);
    var req = http.request(options, function(res) {
      resolve(res.statusCode);
    });
    req.on('error', function(e) {
      reject(e);
    });
    req.end();
    return "??";
  })
  .then(function(statusCode) {
    console.log("called 3");
    if (statusCode == 200) {
      if (lastStatus == "DOWN") {
        console.log('Email up notice sending...');
        resolve("upTrigger");
      } else {
        resolve("upNoTrigger");
      }
      s3.putObject({ Key: 'lastPingStatus', Body: 'UP', ContentType: 'text/plain' }, (err, data) => { console.log("Saved last state as UP"); });
      callback(null, 'Website is OK.');
    }
  })
  .catch(function(err){
    console.log("called 3 - error");
    // Send mail notifying of error
    if (lastStatus == "UP") {
      console.log('Email down notice sending...');
      resolve("downTrigger");
      s3.putObject({ Key: 'lastPingStatus', Body: 'DOWN', ContentType: 'text/plain' }, (error, data) => { console.log("Saved last state as DOWN"); });
      callback(null, 'Website is DOWN.');
      return("downTrigger");
    } else {
      return "downNoTrigger";
    }
  })
  .then(function(trigger) {
    console.log("called 4");
    if (trigger == "upTrigger") {
      transporter.sendMail(upMail, (error, info) => {
        if (error) {
          console.log("ERROR: " + error);
          callback(null, "ERROR: " + error);
        } else {
          console.log('Up message sent.');
          callback(null, 'Up message sent');
        }
      });
    } else if (trigger == "downTrigger") {
      transporter.sendMail(downMail, (error, info) => {
        if (error) {
          console.log("ERROR: " + error);
          callback(null, "ERROR: " + error);
        } else {
          console.log('Down message sent.');
          callback(null, 'Down message sent');
        }
      });
    }
    console.log("Outcome of ping was: ", trigger);
  });
};

This doesn't quite work. The result logs are:

called 1
called 2
Last observed status: UP
called 3
called 4
Outcome of ping was:  undefined
ReferenceError: resolve is not defined
Roamer-1888
  • 19,138
  • 5
  • 33
  • 44
GuruJ
  • 348
  • 2
  • 4
  • 9
  • Big hint: 2 stages, (1) [promisify](http://stackoverflow.com/documentation/javascript/231/promises/2109/promisifying-functions-with-callbacks#t=201611071506458662953) at the lowest level - s3.getObject(), s3.putObject(), transporter.sendMail() and http.request(), (2) construct the application flow using the promisified versions of those four methods. – Roamer-1888 Nov 07 '16 at 15:05

5 Answers5

11

Converting your typical async function to a promise is pretty straight forward. I'd rather try and demonstrate how to convert it than write the code as you don't learn anything from that.

Usually with node you'll have something that looks similar to this:

  doSomethingAsync(callback);

    function doSomethingAsync(callback){
        var err, result;
        // Do some work
        ... 

        callback(err, result);
    }
    function callback(err, result){
        if(err){
            // Handle error
        } else{
            // Success so do something with result
        }
    }

A promise wrapping an async function generally looks something like this:

var myProm = new Promise(function(resolve, reject){
     doSomethingAsync(function(err, result){       
        if(err){
            reject(err);
        } else{
            resolve(result)
        } 
     });
})
.then(function(result){
  // Success so do something with result
  console.log("Success:", result)
})
.catch(function(err){
 // Handle error
  console.log("Error: ", err);
})
.then(function(result){
   // Where's my result? - result == undefined as we didn't return anything up the chain
  console.log("I always execute but result is gone", result)
})

To pass the result down the chain to our "always then" method we need to return a promise or a value:

var myProm = new Promise(function(resolve, reject){
         doSomethingAsync(function(err, result){       
            if(err){
                reject(err);
            } else{
                resolve(result)
            } 
         });
    })
    .then(function(result){
      // Success so do something with result
      console.log("Success:", result)
      return result;
    })
    .catch(function(err){
     // Handle error
      console.log("Error: ", err);
      return err;
    })
    .then(function(result){
      // The err/result now gets passed down the chain :)
      console.log("Oh there it is", result)
    })

I think that using the above patterns should cater to most of the async methods and events in your code example if any particular ones are giving you trouble drop a comment in and I'll try to cover those specific examples.

Here's an attempt at converting it over to promises - I'm pretty tired so apologies about any mess or mistakes - also there's still plenty of cleanup that could be done.

Essentially what I've done is try to break down the code into tasks and wrap each of those tasks in a promise. That way we can resolve/reject and chain them as needed.

'use strict';

var http = require('http');
var nodemailer = require('nodemailer');
var AWS = require('aws-sdk');
var s3 = new AWS.S3( { params: { Bucket: 'my-bucket' } } );

exports.handler = function (event, context, callback) {
    var lastStatus;

    var options = {
        host: event.server.host,
        port: event.server.port ? event.server.port : 80,
        path: event.server.path ? event.server.path : '',
        method: event.server.method ? event.server.method : 'HEAD',
        timeout: 5000
    };
    var transporter = nodemailer.createTransport({
        host: event.mail.host,
        port: event.mail.port ? event.mail.port : 587,
        auth: {
            user: event.mail.user,
            pass: event.mail.pass
        }
    });

    var d = new Date();
    var UTCstring = d.toUTCString();

    // email templates
    var downMail = {
        from: event.mail.from,
        to: event.mail.to,
        subject: 'Lambda DOWN alert: SITE (' + event.server.host + ') is DOWN',
        text: 'LambdaAlert DOWN:\r\nSITE (' + event.server.host + ') is DOWN as at ' + UTCstring + '.'
    };
    var upMail = {
        from: event.mail.from,
        to: event.mail.to,
        subject: 'Lambda UP alert: SITE (' + event.server.host + ') is UP',
        text: 'LambdaAlert UP:\r\nSITE (' + event.server.host + ') is UP as at ' + UTCstring + '.'
    };

    // Run async chain to ensure that S3 calls execute in proper order

    function getLastPingStatus(){
        return new Promise(function(resolve, reject){
            s3.getObject( { Key: 'lastPingStatus' }, function(err, data) {
                // get last status from S3
                if (err) {
                    lastStatus = "UP";
                    reject(lastStatus)
                } else {
                    lastStatus = data.Body.toString();
                    resolve(lastStatus);
                    console.log("Last observed status: " + lastStatus);
                }
            });
        })
    }
    getLastPingStatus()
        .then(httpRequest)
        .catch(httpRequest); // Otherwise a reject will throw an error

    function sendMail(mail, status){ // status = "up" or "down" -
        return new Promise(function(resolve, reject){
            transporter.sendMail(mail, function(error, info) {
                if (error) {
                    console.log("ERROR: " + error);
                    reject(null, "ERROR: " + error);
                } else {
                    console.log('No further details available.');
                    resolve(null, status + ' message sent');
                }
            });
        });
    }

    function saveStatus(up) {
        return new Promise(function (resolve, reject) {
            var saveOptions,
                message;
            // I didn't bother refactoring these as promises at they do the same thing regardless of outcome
            if(up){
                saveOptions = [{ Key: 'lastPingStatus', Body: 'UP', ContentType: 'text/plain' }, function(error, data) { console.log("Saved last state as UP"); }];
                message = 'Website is OK.';
            } else{
                saveOptions = [{ Key: 'lastPingStatus', Body: 'DOWN', ContentType: 'text/plain' }, function(error, data)  { console.log("Saved last state as DOWN"); }];
                message = 'Website is DOWN.';
            }
            s3.putObject.apply(this, saveOptions);
            callback(null, message);
        });
    }

    function httpRequest(lastStatus) {
        var requestOptions = options;
        return new Promise (function (resolve, reject){
            var req = http.request(requestOptions, function(res) {
                if (res.statusCode == 200) {
                    if (lastStatus == "DOWN") {
                        console.log('Email up notice sending...');
                        sendMail(upMail, "Up")
                            .then(resolve, reject) 
                            .then(saveStatus(true))
                            .then(callback)
                    }
                }
            });
            req.on('error', function(e) {
                if (lastStatus == "UP") {
                    console.log('Email down notice sending...');
                    sendmail(downMail, "Down")
                        .then(resolve, reject)
                        .then(saveStatus(false))
                        .then(callback)
                }
            });
            req.end();

        });

    }
};
Brian
  • 2,822
  • 1
  • 16
  • 19
  • I've put my attempt at Promises code in the original Q. It mostly works but I think I am getting all confused about when I should resolve, when I should return, and when I should callback. – GuruJ Nov 06 '16 at 11:31
  • 1
    I've added my shot at converting it to promises - I'm pretty tired so sorry about mistakes. Generally it's best to break your code down into small tasks which return promises. You resolve a task once it's successful or reject it if it fails, you pass the "return" value as an argument to resolve/reject which is then passed to the next then/catch in the chain and assigned as the value of that particular promise. Once a promise is resolved or rejected it's value is immutable. In your attempt you're trying to resolve the promise within a .then, but the resolve and reject arguments are out of scope – Brian Nov 06 '16 at 12:57
7

The AWS-SDK supports native promises, for all services. Some need additional parameters to return properly, like Lambda.invoke().

You would essentially do

s3.putObject({ Key: 'key', Bucket: 'bucket' }).promise()
    .then(data => {
        // this is the same as the data callback parameter
    })
    .catch(error => {
        // handle your error
    })

Or, you could use async/await:

const file = await s3.getObject(params).promise()
// do things with the result

For quick access to the actual file (and not metadata):

const file = JSON.parse(await s3.getObject(params).promise().then(res => res.Body));

https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/using-promises.html

slaughtr
  • 536
  • 5
  • 17
  • Although Brian's answer further down is very nice and explanatory, this lead me to the best way of implementing chaining – jsaddwater Sep 06 '18 at 21:06
5

In order to "promisify" callback function, imho, the easiest and cleaner way is to use bluebird. You just don't want to write glue code in order to simplify your code, it's counter productive (and it's error prone).

From the doc :

var Promise = require("bluebird");
var readFile = Promise.promisify(require("fs").readFile);

readFile("myfile.js", "utf8").then(function(contents) {
    return eval(contents);
}).then(function(result) {
    console.log("The result of evaluating myfile.js", result);
}).catch(SyntaxError, function(e) {
    console.log("File had syntax error", e);
//Catch any other error
}).catch(function(e) {
    console.log("Error reading file", e);
});
Boris Charpentier
  • 3,515
  • 25
  • 28
0

After reading slaughtr's answer I decided to do it like this, for doing some data saving when pressing the AWS IoT button:

var AWS = require("aws-sdk");
var iot = new AWS.Iot();

exports.handler = (event, context, callback) => {
  iot.listThings({
    attributeName: 'dsn',
    attributeValue: event.serialNumber,
    maxResults: 1
  })
  .promise()
  .then(response => {
    return iot.listThingGroupsForThing({thingName: response.things[0].thingName}).promise();
  })
  .then(groupsList => insertRecordIntoDDB(date, serialNumber, groupsList.thingGroups[0].groupName))
  .catch(err => console.log(err))
};

and shortly after I decided to compress it even further with async/await

exports.handler = async (event, context, callback) => {
var eventText = JSON.stringify(event, null, 2);

var thingsList = await iot.listThings({ 
  attributeName: 'dsn', 
  attributeValue: event.serialNumber, 
  maxResults: 1
}).promise()

var groupsList = await iot.listThingGroupsForThing({
  'thingName': thingsList.things[0].thingName
}).promise();
insertRecordIntoDDB(date, serialNumber, groupsList.thingGroups[0].groupName)

};

I'm still fairly new at this async programming stuff so I'm not sure what I should like the most. Promise chaining can get a bit spaghetti-like, while async await only helps mask all that into something that's easier to comprehend

jsaddwater
  • 1,781
  • 2
  • 18
  • 28
0

Using node http promisified to call external api in aws lambda.

exports.handler = async (event) => {
    return httprequest().then((data) => {
        const response = {
            statusCode: 200,
            body: JSON.stringify(data),
        };
    return response;
    });
};
function httprequest() {
     return new Promise((resolve, reject) => {
        const options = {
            host: 'jsonplaceholder.typicode.com',
            path: '/todos',
            port: 443,
            method: 'GET'
        };
        const req = http.request(options, (res) => {
          if (res.statusCode < 200 || res.statusCode >= 300) {
                return reject(new Error('statusCode=' + res.statusCode));
            }
            var body = [];
            res.on('data', function(chunk) {
                body.push(chunk);
            });
            res.on('end', function() {
                try {
                    body = JSON.parse(Buffer.concat(body).toString());
                } catch(e) {
                    reject(e);
                }
                resolve(body);
            });
        });
        req.on('error', (e) => {
          reject(e.message);
        });
        // send the request
       req.end();
    });
}
smsivaprakaash
  • 1,564
  • 15
  • 11