feat(core) - PDE-5490 calling lambda callback doesnt wait for event loop #904
Conversation
| // In some cases, perhaps due to fetching large data blobs, the Lambda function | ||
| // will hang waiting for the event loop to drain. Setting this to false will | ||
| // prevent that and return when the callback is called. | ||
| context.callbackWaitsForEmptyEventLoop = false; |
There was a problem hiding this comment.
🤔 I'm a bit nervous about enabling this by default. It seems like a good default for Lambda to wait until the event loop is empty before returning. Otherwise, I imagine we can introduce unpredictability that's quite hard to debug since some async tasks will complete sometimes and not other times.
We're actually explicitly setting this to true a bit above, here.
What about enabling this per-app to start with? At this point in the code we have access to the event payload from Lambda in which we could provide app-specific flags.
standielpls
force-pushed
the
pde-5490/exit-on-callback
branch
from
c03e607
to
82a1ac1
Compare
| context.callbackWaitsForEmptyEventLoop = true; | ||
| // In some cases, the code hangs and never exits because the event loop is not | ||
| // empty, so we can override the default behavior and exit after the app is done. | ||
| context.callbackWaitsForEmptyEventLoop = !event.skipWaitForAsync || true; |
There was a problem hiding this comment.
@standielpls I think this is always true, no? false || true === true, undefined || true === true.
| context.callbackWaitsForEmptyEventLoop = !event.skipWaitForAsync || true; | |
| context.callbackWaitsForEmptyEventLoop = true; | |
| if (event.skipWaitForAsync === true) { | |
| context.callbackWaitsForEmptyEventLoop = false; | |
| } |
There was a problem hiding this comment.
Alternatively: event.skipWaitForAsync !== true;
There was a problem hiding this comment.
Whoops, yes you're right!
In debugging some apps, it seems like sometimes lambdas may be hanging indefinitely until the timeout expires.
This causes the action to fail, even if all the app code has completed.
Debugging further it looks like it's possible that when processing larger datasets, some open connections will be kept open and block the event loop from clearing.
This heuristic is something we can add from Lambda
https://docs.aws.amazon.com/lambda/latest/dg/nodejs-context.html
the gist is, if we set
context.callbackWaitsForEmptyEventLoop = false;then once the callback is called, we exit, despite having other connectionshttps://zapierorg.atlassian.net/browse/PDE-5490