-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
refactor(core): remove useless build forceTerminate exit #10410
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: +388 B (0%) Total Size: 11.6 MB ℹ️ View Unchanged
|
My guess is that #6958 is a problem in dev only, because we have this forceTerminate logic. Removing it would cause prod build to be stuck too, but I'm not too sure it has a major impact either.
How can memory leaks be an issue if we are right about to exit anyway? Build and deploy are not persistent processes. |
Not sure to understand how it could be related. This only affects
If devs (including us or plugin authors) are starting timers, never cleaning listeners, creating never-resolving async processes and that gets the build to be stuck, then it likely also affects dev. A program that exit can still have unexpected problems that should rather be fixed. |
I appreciate that this
#6958 is about the dev server process getting stuck if a loader is stuck, e.g. it enters an infinite loop. I don't remember if the loader getting stuck would also hang the build. |
Probably true but we'll only know how to uncover bugs in dev if we are aware first of those bugs we should uncover. I suspect bugs like this would be hidden in both dev/prod currently. |
For history: removing this led to a side effect on my own newsletter website, with This was because of a client module using posthog SDK on the server. Here's the fix: It turns out the This SSR bundle is the part that is most likely to trigger builds that do not terminate. For this reason, I'm likely to isolate the SSR in a dedicated process we can kill after the SSG phase. And possibly introduce a thread pool to execute the SSG even faster. I also suspect it's all related to some sites reporting build errors due to out of memory, in particular i18n sites. It might be related to similar leaks that prevent the server bundler from being garbage-collected as we iterate on subsequent locales. See also #4765 |
Motivation
Calling
exit(0)
this way at the end of the build is annoying for various reasons:It was introduced a long time ago in #2443 due to the build being stuck. I believe the underlying problem has been solved now.
And if user code does a weird thing that gets the build stuck, they should rather know about it.
We'll see if that cause troubles in practice but I'd like to take my bet a remove this code for now. We'll eventually find an alternative solution if this becomes a problem.
Test Plan
CI
Test links
https://deploy-preview-10410--docusaurus-2.netlify.app/