Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default callbackWaitsForEmptyEventLoop to false? #190

Closed
FinnWoelm opened this issue Apr 3, 2021 · 8 comments · Fixed by #360
Closed

Default callbackWaitsForEmptyEventLoop to false? #190

FinnWoelm opened this issue Apr 3, 2021 · 8 comments · Fixed by #360

Comments

@FinnWoelm
Copy link

Hi y'all! Amazing plugin. Love how it now works completely out of the box with zero setup 🤩 ❤️

I just ran into the Task timed out after 10.01 seconds issue:

{"errorMessage":"2021-04-03T07:42:16.336Z 3be439c6-d16e-42d2-859f-a66bffe9135e Task timed out after 10.01 seconds"}

I know this is caused because, by default, AWS lambda waits for empty event loop. And this needs to be explicitly turned off for some libraries (firebase, faunadb). See: netlify/next-on-netlify#66 (comment)

Is there any harm in turning off this waiting-for-empty-event-loop-behavior by default for the Netlify Functions created by this plugin?

It would solve issues like this: https://stackoverflow.com/questions/65243013/next-on-netlify-function-times-out-10s-when-deployed-renders-in-0-3s-locally

The fact that it works out of the box with Vercel makes me think that there might be no harm in turning this off as the default. Perhaps one of you actually knows what the implications would be?

The implementation would be a very simple one-liner in https://github.com/netlify/netlify-plugin-nextjs/blob/main/src/lib/templates/netlifyFunction.js:

context.callbackWaitsForEmptyEventLoop = false;

Look forward to hearing your thoughts! :)

@lindsaylevine
Copy link

@FinnWoelm oh hey finn

@lindsaylevine
Copy link

after some internal discussion, seems like we may add an option to turn callbackWaitsForEmptyEventLoop "off" (aka set it to false) as a plugin input.

@FinnWoelm
Copy link
Author

FinnWoelm commented Apr 8, 2021

Ok, that's a pretty good solution. I'm still trying to better understand what this thing actually means...

I just saw that there's an async version of the function handler, that doesn't use callback at all: https://docs.aws.amazon.com/lambda/latest/dg/nodejs-handler.html#nodejs-prog-model-handler-callback (see Async handlers).

I wonder if this whole question would be solved by just switching to async handler. AWS docs say this:

For non-async handlers, function execution continues until the event loop is empty or the function times out. The response isn't sent to the invoker until all event loop tasks are finished. If the function times out, an error is returned instead. You can configure the runtime to send the response immediately by setting context.callbackWaitsForEmptyEventLoop to false.

(source: same link as above)

If I get a chance, I'll do some experimentation...

@FinnWoelm
Copy link
Author

Netlify also advertises the non-callback async handler:

image

https://www.netlify.com/products/functions/

@speakk
Copy link

speakk commented May 25, 2021

What's a relatively easy way to test this before it gets implemented? This is a pretty huge blocker for us at the moment.

@lindsaylevine
Copy link

@speakk whoa! wild to see you comment this because it's next up on my to-do list. i will get to it this week, but for now, you could use patch-package like described in this comment: #191 (comment)

hang tight!

@speakk
Copy link

speakk commented May 25, 2021

Awesome, thank you muchly! :)

@lindsaylevine lindsaylevine self-assigned this May 26, 2021
@lindsaylevine
Copy link

@speakk do you have a repo to test a fix/branch against?

serhalp pushed a commit that referenced this issue Apr 5, 2024
* fix: support wasm chunks

* Import from vendor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants