Nest-cli: Nest-cli depends on tree-kill, which depends on `ps` command, which may be unavailable and is not documented

[OJezu]

I'm submitting a...

[ ] Regression 
[X] Bug report
[ ] Feature request
[X] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

When application is started with yarn run start:dev inside a container with no ps program available, after first code change application crashes completely (along with the container)

$ yarn run start:dev
5:39:37 PM - File change detected. Starting incremental compilation...


5:39:39 PM - Found 0 errors. Watching for file changes.
events.js:186
      throw er; // Unhandled 'error' event
      ^

Error: spawn ps ENOENT
    at Process.ChildProcess._handle.onexit (internal/child_process.js:264:19)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
Emitted 'error' event on ChildProcess instance at:
    at Process.ChildProcess._handle.onexit (internal/child_process.js:270:12)
    at onErrorNT (internal/child_process.js:456:16)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn ps',
  path: 'ps',
  spawnargs: [ '-o', 'pid', '--no-headers', '--ppid', 185 ]
}

Expected behavior

Application reloads with new code changes

Minimal reproduction of the problem with instructions

1. Create new docker container from node image
2. Create new app with nest-cli inside the container
3. Start the app with npm run start:dev
4. Change something in application code
5. npm run start:dev crashes completely

What is the motivation / use case for changing the behavior?

Unexpected crashes.

Possible fixes:

• dont' use tree-kill, make child a detached process with detach option, and use process.kill(-childrenPid) to kill entire process tree, faff around with signals and stdio to make sure child process' tree is killed when needed https://nodejs.org/api/child_process.html#child_process_options_detached
• add ps dependency to documentation
• check for ps being available and print sensible error
• ask nicely tree-kill to output more legible error when no ps is available

Environment

 _   _             _      ___  _____  _____  _     _____
| \ | |           | |    |_  |/  ___|/  __ \| |   |_   _|
|  \| |  ___  ___ | |_     | |\ `--. | /  \/| |     | |
| . ` | / _ \/ __|| __|    | | `--. \| |    | |     | |
| |\  ||  __/\__ \| |_ /\__/ //\__/ /| \__/\| |_____| |_
\_| \_/ \___||___/ \__|\____/ \____/  \____/\_____/\___/


[System Information]
OS Version     : Linux 5.0
NodeJS Version : v12.10.0
YARN Version    : 1.19.2
[Nest Information]
platform-express version : 6.7.2
typeorm version          : 6.2.0
common version           : 6.7.2
core version             : 6.7.2

[kamilmysliwiec]

ask nicely tree-kill to output more legible error when no ps is available

Would you like to create an issue in the tree-kill repo?

• add ps dependency to documentation
• check for ps being available and print sensible error

We can do both on our side ^

[b4dnewz]

I've encountered this problem right now, @kamilmysliwiec if you are interested I can make a PR (to nestjs) with the first proposed solution:

dont' use tree-kill, make child a detached process with detach option

I'm running nest in dev mode inside a docker container because I'm interacting with other services and it's a features/fix that I really need

[OJezu]

@b4dnewz not sure it will work on Windows though, might still have to use taskkill program there.
Also, for UNIX-like, if children start their own detached children, those grandchildren won't be hunted down and killed. However this may be an expected outcome - after all they are detached from their parent, so they should not be auto-killed when parent dies.

[b4dnewz]

@OJezu good point

if children start their own detached children

it this the case for nestjs start:dev? i'm not familiar with how it's working in deep but looks like it will spawn one per time process after the build has completed successfully

not sure it will work on Windows though

I'm not sure either but after looking round in github issues I found this comment and this comment in the same page, which may lead to a nice and clean solution, I still haven't tested

In particular the second comment could solve the issue of children and grandchildren

[OJezu]

@b4dnewz start:dev starts the application, if that application starts it's own children is not up to nest.

The comments below mention, that killing process group (with kill(-pid)) works on non-windows. I don't see nothing there about killing process groups on windows.

[b4dnewz]

yep you right, I misunderstood the comment, my english understanding sucks..

[jexperton]

I just spent a couple of days investigating this issue, including unsuccessfully asking for information on Discord.

I've been working on other projects with ts-node-dev which does the same as start:dev so I knew it could work with Docker.

After digging into the nest-cli code, I've found the _killProcess_ function.

This made me realized that I should have clicked on this slightly misleading issue title when I first saw it in the issue list two days ago.

Don't use official debian slim node images. Use debian full or alpine images

+1 for a better documentation.

[OJezu]

@jexperton re: the issue titile
If you were to look up the error message using issue search, you would find this issue. I'm sorry if the title is not searchable, but I'd rather have the title describe the issue, not the symptom. For searching... well, use search, GitHub's, or Google.

[jexperton]

@OJezu you're right I should blame my search skills not the issue title. That's why I agree with documenting this issue. It could be interesting to add a short advice such as:

If you use Docker for development, make sure that your image has ps installed by running the following command:

docker run --rm --entrypoint ps YOUR_IMAGE &> /dev/null && echo "installed" || echo "missing"

[jmlopez-rod]

Ran into this issue today. After installing ps on my docker image it works:

FROM node:12.18.3-buster-slim

RUN apt-get update && apt-get install -y procps

I think a note on using Docker like the one @jexperton suggested would help.

[dimacros]

Lol, this is surely a common problem for those who use Docker.