alexa-js / alexa-app

A framework for Alexa (Amazon Echo) apps using Node.js
https://www.youtube.com/watch?v=pzM4jv7k7Rg
MIT License
1.03k stars 212 forks source link

Errors when using KeepAlive-style CloudWatch events #369

Open danepowell opened 6 years ago

danepowell commented 6 years ago

I have a CloudWatch event that essentially acts as a keepalive function by calling my Alexa Lambda backend with a "matched event" once every 10 minutes.

This generates a ton of errors like this in my logs:

"errorMessage": "Cannot read property 'new' of undefined",
    "errorType": "TypeError",
    "stackTrace": [
        "new alexa.request (/var/task/node_modules/alexa-app/index.js:149:30)",
        "/var/task/node_modules/alexa-app/index.js:227:21",
        "tryCatcher (/var/task/node_modules/bluebird/js/main/util.js:26:23)",
        "Promise._resolveFromResolver (/var/task/node_modules/bluebird/js/main/promise.js:483:31)",
        "new Promise (/var/task/node_modules/bluebird/js/main/promise.js:71:37)",
        "alexa.app.request (/var/task/node_modules/alexa-app/index.js:226:12)",
        "handler (/var/task/node_modules/alexa-app/index.js:357:10)"
    ]

I don't think this is hurting anything, but obviously I'd prefer to see something more constructive and less distracting than those errors.

Is there a better way to build a keepalive function with CloudWatch? Or should alexa-app be a little more resilient when it receives events that it's not expecting?

dblock commented 6 years ago

Looks like something expects a request with certain fields and should fail earlier. Please feel free to reproduce in a spec and fix.

danepowell commented 6 years ago

Ah, minor update: that particular error was with an earlier version of alexa-app (2.3.4). After upgrading to the latest version (4.2.2) I get a different error: missing request type

The root cause is the same though. The problem is that the KeepAlive Cloudwatch event doesn't really look like an Alexa request. It looks like this:

{
  "version": "0",
  "id": "89d1a02d-5ec7-412e-82f5-13505f849b41",
  "detail-type": "Scheduled Event",
  "source": "aws.events",
  "account": "123456789012",
  "time": "2016-12-30T18:44:49Z",
  "region": "us-east-1",
  "resources": [
    "arn:aws:events:us-east-1:123456789012:rule/SampleRule"
  ],
  "detail": {}
}

I'm not sure what the best solution is here. It kind of makes sense for alexa-app to throw an error if it gets a clearly malformed request like that. But at the same time, it's a well-known practice to use Cloudwatch keepalive events, and I think there should be some way to support that without throwing errors.

matt-kruse commented 6 years ago

I have a background task which triggers my Alexa Lambda function from a CloudWatch Event also. I have changed the lambda handler in my code to this, which handles the different request type. I defined my own app.scheduled() function that handles the CW event.

// connect to lambda
exports.handler = function(event, context, callback) {
  if (event && "aws.events"===event.source) {
    // Scheduled Event!
    app.scheduled(event).then((response)=>{
      callback(null,response);
    }).catch((e)=>{
      callback(e);
    });
  }
  else {
    // Alexa Request
    log("Alexa Request");
    app.handler(event, context, callback);
  }
};

This is another common use case that I think would be great to have handled within alexa-app itself.

dblock commented 6 years ago

@matt-kruse 👍

Lets leave this open as a feature request. I think a generic way to handle non-alexa events could be good.

kobim commented 5 years ago

maybe calling to .pre() and resolving the request will help?

app.pre(function(request, response) {
  if (request.data && request.data.source === 'aws.events') {
    console.log('ping!');
    response.resolved = true; // will skip the next handlers.
    response.response = {}; // will nullify the response. You can omit this if you don't care for the output.
  }
  return true;
});