By default, when you build a node app the start command is going to be npm start.
https://github.com/paketo-buildpacks/npm/blob/master/npm/build.go#L112
This results in a process tree for a running app that looks like this:
Dr-No:node-signals dmikusa$ docker exec -it afc ps auxf
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
vcap 31 0.0 0.1 34404 2760 pts/1 Rs+ 13:22 0:00 ps auxf
vcap 1 0.0 0.0 1052 4 pts/0 Ss 13:21 0:00 /sbin/docker-in
vcap 6 1.2 2.3 740168 47656 pts/0 Sl+ 13:21 0:00 npm
vcap 23 0.0 0.0 4632 784 pts/0 S+ 13:21 0:00 \_ sh -c node
vcap 24 0.2 1.6 561804 32780 pts/0 Sl+ 13:21 0:00 \_ node ap
You have npm, which starts sh which starts node. Unfortunately, npm doesn't relay SIGTERM to it's child processes and wait for them to stop, so if you issue docker stop on this container it stops immediately and an shutdown hooks registered with process.on() do not fire.
You can test this with the attached sample app.
pack build node-signalsdocker run -it node-signals- Then in another terminal run
docker stop on the container
You'll see that it exits immediately and you don't see the messages on exit.
If you add --init and -e TINI_KILL_PROCESS_GROUP=1 to the docker run argument, it gets a little better. Sometimes you can see the message logged that the SIGTERM was caught, but it doesn't wait for the full shutdown.
It appears that npm doesn't relay the SIGTERM. It does relay SIGINT, which you can confirm by repeating the test above and using docker kill -s SIGINT. That doesn't help a log though because the default behavior on Kubernetes and Cloud Foundry is to send a SIGTERM followed by a SIGKILL after a certain timeout period.
Having graceful shutdown work out of the box is important. It's expected by developers so, while you can just override the start command with node app.js and get graceful shutdown working, I don't believe that's an OK solution. It should Just Work™.
Any solution that supports graceful shutdown by default would be acceptable. One possibility, would be to pull the start command out of package.json and copy that as the start command. This would a.) fix graceful shutdown and b.) remove npm and an extraneous shell from the list of running processes. Both wins, IMHO.
It appears that npm start isn't doing much, just setting up PATH and running the command so that seems feasible for the buildpack to handle that instead.
node-signals.tar.gz
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent