mreinstein / alexa-verifier

✓ Verify HTTP requests sent to an Alexa skill are sent from Amazon
MIT License
76 stars 23 forks source link

add promise support #35

Closed mreinstein closed 6 years ago

mreinstein commented 6 years ago

if invoked without a callback function (e.g., verifier(cert_url, signature, requestBody) return a promise.

tejashah88 commented 6 years ago

I'd suggest that a separate module should be made, so that we can keep the bloat out and it'll be a lot easier for maintaining the module itself, since the "promised" version would simple have to simply wrap this module in a resolve/reject statement. What do you think?

mreinstein commented 6 years ago

I think it's not necessary to make a seperate module. If we wanted to add new functionality or really change it's behavior I'd say a new module might be appropriate.

In this case, we're talking about moving away from callbacks torwards promises. This is something happening in the core ecosystem, now that async/await and native promises landed in an LTS release. Eventually old versions of node that don't support native promises will go away (April 2019 according to https://github.com/nodejs/Release#release-schedule) and the callback usage could be removed at that point.

I don't think it would add that much bloat to things. essentially some simple logic:


function verifier(cert_url, signature, requestBody, cb) {
   // normal module logic here
}

module.exports = function(cert_url, signature, requestBody, cb) {
  if(cb)
    return verifier(cert_url, signature, requestBody, cb)

  return new Promise(function( resolve, reject) {
     verifier(cert_url, signature, requestBody, function(er) {
        if(er)
          return reject(er)
        resolve()
     })
  })
}