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

[updatenotification] Fix pm2 using detection when pm2 script is inside or outside MagicMirror root folder #3605

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

bugsounet
Copy link
Contributor

This will fix #3576

@FrankBlackMG:

I don't use *env.unique_id because some others modules can use pm2 too for starting a service and unique_id is the same (this will make confusion)
So I check name and pm_id for found it

@bugsounet bugsounet changed the base branch from master to develop October 27, 2024 17:14
@khassel khassel requested a review from rejas October 27, 2024 18:32
@rejas rejas merged commit 399e2ae into MagicMirrorOrg:develop Oct 28, 2024
10 checks passed
@FrankBlackMG
Copy link

@bugsounet , I think what you have is fine too. But I will say that unique_id is in fact unique and can't be confused with any other process. There will be no other pm2 processes using this. It's a GUID/UUID.

https://github.com/Unitech/pm2/blob/5e20239d63c0664cadda12c52f60e03caaf8cc14/lib/God.js#L102

@bugsounet
Copy link
Contributor Author

Not sure, just check this:

<...>
list.forEach((pm) => {
  Log.debug(`[PM2] pm2 name: ${pm.name} -- in process env: ${process.env.name} unique_id: ${process.env.unique_id}`)
  Log.debug(`[PM2] pm2 pm_id: ${pm.pm_id} -- in process env: ${process.env.pm_id}`)
  if (pm.pm2_env.status === "online" && process.env.name === pm.name && +process.env.pm_id === +pm.pm_id) {
    this.PM2 = pm.name;
    this.usePM2 = true;
    Log.info("updatenotification: [PM2] You are using pm2 with", this.PM2);
    resolve(true);
  }
});
<...>

Result:

[DEBUG] [PM2] pm2 name: VLCServer -- in process env: MagicMirror unique_id: 6081a1d6-1ada-4dc9-81e5-91ee79ad2906 
[DEBUG] [PM2] pm2 pm_id: 0 -- in process env: 2 
[DEBUG] [PM2] pm2 name: librespot -- in process env: MagicMirror unique_id: 6081a1d6-1ada-4dc9-81e5-91ee79ad2906 
[DEBUG] [PM2] pm2 pm_id: 1 -- in process env: 2 
[DEBUG] [PM2] pm2 name: MagicMirror -- in process env: MagicMirror unique_id: 6081a1d6-1ada-4dc9-81e5-91ee79ad2906 
[DEBUG] [PM2] pm2 pm_id: 2 -- in process env: 2
[INFO]  updatenotification: [PM2] You are using pm2 with MagicMirror 

That you can see unique_id is the same on all pm2 process

@FrankBlackMG
Copy link

What you're logging is the MagicMirror process's unique id (this process) i.e. what you need to compare with. That won't change. Try logging the value which is for each of the PM2 managed process.

pm.pm2_env.unique_id

I did this and got different values for each pm2 managed process. One of which was the MagicMirror process and it match the

process.env.unique_id

In my logging I saw unique ids that are different for each pm2 managed process list, one of which matched this process's unique id.

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 6, 2024

i do this in my MMM-Config

pm2 jlist
then loop thru the apps

if(__dirname.startsWith(managed_process.pm2_env.pm_cwd)

this has issues if multiple instances are running from the same MagicMirror folder. but this is an odd env

@FrankBlackMG
Copy link

using unique id covers that case. It doesn't matter where or how PM2 is used to manage MagicMirror or any other process.

if (process.env.unique_id !== undefined && process.env.unique_id === pm.pm2_env.unique_id) {

@bugsounet
Copy link
Contributor Author

That will not work process.env.unique_id will be equal to pm.pm2_env.unique_id:

[DEBUG] [PM2] pm2 name: VLCServer -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 

[DEBUG] [PM2] pm2 name: librespot -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 

[DEBUG] [PM2] pm2 name: MagicMirror -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac

[INFO]  updatenotification: [PM2] You are using pm2 with MagicMirror

Test with your solution:

[DEBUG] [PM2] pm2 name: VLCServer -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 
[INFO]  updatenotification: [PM2] You are using pm2 with VLCServer 

[DEBUG] [PM2] pm2 name: librespot -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 
[INFO]  updatenotification: [PM2] You are using pm2 with librespot 

[DEBUG] [PM2] pm2 name: MagicMirror -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 
[INFO]  updatenotification: [PM2] You are using pm2 with MagicMirror

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 6, 2024

hm, just worked for me

the process.env.unique_id is added by pm2 when the app is launched
and the pm2 info will only match 1

i have 8 processes defined under pm2 only 1 matches
parsed content from pm2 jlist

output.forEach((managed_process) => {
                  if(managed_process.pm2_env.status === 'online' ){
                    if(process.env.unique_id === managed_process.pm2_env.env.unique_id){
                     ...
                    }
                  }
})

i don't break, and only find one matching when others are running

@bugsounet
Copy link
Contributor Author

yes but the other processes are not launched by MM itself

I say:

I don't use *env.unique_id because some others modules can use pm2 too for starting a service and unique_id is the same (this will make confusion)

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 6, 2024

yes but the other processes are not launched by MM itself

i don't understand this..

if they are launched by pm2, then they are launched by pm2. (thru exec or outside MM)
if they are launched by MM, thru exec, then they are NOT launched by pm2

we only care in MM is launched by pm2.. pm2 apps run independent of the launcher
if an MM module uses pm2(exec...) to launch sub'tasks', then its the modules job to find them again if MM is restarted

@bugsounet
Copy link
Contributor Author

don't forget that all exec are executed inside MM process (like a sub process)

@bugsounet
Copy link
Contributor Author

bugsounet commented Nov 6, 2024

[DEBUG] [PM2] pm2 name: VLCServer -- in process env: MagicMirror unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac --> pm2_env.unique_id: 8ef0320a-da01-4155-87ba-d26b04d4aaac 
[INFO]  updatenotification: [PM2] You are using pm2 with VLCServer

So explain me why we are able to detect an MagicMirror pm2 using with this process ?
(by using: if (process.env.unique_id !== undefined && process.env.unique_id === pm.pm2_env.unique_id) {

---> So the result will be : "ok i found MagicMirror with VLCServer process and i will use it for restart MagicMirror"

That why if (pm.pm2_env.status === "online" && process.env.name === pm.name && +process.env.pm_id === +pm.pm_id) { is better

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 6, 2024

I have multiple apps called MagicMirror, but with different IDs'
I don't know what 'vlcserver' means above..

MM is an app.. it has a process env, seen by the node_helpers and base process (before electron createWindow)
when pm2 starts a registered app, it adds that apps unqiue_id to the the process environment

now you can match the process env value with the pm2 app list value by app..
if those match, then you know the pm2 app info to start/stop/restart... as compared to any other app.
if there is no pm2_unique_id, then it was NOT started with pm2. (and can't be restarted w pm2)
I don't see any app identifier in your code.. is this because of the pm2 library?
I use the pm2 command process jlist to get the json for all the apps.

@sdetweil
Copy link
Collaborator

sdetweil commented Nov 6, 2024

my code to detect the id in pm2 for this running MM, using the ID now

        // if the id was not set from config
        if(pm2_id  ==  -1){
          // if the pm2 process_env is set
          if(process.env.unique_id !== undefined){
            // running under pm2
            if (debug) console.log("getting pm2 process list");
            exec("pm2 jlist", (error, stdout, stderr) => {
              if (!error) {
                let o=stdout.toString()
                // maybe there is a pm2 error message in front of the json
                if(!o.startsWith('[')){
                  // remove everything before process list
                  o=o.slice(o.indexOf('['))
                }
                if(debug){
                  console.log("json="+o)
                }
                let output = JSON.parse(o);
                if (debug)
                  console.log(
                    "processing pm2 jlist output, " + output.length + " entries"
                  );
                output.forEach((managed_process) => {
                  if(managed_process.pm2_env.status === 'online' ){
                    if(process.env.unique_id === managed_process.pm2_env.env.unique_id){
                      if (debug)
                        console.info(
                          "found our pm2 entry, id=" + managed_process.pm_id
                        );
                      pm2_id = managed_process.pm_id;
                    }
                  }
                });
              }
            });
          } else {
            if(debug)
              console.info(this.name+" MagicMirror not running under pm2")
          }
        }
        else {
          if(debug){
            console.info(this.name+" pm2 restart app id is ", pm2_id)
          }
        }

@bugsounet
Copy link
Contributor Author

Your part of the code requires a lot of effort but don't works :/

[LOG]   processing pm2 jlist output, 3 entries 
[LOG]   managed_process.pm_id: 0 managed_process.pm2_env.env.unique_id: fb62b1cf-a0ef-4e52-8031-e216a788f39f 
[INFO]  found our pm2 entry, id=0 
[LOG]   managed_process.pm_id: 1 managed_process.pm2_env.env.unique_id: fb62b1cf-a0ef-4e52-8031-e216a788f39f 
[INFO]  found our pm2 entry, id=1 
[LOG]   managed_process.pm_id: 2 managed_process.pm2_env.env.unique_id: fb62b1cf-a0ef-4e52-8031-e216a788f39f 
[INFO]  found our pm2 entry, id=2

I will purpose a new code with process.pm2_env.env.unique_id checking

@bugsounet bugsounet deleted the fix_pm2 branch November 7, 2024 03:56
rejas pushed a commit that referenced this pull request Nov 7, 2024
#3605 : new purpose code with `pm2_env.unique_id` checking
@sdetweil
Copy link
Collaborator

sdetweil commented Nov 7, 2024

ok, i don't see that problem in my module, only one pm2 app matches the environment variable

@sdetweil sdetweil mentioned this pull request Jan 1, 2025
sdetweil added a commit that referenced this pull request Jan 1, 2025
## [2.30.0] - 2025-01-01

Thanks to: @xsorifc28, @HeikoGr, @bugsounet, @khassel,
@KristjanESPERANTO, @rejas, @sdetweil.

> ⚠️ This release needs nodejs version `v20` or `v22 or higher`, minimum
version is `v20.18.1`

### Added

- [core] Add wayland and windows start options to `package.json` (#3594)
- [docs] Add step for npm publishing in release process (#3595)
- [core] Add GitHub workflow to run spellcheck a few days before each
release (#3623)
- [core] Add test flag to `index.html` to pass to module js for test
mode detection (needed by #3630)
- [core] Add export on animation names (#3644)
- [compliments] Add support for refreshing remote compliments file, and
test cases (#3630)
- [linter] Re-add `eslint-plugin-import`now that it supports ESLint v9
(#3586)
- [linter] Re-activate `eslint-plugin-package-json` to lint
`package.json` (#3643)
- [linter] Add linting for markdown files (#3646)
- [linter] Add some handy ESLint rules.
- [calendar] Add ability to display end date for full date events, where
end is not same day (showEnd=true) (#3650)
- [core] Add text to the config.js.sample file about the locale variable
(#3654, #3655)
- [core] Add fetch timeout for all node_helpers (thru undici, forces
node 20.18.1 minimum) to help on slower systems. (#3660) (3661)

### Changed

- [core] Run code style checks in workflow only once (#3648)
- [core] Fix animations export #3644 only on server side (#3649)
- [core] Use project URL in fallback config (#3656)
- [core] Fix Access Denied crash writing js/positions.js (on synology
nas) #3651. new message, MM starts, but no modules showing (#3652)
- [linter] Switch to 'npx' for lint-staged in pre-commit hook (#3658)

### Removed

- [tests] Remove `node-pty` and `drivelist` from rebuilded test (#3575)
- [deps] Remove `@eslint/js` dependency. Already installed with `eslint`
in deep (#3636)

### Updated

- [repo] Reactivate `stale.yaml` as GitHub action to mark issues as
stale after 60 days and close them 7 days later (if no activity) (#3577,
#3580, #3581)
- [core] Update electron dependency to v32 (test electron rebuild) and
all other dependencies too (#3657)
- [tests] All test configs have been updated to allow full external
access, allowing for easier debugging (especially when running as a
container)
- [core] Run and test with node 23 (#3588)
- [workflow] delete exception `allow-ghsas: GHSA-8hc4-vh64-cxmj` in
`dep-review.yaml` (#3659)

### Fixed

- [updatenotification] Fix pm2 using detection when pm2 script is inside
or outside MagicMirror root folder (#3576) (#3605) (#3626) (#3628)
- [core] Fix loading node_helper of modules: avoid black screen, display
errors and continue loading with next module (#3578)
- [weather] Change default value for weatherEndpoint of provider
openweathermap to "/onecall" (#3574)
- [tests] Fix electron tests with mock dates, the mock on server side
was missing (#3597)
- [tests] Fix testcases with hard coded Date.now (#3597)
- [core] Fix missing `basePath` where `location.host` is used (#3613)
- [compliments] croner library changed filenames used in latest version
(#3624)
- [linter] Fix ESLint ignore pattern which caused that default modules
not to be linted (#3632)
- [core] Fix module path in case of sub/sub folder is used and use
path.resolve for resolve `moduleFolder` and `defaultModuleFolder` in
app.js (#3653)
- [calendar] Update to resolve issues #3098 #3144 #3351 #3422 #3443
#3467 #3537 related to timezone changes
- [calendar] Fix #3267 (styles array), also fixes event with both exdate
AND recurrence(and testcase)
- [calendar] Fix showEndsOnlyWithDuration not working, #3598, applies
ONLY to full day events
- [calendar] Fix showEnd for Full Day events (#3602)
- [tests] Suppress "module is not defined" in e2e tests (#3647)
- [calendar] Fix #3267 (styles array, really this time!)
- [core] Fix #3662 js/positions.js created incorrectly

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Michael Teeuw <[email protected]>
Co-authored-by: Kristjan ESPERANTO <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Karsten Hassel <[email protected]>
Co-authored-by: Ross Younger <[email protected]>
Co-authored-by: Veeck <[email protected]>
Co-authored-by: Bugsounet - Cédric <[email protected]>
Co-authored-by: jkriegshauser <[email protected]>
Co-authored-by: illimarkangur <[email protected]>
Co-authored-by: vppencilsharpener <[email protected]>
Co-authored-by: veeck <[email protected]>
Co-authored-by: Paranoid93 <[email protected]>
Co-authored-by: Brian O'Connor <[email protected]>
Co-authored-by: WallysWellies <[email protected]>
Co-authored-by: Jason Stieber <[email protected]>
Co-authored-by: jargordon <[email protected]>
Co-authored-by: Daniel <[email protected]>
Co-authored-by: Ryan Williams <[email protected]>
Co-authored-by: Panagiotis Skias <[email protected]>
Co-authored-by: Marc Landis <[email protected]>
Co-authored-by: HeikoGr <[email protected]>
Co-authored-by: Pedro Lamas <[email protected]>
Co-authored-by: veeck <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants