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

Connect to pm2 in no-daemon-mode #90

Merged
merged 5 commits into from
Dec 12, 2022
Merged

Connect to pm2 in no-daemon-mode #90

merged 5 commits into from
Dec 12, 2022

Conversation

claytercek
Copy link
Member

@claytercek claytercek commented Dec 12, 2022

  • connects to pm2 in no-daemon-mode
  • overrides console methods to use parent logger
  • freezes console methods so that pm2 can't override them

Those last two bullets are more of a temporary fix. A better, more permanent solution would be refactoring pm2 to allow for custom loggers instead of using console directly, perhaps by forking the pm2 repo or using patch-package.

Tested with a handful of prod apps, everything seems to be working as expected.

@claytercek claytercek added bug Something isn't working #monitor labels Dec 12, 2022
@benjaminbojko
Copy link
Collaborator

I did some more testing too and overall this seems great!

One issue I found is that we loose a few of the daemon benefits like single-instance apps:

  1. Running launchpad via code (launchpad.startup()) won't make it listen to shutdown signals by default. Because of that, apps aren't terminated on shutdown and subsequent launchpad runs will spawn new app instances. Killing node via taskkill /F /IM node.exe also doesn't kill child processes because they're not handled by a PM2 daemon.
  2. Running launchpad in a Windows 11 terminal host (instead of directly via cmd.exe and closing the host (or hitting CTRL+C if launching through a .bat file via explorer) also doesn't close the child processes (also see Window foregrounding and process management failing when Windows 11 default shell is set to Terminal #77 )

The first one was an easy fix: I added a shutdownOnExit option that now automatically listens for onExit within the LaunchpadCore constructor. That can be set to false to disable this behavior. The packages/launchpad/index.js, which is what npx launchpad actually runs, had that as an explicit step, but since we're not running PM2 as a daemon anymore I removed that.

Not quite sure what to do about (2) yet... We may have to look into our own type of daemon options that can register if the main process has terminated. So maybe npx launchpad launches a daemon, that in turn runs LaunchpadCore.startup() (instead of this being directly run via npx launchpad).

@benjaminbojko
Copy link
Collaborator

I would say let's merge this change in as it resolve other issues and streamlines the whole stack. I'll create a new ticket for (2).

@benjaminbojko benjaminbojko merged commit 94eae0c into main Dec 12, 2022
@benjaminbojko benjaminbojko deleted the fix/pm2-no-daemon branch December 12, 2022 22:00
@claytercek claytercek restored the fix/pm2-no-daemon branch December 13, 2022 15:29
@github-actions github-actions bot mentioned this pull request Dec 13, 2022
@claytercek claytercek deleted the fix/pm2-no-daemon branch December 13, 2022 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working #monitor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monitor quits on startup if there is an existing pm2 daemon with matching name
2 participants