VoxaAI / voxa

Voxa is a framework that uses state machines to create elegant cross platform conversational experiences.
http://voxa.readthedocs.io/
MIT License
73 stars 26 forks source link

Lambda's Context Object is not being passed on #83

Closed nkahoang closed 6 years ago

nkahoang commented 6 years ago

Hi,

I'm having an issue with Voxa library on AlexaSkill.js not passing on the context object that comes as a parameter for all Lambda handlers:

https://github.com/mediarain/voxa/blob/ec0d98d61f161970dcf52c6073aa42815b5ea2b3/lib/AlexaSkill.js#L93

We need to be able to change a property in the context item, specifically: context.callbackWaitsForEmptyEventLoop = false so that Lambda function will returns immediately as callback is called. Without this property, if we still have any nodejs event in the loop (e.g: shared resources connecting to redis/elasticCache, DB Connection), the function will just freeze and timed out (see: http://docs.aws.amazon.com/lambda/latest/dg/nodejs-prog-model-context.html#nodejs-prog-model-context-methods).

Is it possible that this context item being exposed to the AlexaSkill object?

armonge commented 6 years ago

Hi @nkahoang, you have access to that property from the alexaEvent object,

Example:

skill.onState('state', (event) => {
  event.lambdaContext 
})

However, in general it's better to just use promises and let voxa handle this. Voxa will wait for all promises to complete before finishing the lambda if you return them from your controllers


skill.onState('someState', (event) => {
   return saveToDBPromise()
    .then(() => {
      return { reply: 'SomeView'}
    })
})
nkahoang commented 6 years ago

Hi @armonge ,

Thank you for that. We will test the lambdaContext object via event.lambdaContext.

This is not really because we are not using Promise. In fact, we are using them on every onState / onIntent. We we did a trace and the final promise did resolve (trace in cloudwatch log) and found out that the code managed to reach the onBeforeReplySent event.

In my opinion by the fact that many packages such as cache-manager-redis and other (we had this issue using firebase with Lambda) caches the connection internally; and killing / opening new connection manually causes delays and additional resource usage. We've found an article regarding this behaviour for MongoDB as well (https://www.mongodb.com/blog/post/optimizing-aws-lambda-performance-with-mongodb-atlas-and-nodejs).

But as we haven't tested this extensively (we will do now that we know we can access the context directly from event object), we might not have known the side effects. It seems like all of Voxa's middlewares are being handled fine (we use Opearlo and Voicelabs) as we found their traces in the log after setting context.callbackWaitsForEmptyEventLoop=false. If you have know any problems with the usage please let us know.

nkahoang commented 6 years ago

Hi @armonge ,

We have just tested and found that the alexaEvent object (or event) has the lambdaContext property but it is null. We tested by both checking if alexaEvent.lambdaContext is not undefined and confirmed by json encode the alexaEvent object out to lambda cloudwatch log.

Can you please assist us? Below is the raw log


var a = {
session: 
{ attributes: {},
sessionId: 'SessionId.xxxxxxxxxxxxx',
application: { applicationId: 'amzn1.ask.skill.xxxxxxxxxxxxxxxxxx' },
user: 
{ userId: 'amzn1.ask.account.xxxxxxxxxxxxxxxxxxxxxxxxxxx',
accessToken: null },
new: true },
request: 
{ intent: { name: 'GetInfo', slots: [Object] },
requestId: 'EdwRequestId.xxxxxxxxxxxxxxxxxxx',
type: 'IntentRequest',
locale: 'en-AU',
timestamp: '2017-11-27T22:32:21Z' },
context: 
{ AudioPlayer: { playerActivity: 'IDLE' },
System: { application: [Object], user: [Object], device: [Object] } },
version: '1.0',
lambdaContext: undefined,
intent: 
Intent {
name: 'GetInfo',
slots: 
{ date: [Object],
movie: [Object],
specialTime: [Object],
concept: [Object],
genre: [Object],
time: [Object],
cinema: [Object] } },
model: 
Model { ... },
opearlo: 
VoxaOpearloEventRider {
name: 'CheckCinema',
alexaEvent: [Circular],
variables: {},
ignoreState: false } }```
armonge commented 6 years ago

@nkahoang really weird, how does your look lambda handler look like? are you using

exports.handler = skill.lambda();

?

I'm available to chat over slack on http://alexaslack.com/ , you can DM me, or you can also check https://gitter.im/voxa-rain/voxa

armonge commented 6 years ago

Oh wait, i think you actually found a bug! seems like we're not passing the context to the AlexaEvent constructor on https://github.com/mediarain/voxa/blob/master/lib/AlexaSkill.js#L100, that would cause the undefined. Sorry about that, i'll fix and push a new version tomorrow, also create some tests so this wont happen again

armonge commented 6 years ago

@nkahoang we pushed a new version that should fix the issue about lambdaContext being undefined. Let me know if that helps

nkahoang commented 6 years ago

Hi @armonge ,

We've just tested with the latest version and the lambdaContext is confirmed to be working via alexaEvent.lambdaContext! Thanks!